Re: [PATCH v4 18/28] scsi-multipath: revalidate paths upon device unblock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The newly added call to `scsi_mpath_revalidate_paths()` in `scsi_internal_device_unblock()` races with device initialization and teardown, leading to a NULL pointer dereference or Use-After-Free.
- [High] Missing cancellation of `mpath_head->requeue_work` causes use-after-free when `scsi_mpath_head` is freed.
- [Medium] Path revalidation is skipped for devices unblocked via `scsi_internal_device_unblock_nowait()`.
--

--- Patch [18]: [PATCH v4 18/28] scsi-multipath: revalidate paths upon device unblock ---
commit b434ea17223b4b11754a84ac3b62a1af5b823144
Author: John Garry <john.garry@linux.dev>

scsi-multipath: revalidate paths upon device unblock

This patch adds functionality to revalidate paths when a scsi_device is
unblocked. It introduces scsi_mpath_revalidate_paths() to clear paths and
requeue bios, and hooks it into scsi_internal_device_unblock().

> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index f2729c0969801..d98c3239c13ad 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -3092,6 +3092,8 @@ static int scsi_internal_device_unblock(struct scsi_device *sdev,
>  
>  	mutex_lock(&sdev->state_mutex);
>  	ret = scsi_internal_device_unblock_nowait(sdev, new_state);
> +	if ((ret == 0) && sdev->scsi_mpath_dev)
> +		scsi_mpath_revalidate_paths(sdev->scsi_mpath_dev);

[Severity: High]
Can this lead to a use-after-free or NULL pointer dereference during device
removal?

If __scsi_remove_device() transitions the device to SDEV_CANCEL and unlocks
state_mutex, it then locklessly calls scsi_mpath_remove_device() which sets
scsi_mpath_head to NULL and frees it.

If scsi_internal_device_unblock() runs concurrently, 
scsi_internal_device_unblock_nowait() will return 0 (success) because the
device state is SDEV_CANCEL.

Does this cause the unblock path to mistakenly proceed and call
scsi_mpath_revalidate_paths(), dereferencing the freed or NULL
scsi_mpath_head?

[Severity: Medium]
Are there callers that bypass scsi_internal_device_unblock() and call
scsi_internal_device_unblock_nowait() directly?

If drivers directly call the nowait variant to unblock a device, it seems
they will successfully transition the device to SDEV_RUNNING but skip this
new path revalidation. Will this leave their queued bios stuck?

>  	mutex_unlock(&sdev->state_mutex);
>  
>  	return ret;
> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index 6a2b06c148c1a..b896d28a7dd9a 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
> [ ... ]
> @@ -230,6 +230,14 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
>  	return 0;
>  }
>  
> +void scsi_mpath_revalidate_paths(struct scsi_mpath_device *scsi_mpath_dev)
> +{
> +       struct scsi_mpath_head *scsi_mpath_head = scsi_mpath_dev->scsi_mpath_head;
> +       struct mpath_head *mpath_head = &scsi_mpath_head->mpath_head;
> +
> +       mpath_revalidate_paths(mpath_head);
> +}

[Severity: High]
Does this introduce a race condition with mpath_head->requeue_work?

Calling mpath_revalidate_paths() schedules requeue_work locklessly. If this
revalidation happens right as the device is being removed, is the work item
properly canceled?

If scsi_mpath_head_release() frees the scsi_mpath_head structure without
calling cancel_work_sync() or flush_work() on the embedded requeue_work,
could the mpath_requeue_work() callback execute against freed memory?

> +
>  void scsi_mpath_dev_clear_path(struct scsi_mpath_device *scsi_mpath_dev)
>  {

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