From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752346AbbKVPIr (ORCPT ); Sun, 22 Nov 2015 10:08:47 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:58498 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751598AbbKVPIp (ORCPT ); Sun, 22 Nov 2015 10:08:45 -0500 Date: Sun, 22 Nov 2015 07:08:43 -0800 From: Christoph Hellwig To: Bart Van Assche Cc: Christoph Hellwig , James Bottomley , "Martin K. Petersen" , Hannes Reinecke , "linux-scsi@vger.kernel.org" , linux-kernel@vger.kernel.org, Greg Kroah-Hartman Subject: Re: [PATCH] Fix a memory leak in scsi_host_dev_release() Message-ID: <20151122150843.GA22822@infradead.org> References: <564D0224.9040001@sandisk.com> <20151120115212.GA23875@infradead.org> <564F5D36.8010402@sandisk.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <564F5D36.8010402@sandisk.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org See http://www.infradead.org/rpr.html Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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