[PATCH] Improve error checks when generating LUKS master key

Hello!

I've used cryptsetup for a while now and started looking at the code. I 
thought I'd share some additional error checks.

I noticed that when a new master key is generated (when doing 
luksFormat) a call to getRandom did not check the return value. If 
getRandom were to fail, the master key would not be set (or set to the 
uninitialized memory previously allocated), but aside from an error 
message the program would continue as if nothing happened.

The patch merely checks the return value of getRandom and fails if 
getRandom fails. I also removed an unused variable in the getRandom 
function itself. In that function there were two variables r, one 
shadowing the other, so I removed one which wasn't used.

The change is fairly simple, it compiles and "make test" in the luks 
sub-directory succeeds as far as I can tell, but I haven't tested it 
further.

Regards,
Erik Edin

Index: lib/setup.c
===================================================================
--- lib/setup.c	(revision 58)
+++ lib/setup.c	(arbetskopia)
@@ -390,7 +390,7 @@
 	}

 	mk = LUKS_generate_masterkey(options->key_size);
-	if(NULL == mk) return -ENOMEM; 
+	if(NULL == mk) return -ENOMEM; // FIXME This may be misleading, since we don't know what went wrong

 #ifdef LUKS_DEBUG
 #define printoffset(entry) logger(options, CRYPT_LOG_ERROR, ("offset of " #entry " = %d\n", (char
*)(&header.entry)-(char *)(&header))
Index: luks/random.c
===================================================================
--- luks/random.c	(revision 58)
+++ luks/random.c	(arbetskopia)
@@ -23,8 +23,6 @@
    closeRandom */
 int getRandom(char *buf, size_t len)
 {
-    int r = 0;
-
     if(openRandom() == -1) {
 	perror("getRandom:");
 	return -EINVAL;
@@ -37,7 +35,7 @@
 	}
 	len-= r; buf += r;
     }
-    return r;
+    return 0;
 }

 void closeRandom() {
Index: luks/keymanage.c
===================================================================
--- luks/keymanage.c	(revision 58)
+++ luks/keymanage.c	(arbetskopia)
@@ -50,6 +50,7 @@
 struct luks_masterkey *LUKS_alloc_masterkey(int keylength)
 { 
 	struct luks_masterkey *mk=malloc(sizeof(*mk) + keylength);
+	if(NULL == mk) return NULL;
 	mk->keyLength=keylength;
 	return mk;
 }
@@ -66,7 +67,13 @@
 struct luks_masterkey *LUKS_generate_masterkey(int keylength)
 {
 	struct luks_masterkey *mk=LUKS_alloc_masterkey(keylength);
-	getRandom(mk->key,keylength);
+	if(NULL == mk) return NULL;
+
+	int r = getRandom(mk->key,keylength);
+	if(r < 0) {
+		LUKS_dealloc_masterkey(mk);
+		return NULL;
+	}
 	return mk;
 }

Index: lib/setup.c
===================================================================
--- lib/setup.c	(revision 58)
+++ lib/setup.c	(arbetskopia)
@@ -390,7 +390,7 @@
 	}

 	mk = LUKS_generate_masterkey(options->key_size);
-	if(NULL == mk) return -ENOMEM; 
+	if(NULL == mk) return -ENOMEM; // FIXME This may be misleading, since we don't know what went wrong

 #ifdef LUKS_DEBUG
 #define printoffset(entry) logger(options, CRYPT_LOG_ERROR, ("offset of " #entry " = %d\n", (char
*)(&header.entry)-(char *)(&header))
Index: luks/random.c
===================================================================
--- luks/random.c	(revision 58)
+++ luks/random.c	(arbetskopia)
@@ -23,8 +23,6 @@
    closeRandom */
 int getRandom(char *buf, size_t len)
 {
-    int r = 0;
-
     if(openRandom() == -1) {
 	perror("getRandom:");
 	return -EINVAL;
@@ -37,7 +35,7 @@
 	}
 	len-= r; buf += r;
     }
-    return r;
+    return 0;
 }

 void closeRandom() {
Index: luks/keymanage.c
===================================================================
--- luks/keymanage.c	(revision 58)
+++ luks/keymanage.c	(arbetskopia)
@@ -50,6 +50,7 @@
 struct luks_masterkey *LUKS_alloc_masterkey(int keylength)
 { 
 	struct luks_masterkey *mk=malloc(sizeof(*mk) + keylength);
+	if(NULL == mk) return NULL;
 	mk->keyLength=keylength;
 	return mk;
 }
@@ -66,7 +67,13 @@
 struct luks_masterkey *LUKS_generate_masterkey(int keylength)
 {
 	struct luks_masterkey *mk=LUKS_alloc_masterkey(keylength);
-	getRandom(mk->key,keylength);
+	if(NULL == mk) return NULL;
+
+	int r = getRandom(mk->key,keylength);
+	if(r < 0) {
+		LUKS_dealloc_masterkey(mk);
+		return NULL;
+	}
 	return mk;
 }