lib: Fix a memory leak in scsi_cdb_persistent_reserve_out()

Message-id: <1383729402-27559-5-git-send-email-pbonzini@redhat.com>
Patchwork-id: 55499
O-Subject: [PATCH 04/11] lib: Fix a memory leak in scsi_cdb_persistent_reserve_out()
Bugzilla: 1026820
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Acked-by: Orit Wasserman <owasserm@redhat.com>
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

From: Bart Van Assche <bvanassche@acm.org>

If scsi_cdb_persistent_reserve_out() succeeds a call to
scsi_free_scsi_task() won't free any memory allocated with scsi_malloc()
in this function because the memset() call in this function overwrites
the task->mem pointer. Move the memset() call up such that it doesn't
clear task->mem. This makes it possible for the caller of this function
to free the memory allocated by this function by calling
scsi_free_scsi_task(). Merge the error handling code such that the code
for freeing memory only occurs once.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
(cherry picked from commit 3fdc3f2327939174b1691eca6a55915cc3e08b8b)
---
        The problem reported by Coverity is that scsi_malloc accesses
        task->mem, which is uninitialized.

 lib/scsi-lowlevel.c | 37 +++++++++++++++----------------------
 1 file changed, 15 insertions(+), 22 deletions(-)
diff --git a/lib/scsi-lowlevel.c b/lib/scsi-lowlevel.c
index 0f97a79..28d0a08 100644
--- a/lib/scsi-lowlevel.c
+++ b/lib/scsi-lowlevel.c
@@ -1874,15 +1874,14 @@ scsi_cdb_persistent_reserve_out(enum scsi_persistent_out_sa sa, enum scsi_persis
 	int xferlen;
 
 	task = malloc(sizeof(struct scsi_task));
-	if (task == NULL) {
-		return NULL;
-	}
+	if (task == NULL)
+		goto err;
+
+	memset(task, 0, sizeof(struct scsi_task));
 
 	iov = scsi_malloc(task, sizeof(struct scsi_iovec));
-	if (iov == NULL) {
-		free(task);
-		return NULL;
-	}
+	if (iov == NULL)
+		goto err;
 
 	switch(sa) {
 	case SCSI_PERSISTENT_RESERVE_REGISTER:
@@ -1896,11 +1895,8 @@ scsi_cdb_persistent_reserve_out(enum scsi_persistent_out_sa sa, enum scsi_persis
 
 		xferlen = 24;
 		buf = scsi_malloc(task, xferlen);
-		if (buf == NULL) {
-			free(task);
-			free(iov);
-			return NULL;
-		}
+		if (buf == NULL)
+			goto err;
 		
 		memset(buf, 0, xferlen);
 		scsi_set_uint64(&buf[0], basic->reservation_key);
@@ -1917,19 +1913,12 @@ scsi_cdb_persistent_reserve_out(enum scsi_persistent_out_sa sa, enum scsi_persis
 		break;
 	case SCSI_PERSISTENT_RESERVE_REGISTER_AND_MOVE:
 		/* XXX FIXME */
-		free(task);
-		free(iov);
-		return NULL;
+		goto err;
 	default:
-		free(task);
-		free(iov);
-		return NULL;
+		goto err;
 	}
 
-
-	memset(task, 0, sizeof(struct scsi_task));
-	task->cdb[0]   = SCSI_OPCODE_PERSISTENT_RESERVE_OUT;
-
+	task->cdb[0] = SCSI_OPCODE_PERSISTENT_RESERVE_OUT;
 	task->cdb[1] |= sa & 0x1f;
 	task->cdb[2] = ((scope << 4) & 0xf0) | (type & 0x0f);
 	
@@ -1944,6 +1933,10 @@ scsi_cdb_persistent_reserve_out(enum scsi_persistent_out_sa sa, enum scsi_persis
 	scsi_task_set_iov_out(task, iov, 1);
 
 	return task;
+
+err:
+	scsi_free_scsi_task(task);
+	return NULL;
 }
 
 /*