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

On 27/07/2026 16:06, [email protected] wrote:
> 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?

I'll check it

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