Re: [PATCH v4 27/28] scsi: sd: add mpath_numa_nodes dev attribute

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Reading the `mpath_numa_nodes` sysfs attribute before the device is fully initialized causes a guaranteed NULL pointer dereference on `mpath_device->mpath_head`.
--

--- Patch [27]: [PATCH v4 27/28] scsi: sd: add mpath_numa_nodes dev attribute ---
commit cc5c94ceb8cbeeccd7a5678ef260c6bd21a0fd6b
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 f8c9096e6c43c..7255df6e3e971 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -4076,8 +4076,22 @@ static ssize_t sd_mpath_dev_show(struct device *dev,
>  }
>  static DEVICE_ATTR(mpath_dev, 0444, sd_mpath_dev_show, NULL);
>  
> +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: High]
Does this code risk a NULL pointer dereference if the attribute is read early?

When sd_probe() calls device_add_disk(), it exposes the mpath_numa_nodes
sysfs attribute to userspace and fires a uevent. If userspace (like udev)
reads this file before sd_mpath_add_disk() runs, mpath_device->mpath_head
will still be NULL.

When mpath_numa_nodes_show() receives this mpath_device, it passes the NULL
mpath_head to mpath_read_iopolicy(), which will dereference it:

lib/multipath.c:mpath_numa_nodes_show() {
	struct mpath_head *mpath_head = mpath_device->mpath_head;
	...
	if (mpath_read_iopolicy(mpath_head) != MPATH_IOPOLICY_NUMA)
		...
}

include/linux/multipath.h:mpath_read_iopolicy() {
	return READ_ONCE(*mpath_head->iopolicy);
}

Should we check if mpath_device->mpath_head is valid before attempting to
dereference it?

> +}
> +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/20260723093627.2327456-1-john.g.garry@oracle.com?part=27