Re: [PATCH v5 14/27] scsi-multipath: support iopolicy

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `cancel_work_sync()` for `requeue_work` in the device release path leads to a Use-After-Free.
--

--- Patch [14]: [PATCH v5 14/27] scsi-multipath: support iopolicy ---
commit fea581175f41f33e0cbfa8e1c00312d8a25daa1c
Author: John Garry <[email protected]>

scsi-multipath: support iopolicy
    
Add support to set the multipath iopolicy. The iopolicy member is per
scsi_mpath_head structure. A module param is added so that the default
iopolicy may be set.

> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index 1866f282c15cc..1b955e19a6814 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
[ ... ]
> @@ -97,8 +114,36 @@ static ssize_t scsi_mpath_device_vpd_id_show(struct device *dev,
>  }
>  static DEVICE_ATTR(vpd_id, S_IRUGO, scsi_mpath_device_vpd_id_show, NULL);
>  
> +static ssize_t scsi_mpath_device_iopolicy_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	struct scsi_mpath_head *scsi_mpath_head =
> +		container_of(dev, struct scsi_mpath_head, dev);
> +	struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
> +
> +	if (!mpath_iopolicy_store(&scsi_mpath_head->iopolicy, buf))
> +		return -EINVAL;
> +
> +	mpath_clear_paths(mpath_head);
> +	mpath_schedule_requeue_work(mpath_head);

[Severity: High]
Does this code introduce a use-after-free risk during device teardown?

If a user writes to the iopolicy sysfs attribute here, mpath_schedule_requeue_work()
is called, which schedules mpath_head->requeue_work asynchronously. 

If the SCSI device is subsequently unbound or removed before this work completes,
scsi_mpath_head_release() frees the scsi_mpath_head structure. It doesn't appear
to call cancel_work_sync() to ensure the scheduled work is stopped before freeing.

When the workqueue eventually executes mpath_requeue_work(), could it dereference
the freed mpath_head memory when accessing requeue_lock?

lib/multipath.c:mpath_requeue_work() {
    ...
    spin_lock_irq(&mpath_head->requeue_lock);
    ...
}

> +	return count;
> +}

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