[PATCH] SCSI: fix /proc memory leak in the SCSI core

[PATCH] SCSI: fix /proc memory leak in the SCSI core

[Alan Stern]

The SCSI core calls scsi_proc_hostdir_add() from within
scsi_host_alloc(), but the corresponding scsi_proc_hostdir_rm()
routine is called from within scsi_remove_host().  As a result, if a
host is allocated and then deallocated without ever being registered,
the host's directory in /proc is leaked.

This patch (as1181) fixes this bug in the SCSI core by moving
scsi_proc_hostdir_add() into scsi_host_add().

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

---

Can somebody check to make sure this is compatible with legacy drivers 
using the non-hotplug API?



Index: usb-2.6/drivers/scsi/hosts.c
===================================================================
--- usb-2.6.orig/drivers/scsi/hosts.c
+++ usb-2.6/drivers/scsi/hosts.c
@@ -206,9 +206,10 @@ int scsi_add_host(struct Scsi_Host *shos
 	if (error)
 		goto fail;
 
+	scsi_proc_hostdir_add(shost->hostt);
+
 	if (!shost->shost_gendev.parent)
 		shost->shost_gendev.parent = dev ? dev : &platform_bus;
-
 	error = device_add(&shost->shost_gendev);
 	if (error)
 		goto out;
@@ -259,6 +260,7 @@ int scsi_add_host(struct Scsi_Host *shos
  out_del_gendev:
 	device_del(&shost->shost_gendev);
  out:
+	scsi_proc_hostdir_rm(shost->hostt);
 	scsi_destroy_command_freelist(shost);
  fail:
 	return error;
@@ -407,7 +409,6 @@ struct Scsi_Host *scsi_host_alloc(struct
 		goto fail_kfree;
 	}
 
-	scsi_proc_hostdir_add(shost->hostt);
 	return shost;
 
  fail_kfree:

Re: [PATCH] SCSI: fix /proc memory leak in the SCSI core

[Boaz Harrosh]

Alan Stern wrote:
> The SCSI core calls scsi_proc_hostdir_add() from within
> scsi_host_alloc(), but the corresponding scsi_proc_hostdir_rm()
> routine is called from within scsi_remove_host().  As a result, if a
> host is allocated and then deallocated without ever being registered,
> the host's directory in /proc is leaked.
> 
> This patch (as1181) fixes this bug in the SCSI core by moving
> scsi_proc_hostdir_add() into scsi_host_add().
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> 

Nice this also fixes the premature visibility of the directory
and the theoretical races, that can cause.

> ---
> 
> Can somebody check to make sure this is compatible with legacy drivers 
> using the non-hotplug API?
> 
> 
> 
> Index: usb-2.6/drivers/scsi/hosts.c
> ===================================================================
> --- usb-2.6.orig/drivers/scsi/hosts.c
> +++ usb-2.6/drivers/scsi/hosts.c
> @@ -206,9 +206,10 @@ int scsi_add_host(struct Scsi_Host *shos
>  	if (error)
>  		goto fail;
>  
> +	scsi_proc_hostdir_add(shost->hostt);
> +
>  	if (!shost->shost_gendev.parent)
>  		shost->shost_gendev.parent = dev ? dev : &platform_bus;
> -
>  	error = device_add(&shost->shost_gendev);
>  	if (error)
>  		goto out;
> @@ -259,6 +260,7 @@ int scsi_add_host(struct Scsi_Host *shos
>   out_del_gendev:
>  	device_del(&shost->shost_gendev);
>   out:
> +	scsi_proc_hostdir_rm(shost->hostt);
>  	scsi_destroy_command_freelist(shost);
>   fail:
>  	return error;
> @@ -407,7 +409,6 @@ struct Scsi_Host *scsi_host_alloc(struct
>  		goto fail_kfree;
>  	}
>  
> -	scsi_proc_hostdir_add(shost->hostt);
>  	return shost;
>  
>   fail_kfree:
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] SCSI: fix /proc memory leak in the SCSI core

[Alan Stern]

On Mon, 1 Dec 2008, Boaz Harrosh wrote:

> Alan Stern wrote:
> > The SCSI core calls scsi_proc_hostdir_add() from within
> > scsi_host_alloc(), but the corresponding scsi_proc_hostdir_rm()
> > routine is called from within scsi_remove_host().  As a result, if a
> > host is allocated and then deallocated without ever being registered,
> > the host's directory in /proc is leaked.
> > 
> > This patch (as1181) fixes this bug in the SCSI core by moving
> > scsi_proc_hostdir_add() into scsi_host_add().
> > 
> > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > 
> 
> Nice this also fixes the premature visibility of the directory
> and the theoretical races, that can cause.

I'm not so sure about that...  There's still a window in which the
directory is visible but empty; it's just smaller than before.

Besides, the window that really matters is between sending out the
uevent notification (during device_add) and populating the directory.
The patch doesn't address that problem.  Should it?

Alan Stern

Re: [PATCH] SCSI: fix /proc memory leak in the SCSI core

[James Bottomley]

On Mon, 2008-12-01 at 10:26 -0500, Alan Stern wrote:
> Can somebody check to make sure this is compatible with legacy drivers 
> using the non-hotplug API?

I have this one marked for inclusion pending confirmation of this
request ... has anyone tried it?

James

Re: [PATCH] SCSI: fix /proc memory leak in the SCSI core

[Alan Stern]

On Tue, 13 Jan 2009, James Bottomley wrote:

> On Mon, 2008-12-01 at 10:26 -0500, Alan Stern wrote:
> > Can somebody check to make sure this is compatible with legacy drivers 
> > using the non-hotplug API?
> 
> I have this one marked for inclusion pending confirmation of this
> request ... has anyone tried it?

Not that I've heard.

Alan Stern