[PATCH] fix for memory leak in scsi_prep_fn

[PATCH] fix for memory leak in scsi_prep_fn

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 192 bytes --]



This should fix the memory leak Russell King spotted if command
initialisation fails in the request prep function.

The patch is against the scsi-combined-2.5 bkbits.net repository.

James


[-- Attachment #2: Type: text/plain, Size: 1176 bytes --]

===== drivers/scsi/scsi_lib.c 1.63 vs edited =====
--- 1.63/drivers/scsi/scsi_lib.c	Tue Jan 28 10:19:35 2003
+++ edited/drivers/scsi/scsi_lib.c	Tue Jan 28 12:29:45 2003
@@ -796,6 +796,8 @@
 	struct Scsi_Device_Template *STpnt;
 	Scsi_Cmnd *SCpnt;
 	Scsi_Device *SDpnt;
+	int ret;
+
 
 	SDpnt = (Scsi_Device *) q->queuedata;
 	BUG_ON(!SDpnt);
@@ -862,8 +864,6 @@
 	 */
 
 	if (req->flags & (REQ_CMD | REQ_BLOCK_PC)) {
-		int ret;
-
 		/*
 		 * This will do a couple of things:
 		 *  1) Fill in the actual SCSI command.
@@ -884,19 +884,27 @@
 		 * required).
 		 */
 		if ((ret = scsi_init_io(SCpnt)))
-			return ret;
+			goto fail_release_cmd;
 		
 		/*
 		 * Initialize the actual SCSI command for this request.
 		 */
 		if (!STpnt->init_command(SCpnt)) {
-			scsi_release_buffers(SCpnt);
-			return BLKPREP_KILL;
+			ret = BLKPREP_KILL;
+			goto fail_release_buffers;
 		}
 	}
 	/* The request is now prepped, no need to come back here */
 	req->flags |= REQ_DONTPREP;
 	return BLKPREP_OK;
+
+ fail_release_buffers:
+	scsi_release_buffers(SCpnt);
+
+ fail_release_cmd:
+	scsi_put_command(SCpnt);
+
+	return ret;
 }
 
 /*

Re: [PATCH] fix for memory leak in scsi_prep_fn

[James Bottomley]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

On Tue, 2003-01-28 at 12:32, James Bottomley wrote:
> This should fix the memory leak Russell King spotted if command
> initialisation fails in the request prep function.
> 
> The patch is against the scsi-combined-2.5 bkbits.net repository.

Well, OK, I had a closer look at what scsi_init_io is doing.  I
obviously don't want to release the command for a defer.  However,
there's also a bug in scsi_init_io which can cause
end_that_request_first to be called twice for the request.

This hopefully corrects that problem as well as the command leakage.

James


[-- Attachment #2: tmp.diff --]
[-- Type: text/plain, Size: 908 bytes --]

===== drivers/scsi/scsi_lib.c 1.63 vs edited =====
--- 1.63/drivers/scsi/scsi_lib.c	Tue Jan 28 10:19:35 2003
+++ edited/drivers/scsi/scsi_lib.c	Tue Jan 28 12:43:38 2003
@@ -781,11 +781,8 @@
 	printk(KERN_ERR "req nr_sec %lu, cur_nr_sec %u\n", req->nr_sectors,
 			req->current_nr_sectors);
 
-	/*
-	 * kill it. there should be no leftover blocks in this request
-	 */
-	SCpnt = scsi_end_request(SCpnt, 0, req->nr_sectors, 1);
-	BUG_ON(SCpnt);
+	/* release the command and kill it */
+	scsi_put_command(SCpnt);
 	ret = BLKPREP_KILL;
 out:
 	return ret;
@@ -884,6 +881,7 @@
 		 * required).
 		 */
 		if ((ret = scsi_init_io(SCpnt)))
+			/* BLKPREP_KILL return also releases the command */
 			return ret;
 		
 		/*
@@ -891,6 +889,7 @@
 		 */
 		if (!STpnt->init_command(SCpnt)) {
 			scsi_release_buffers(SCpnt);
+			scsi_put_command(SCpnt);
 			return BLKPREP_KILL;
 		}
 	}