Re: [PATCH v5 13/27] scsi-multipath: provide sysfs link from to scsi_device

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

Pre-existing issues:
- [High] Missing cancellation of mpath_head->requeue_work causes Use-After-Free when tearing down the multipath head.
--

--- Patch [13]: [PATCH v5 13/27] scsi-multipath: provide sysfs link from to scsi_device ---
commit 7fc63465ac8f13d95c759573374eebed7fbf19d2
Author: John Garry <[email protected]>

scsi-multipath: provide sysfs link from to scsi_device

This commit provides a sysfs link from a scsi_mpath_device to member
scsi_devices. It adds functions to create and remove these multipath
sysfs links.

[ ... ]

> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 799569b74b535..ce51fcc002bbf 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c

[ ... ]

> @@ -1495,6 +1498,8 @@ void __scsi_remove_device(struct scsi_device *sdev)
>  
>  		if (IS_ENABLED(CONFIG_BLK_DEV_BSG) && sdev->bsg_dev)
>  			bsg_unregister_queue(sdev->bsg_dev);
> +		if (sdev->scsi_mpath_dev)
> +			scsi_mpath_remove_sysfs_link(sdev);
>  		device_unregister(&sdev->sdev_dev);

[Severity: High]
This is a pre-existing issue, but does the teardown of the multipath head
during device removal miss cancelling mpath_head->requeue_work?

Writing to the iopolicy sysfs attribute invokes
scsi_mpath_device_iopolicy_store(), which schedules the work:

drivers/scsi/scsi_multipath.c:scsi_mpath_device_iopolicy_store() {
    ...
    mpath_schedule_requeue_work(mpath_head);
    ...
}

If the SCSI device is removed concurrently (triggering
scsi_mpath_remove_device() and dropping the final reference), the release
function frees the containing structure without ensuring the work is cancelled:

drivers/scsi/scsi_multipath.c:scsi_mpath_head_release() {
    ...
    mpath_head_uninit(mpath_head);
    kfree(scsi_mpath_head);
}

Should mpath_head_uninit() in lib/multipath.c call cancel_work_sync()
to prevent a use-after-free when the requeue work executes on the freed
scsi_mpath_head memory?

[ ... ]

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