Re: [PATCH v5 25/27] scsi: sd: add mpath_dev file

On 27/07/2026 16:33, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `mpath_dev` sysfs attribute is exposed to userspace before the multipath disk's device number is initialized, leading to reads returning `0:0`.
> --
> 
> --- Patch [25]: [PATCH v5 25/27] scsi: sd: add mpath_dev file ---
> commit 947fd6f1a6cab1fe0824d619f628abe9ed07adac
> Author: John Garry <[email protected]>
> 
> scsi: sd: add mpath_dev file
> 
> Add a mpath_dev file so that the multipath disk can be looked up from
> per-path scsi_disk directory.
> 
>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>> index f044b2dfaa9fd..0d73b03a3daee 100644
>> --- a/drivers/scsi/sd.c
>> +++ b/drivers/scsi/sd.c
>> @@ -4109,6 +4109,73 @@ static int sd_mpath_add_disk(struct scsi_disk *sdkp)
> [ ... ]
>> +static ssize_t sd_mpath_dev_show(struct device *dev,
>> +			struct device_attribute *attr, char *page)
>> +{
> [ ... ]
>> +	disk = mpath_head->disk;
>> +	disk_dev = disk_to_dev(disk);
>> +	format_dev_t(devt_str, disk_dev->devt);
> 
> [Severity: Medium]
> Will this return 0:0 if userspace reads the attribute before the multipath
> disk's device number is fully initialized?
> 

I'll just error instead

> Since device_add_disk() on the underlying path exposes these sysfs attributes
> before the multipath disk is fully live, disk_dev->devt might not yet be set
> by the block core.
> 
> Would it be safer to calculate this from disk->major and disk->first_minor,
> which are already initialized?
> 
>> +out_put_disk:
>> +	sd_mpath_put_disk(sd_mpath_disk);
> [ ... ]
>> @@ -4539,7 +4608,7 @@ static int sd_probe(struct scsi_device *sdp)
>>   			sdp->host->rpm_autosuspend_delay);
>>   	}
>>   
>> -	error = device_add_disk(dev, gd, NULL);
>> +	error = device_add_disk(dev, gd, sd_mpath_dev_groups);
> 
> [Severity: Medium]
> Does exposing the mpath_dev sysfs attribute here allow userspace tools like
> udev to read it before the multipath disk is fully initialized?
> 
> If read immediately, before the multipath disk's devt is set, it may return
> an invalid 0:0 device number and cause userspace misbehavior.
> 
>>   	if (error) {
>>   		sd_mpath_fail_probe(sdkp);
>>   		device_unregister(&sdkp->disk_dev);
>