Re: [PATCH] Fix a memory leak in scsi_host_dev_release()

Re: [PATCH] Fix a memory leak in scsi_host_dev_release()

[Christoph Hellwig]

Hi Bart,

the memory leak looks real, and your fix looks corret, but I still
don't like it.

I think it's reasonable for SCSI to assume that the final put_device
fully frees the struct device including the name pointer that is
assigned entirely behind the back of the caller.

So I think the fix for this probably should be in the driver core.

Re: [PATCH] Fix a memory leak in scsi_host_dev_release()

[Bart Van Assche]

On 11/20/2015 03:52 AM, Christoph Hellwig wrote:
> the memory leak looks real, and your fix looks corret, but I still
> don't like it.
>
> I think it's reasonable for SCSI to assume that the final put_device
> fully frees the struct device including the name pointer that is
> assigned entirely behind the back of the caller.
>
> So I think the fix for this probably should be in the driver core.

Hello Christoph,

Thanks for the feedback. However, I'm not sure this can be fixed by 
modifying the driver core. If scsi_host_remove() is not called the SCSI 
core doesn't call put_device(&shost->shost_dev). I will post a second 
version of this patch that ensures that the SCSI core always calls 
put_device(&shost->shost_dev).

Bart.

Re: [PATCH] Fix a memory leak in scsi_host_dev_release()

[Christoph Hellwig]

On Fri, Nov 20, 2015 at 09:49:42AM -0800, Bart Van Assche wrote:
> On 11/20/2015 03:52 AM, Christoph Hellwig wrote:
> >the memory leak looks real, and your fix looks corret, but I still
> >don't like it.
> >
> >I think it's reasonable for SCSI to assume that the final put_device
> >fully frees the struct device including the name pointer that is
> >assigned entirely behind the back of the caller.
> >
> >So I think the fix for this probably should be in the driver core.
> 
> Hello Christoph,
> 
> Thanks for the feedback. However, I'm not sure this can be fixed by
> modifying the driver core. If scsi_host_remove() is not called the SCSI core
> doesn't call put_device(&shost->shost_dev). I will post a second version of
> this patch that ensures that the SCSI core always calls
> put_device(&shost->shost_dev).


Oh, I see.  The release method is called on shost_gendev, but the
name that needs to be freed is in shost_dev.  I take my comment on the
core back.

Let's get this patch in for now and see if we can do something about the
creative driver model (ab-)use for struct Scsi_Host in the long run.

Reviewed-by: Christoph Hellwig <hch@lst.de>