Re: [PATCH v6 22/27] scsi: sd: add multipath disk class

On 29/07/2026 17:33, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Empty `dev_release` callback `sd_mpath_disk_release` bypasses device/kobject memory management.
> - [Medium] Struct `sd_mpath_disk` is incomplete and fails to manage the multipath gendisk as claimed.
> --
> 
> --- Patch [22]: [PATCH v6 22/27] scsi: sd: add multipath disk class ---
> commit f604e0ae8b45c7be3d202656fafcb45e6c81e7cf
> Author: John Garry <[email protected]>
> 
> scsi: sd: add multipath disk class
> 
> Add a new class, sd_mpath_disk_class, which is the multipath version of
> the scsi_disk class. Structure sd_mpath_disk is introduced to manage the
> multipath gendisk.
> 
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index 599e75f333343..242a15bc2c5bb 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
> [ ... ]
>> @@ -113,6 +114,39 @@ static mempool_t *sd_page_pool;
>>   static mempool_t *sd_large_page_pool;
>>   static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
>>   static struct lock_class_key sd_bio_compl_lkclass;
>> +#ifdef CONFIG_SCSI_MULTIPATH
>> +struct sd_mpath_disk {
>> +	struct scsi_mpath_head		*scsi_mpath_head;
>> +};
> 
> [Severity: Medium]
> The commit message states that sd_mpath_disk is introduced to manage the
> multipath gendisk. However, does this struct lack a struct gendisk pointer
> and an embedded struct device? Without these, it doesn't appear capable of
> fulfilling its documented purpose of managing the multipath disk.

This is added later

> 
>> +
>> +static void sd_mpath_disk_release(struct device *dev)
>> +{
>> +}
> 
> [Severity: High]
> Is it safe to provide an empty dev_release callback here?

This is added later

> 
> When a device is assigned to sd_mpath_disk_class and its reference count
> reaches zero, this callback is invoked. Providing an empty release callback
> circumvents proper resource cleanup, silencing kobject warnings while
> guaranteeing that the enclosing structure is never actually freed.
> 
> Could this cause a memory leak of the device structure during teardown?
> 
>> +
>> +static const struct class sd_mpath_disk_class = {
>> +	.name = "scsi_mpath_disk",
>> +	.dev_release = sd_mpath_disk_release,
>> +};
>