two leaks in scsi_alloc_sdev failure paths

two leaks in scsi_alloc_sdev failure paths

[Dave Jones]

If the scsi_alloc_queue or the slave_alloc calls in scsi_alloc_device fail,
we forget to release the locally allocated sdev on the failure path.

Coverity #609
Signed-off-by: Dave Jones <davej@redhat.com>

--- linux-2.6/drivers/scsi/scsi_scan.c~	2006-03-08 22:28:50.000000000 -0500
+++ linux-2.6/drivers/scsi/scsi_scan.c	2006-03-08 22:31:38.000000000 -0500
@@ -252,7 +252,7 @@ static struct scsi_device *scsi_alloc_sd
 		/* release fn is set up in scsi_sysfs_device_initialise, so
 		 * have to free and put manually here */
 		put_device(&starget->dev);
-		goto out;
+		goto out_free;
 	}
 
 	sdev->request_queue->queuedata = sdev;
@@ -278,6 +278,8 @@ static struct scsi_device *scsi_alloc_sd
 out_device_destroy:
 	transport_destroy_device(&sdev->sdev_gendev);
 	put_device(&sdev->sdev_gendev);
+out_free:
+	kfree(sdev);
 out:
 	if (display_failure_msg)
 		printk(ALLOC_FAILURE_MSG, __FUNCTION__);

-- 
http://www.codemonkey.org.uk

Re: two leaks in scsi_alloc_sdev failure paths

[Brian King]

Dave Jones wrote:
> If the scsi_alloc_queue or the slave_alloc calls in scsi_alloc_device fail,
> we forget to release the locally allocated sdev on the failure path.

Actually, I think the slave_alloc failure path works today, and this patch
breaks it. Today, in the slave_alloc failure path, the release function called
as a result of the put_device in out_device_destroy should end up freeing the
sdev. Your patch will result in a double free in this path.

> --- linux-2.6/drivers/scsi/scsi_scan.c~	2006-03-08 22:28:50.000000000 -0500
> +++ linux-2.6/drivers/scsi/scsi_scan.c	2006-03-08 22:31:38.000000000 -0500
> @@ -252,7 +252,7 @@ static struct scsi_device *scsi_alloc_sd
>  		/* release fn is set up in scsi_sysfs_device_initialise, so
>  		 * have to free and put manually here */
>  		put_device(&starget->dev);
> -		goto out;
> +		goto out_free;

Rather than this change, I think just adding a kfree(sdev) before the goto out would
accomplish what you want.

Brian

-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center

Re: two leaks in scsi_alloc_sdev failure paths

[Dave Jones]

On Thu, Mar 09, 2006 at 09:07:34AM -0600, Brian King wrote:
 > Dave Jones wrote:
 > >If the scsi_alloc_queue or the slave_alloc calls in scsi_alloc_device fail,
 > >we forget to release the locally allocated sdev on the failure path.
 > 
 > Actually, I think the slave_alloc failure path works today, and this patch
 > breaks it. Today, in the slave_alloc failure path, the release function 
 > called
 > as a result of the put_device in out_device_destroy should end up freeing 
 > the sdev.

Correct, thanks.

 > Rather than this change, I think just adding a kfree(sdev) before the goto 
 > out would accomplish what you want.

looks a lot simpler.

Thanks.

		Dave



If the scsi_alloc_queue or the slave_alloc calls in scsi_alloc_device fail,
we forget to release the locally allocated sdev on the failure path.

Coverity #609
Signed-off-by: Dave Jones <davej@redhat.com>

--- linux-2.6/drivers/scsi/scsi_scan.c~	2006-03-09 10:19:51.000000000 -0500
+++ linux-2.6/drivers/scsi/scsi_scan.c	2006-03-09 10:20:20.000000000 -0500
@@ -252,6 +252,7 @@ static struct scsi_device *scsi_alloc_sd
 		/* release fn is set up in scsi_sysfs_device_initialise, so
 		 * have to free and put manually here */
 		put_device(&starget->dev);
+		kfree(sdev);
 		goto out;
 	}
 

-- 
http://www.codemonkey.org.uk