Re: [PATCH v4 22/28] scsi-multipath: add delayed disk removal support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Broadcasting `scsi_mpath_head` to all SCSI drivers without halting iteration causes a Use-After-Free and Type Confusion risk if the callback drops the final reference and frees the structure.
- [High] Missing module reference pinning allows `scsi_mod` to be unloaded while delayed removal work is pending, leading to code-execution UAF.
--

--- Patch [22]: [PATCH v4 22/28] scsi-multipath: add delayed disk removal support ---
commit e75160848c3c93c7fdb328d5ebd94850baa881c8
Author: John Garry <john.g.garry@oracle.com>

scsi-multipath: add delayed disk removal support

This commit adds delayed disk removal support to the SCSI multipath core. 
It introduces a new mpath_remove_head callback in the scsi_driver structure 
and implements scsi_mpath_remove_head_work to execute this callback by 
iterating over available SCSI drivers.

> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index 6c50fdde70d65..056b9c35d1144 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c
[ ... ]
> @@ -362,7 +362,25 @@ static bool scsi_mpath_available_path(struct mpath_device *mpath_device)
>  	return false;
>  }
>  
> +static int scsi_mpath_remove_head_drv(struct device_driver *drv, void *data)
> +{
> +	struct scsi_mpath_head *scsi_mpath_head = data;
> +	struct scsi_driver *scsi_driver = to_scsi_driver(drv);
> +
> +	if (scsi_driver->mpath_remove_head)
> +		scsi_driver->mpath_remove_head(scsi_mpath_head);
> +
> +	return 0;
> +}
> +
> +static void scsi_mpath_remove_head_work(struct mpath_head *mpath_head)
> +{
> +	bus_for_each_drv(&scsi_bus_type, NULL, to_scsi_mpath_head(mpath_head),
> +		scsi_mpath_remove_head_drv);
> +}

[Severity: High]
Does this code risk a use-after-free and type confusion regression during bus
iteration?

Looking at scsi_mpath_remove_head_work() in drivers/scsi/scsi_multipath.c, it
unconditionally iterates over all SCSI drivers via bus_for_each_drv(). If the
target driver implements mpath_remove_head and drops the final reference to
scsi_mpath_head freeing the structure, scsi_mpath_remove_head_drv() still
returns 0.

Could the iteration continue and pass the freed scsi_mpath_head pointer to
subsequent drivers on the bus, causing other drivers that implement
mpath_remove_head to dereference a dangling pointer? 

Should the iteration halt by returning a non-zero value once the callback has
been successfully executed?

>  static struct mpath_head_template smpdt = {
> +	.remove_head = scsi_mpath_remove_head_work,

[Severity: High]
Does this delayed work introduction expose a use-after-free regression due to
a missing module reference?

When delayed removal is queued, it relies on mpath_head->drv_module being set
to pin the module. However, looking at scsi_mpath_alloc_head() where smpdt is
assigned:

drivers/scsi/scsi_multipath.c:scsi_mpath_alloc_head() {
    ...
	scsi_mpath_head = kzalloc(sizeof(*scsi_mpath_head), GFP_KERNEL);
	if (!scsi_mpath_head)
		return NULL;
    ...
	if (mpath_head_init(&scsi_mpath_head->mpath_head))
		goto out_free;
	scsi_mpath_head->mpath_head.mpdt = &smpdt;
    ...
}

Since mpath_head->drv_module is never initialized to THIS_MODULE,
try_module_get(mpath_head->drv_module) will return true without taking a
reference. 

Could scsi_mod be unloaded while the delayed remove_work is still pending in
the workqueue, causing the remove_head callback execution to jump to unmapped
memory?

>  	.is_disabled = scsi_mpath_is_disabled,
>  	.is_optimized = scsi_mpath_is_optimized,
[ ... ]

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