Re: [PATCH v3 16/17] scsi: sd: add mpath_numa_nodes dev attribute

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Build failure due to undefined function `mpath_numa_nodes_show`.

Pre-existing issues:
- [Critical] Race condition during device removal leads to a Use-After-Free of `sd_mpath_disk`.
--

commit 90ab5069ccf099a5f4cd42027fac637f7869d718
Author: John Garry <john.g.garry@oracle.com>

scsi: sd: add mpath_numa_nodes dev attribute

Add an attribute to show multipath NUMA node per-path (scsi_disk).

> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 0c77466f8291a..5e0514304d81f 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -4075,8 +4075,22 @@ static ssize_t sd_mpath_dev_show(struct device *dev,
>  }
>  static DEVICE_ATTR(mpath_dev, 0444, sd_mpath_dev_show, NULL);

[Severity: Critical]
This is a pre-existing issue, but does device removal race with sysfs reads
and cause a Use-After-Free?

Looking at sd_remove(), sd_mpath_remove(sdkp) is called before del_gendisk(),
which frees sd_mpath_disk but leaves the sysfs attributes active:

	if (sdp->scsi_mpath_dev)
		sd_mpath_remove(sdkp);

	scsi_autopm_get_device(sdkp->device);

	device_del(&sdkp->disk_dev);
	del_gendisk(sdkp->disk);

If a local user concurrently reads the mpath_dev sysfs attribute, won't
sd_mpath_dev_show() blindly dereference the freed sd_mpath_disk? Will the
newly added attribute suffer from a similar lifecycle problem?

>  
> +static ssize_t sd_mpath_numa_nodes_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct gendisk *gd = dev_to_disk(dev);
> +	struct scsi_disk *sdkp = gd->private_data;
> +	struct scsi_device *sdev = sdkp->device;
> +	struct scsi_mpath_device *scsi_mpath_dev = sdev->scsi_mpath_dev;
> +	struct mpath_device *mpath_device = &scsi_mpath_dev->mpath_device;
> +
> +	return mpath_numa_nodes_show(mpath_device, buf);

[Severity: Low]
Will this compile correctly? I couldn't find a declaration or definition for
mpath_numa_nodes_show() anywhere. Could this lead to an implicit function
declaration and a subsequent linker error?

> +}
> +static DEVICE_ATTR(mpath_numa_nodes, 0444, sd_mpath_numa_nodes_show, NULL);
> +
>  static struct attribute *sd_mpath_dev_attrs[] = {
>  	&dev_attr_mpath_dev.attr,
> +	&dev_attr_mpath_numa_nodes.attr,
>  	NULL
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703103402.3725011-1-john.g.garry@oracle.com?part=16