Re: [PATCH V3 0/3] scsi: two fixes in scsi_add_host_with_dma

From: John Garry <john.garry@huawei.com>
To: Ming Lei <ming.lei@redhat.com>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	<linux-scsi@vger.kernel.org>
Cc: Bart Van Assche <bvanassche@acm.org>, Hannes Reinecke <hare@suse.de>
Subject: Re: [PATCH V3 0/3] scsi: two fixes in scsi_add_host_with_dma
Date: Tue, 1 Jun 2021 11:34:20 +0100	[thread overview]
Message-ID: <d37d30cf-8293-e34e-0b4f-eebfcfa56bc9@huawei.com> (raw)
In-Reply-To: <20210531050727.2353973-1-ming.lei@redhat.com>

On 31/05/2021 06:07, Ming Lei wrote:
> Hello Martin,
> 
> 
> Fix two memory leaks and one double free issue in alloc/add host
> code path.
> 
> 
> V3:
> 	- fix memory leak caused in scsi_host_alloc
> 	- comment typo suggested by John
> 
> V2:
> 	- add patch 2 for addressing shost leak in case of adding host
> 	  failure, reported by John Garry.
> 
> Ming Lei (3):
>    scsi: core: use put_device() to release host
>    scsi: core: fix failure handling of scsi_add_host_with_dma
>    scsi: core: put ->shost_gendev.parent in failure handling path
> 
>   drivers/scsi/hosts.c | 25 ++++++++++++-------------
>   1 file changed, 12 insertions(+), 13 deletions(-)
> 
> Cc: Bart Van Assche <bvanassche@acm.org>
> Cc: John Garry <john.garry@huawei.com>
> Cc: Hannes Reinecke <hare@suse.de>

I tested this again with the same experiment as before to error in
scsi_add_host_with_dma(), and we don't call scsi_host_dev_release() now. 
The shost_gendev dev refcount is 2 upon exit in scsi_add_host_with_dma().

We don't call scsi_host_cls_release() either, so I guess a ref count is 
leaked for shost_dev - I see its refcount is 1 at exit in 
scsi_add_host_with_dma(). We have the device_initialize(), device_add(), 
device_del() in the alloc and add host functions, but I don't know who 
is responsible for the final "device put".

Thanks,
John