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

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?

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);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=25