Re: [PATCH v3 11/17] 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] Memory leak during module unload due to flawed bus driver broadcast for delayed removal.
- [Medium] Use-After-Free (UAF) hazard during SCSI driver iteration because the iterator continues after the structure is freed.
--

commit b2a49437af44776426456e634c1ad321526ae49c
Author: John Garry <john.g.garry@oracle.com>
scsi-multipath: add delayed disk removal support

This commit adds core support for delayed disk removal in SCSI multipath. 
It introduces a new scsi_driver callback to handle the removal work, 
ensuring that the driver maintains a reference to the multipath device while
active.

> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c

[ ... ]

> @@ -355,7 +355,25 @@ static bool scsi_mpath_available_path(struct mpath_device *mpath_device)
> +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;

[Severity: Medium]
Is there a potential use-after-free hazard during the bus iteration here?

When a device hot-unplug triggers delayed head removal, the sd driver will
process it and drop the final reference to the scsi_mpath_head structure,
freeing it.

Because this iterator callback unconditionally returns 0, bus_for_each_drv()
will continue iterating through other registered SCSI drivers (like sr).

Could this result in passing a freed scsi_mpath_head pointer to subsequent
drivers in the iteration? Should the iteration abort once the target driver
successfully handles the removal?

> +}
> +
> +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]
Can this leak memory during module unload?

When the sd module is unloaded, driver_unregister() calls bus_remove_driver(),
which removes the driver from the bus's klist_drivers list before calling
driver_detach() to unbind devices.

If detaching the devices triggers a delayed removal of the multipath head,
this workqueue executes and attempts to find the responsible driver by
broadcasting to all drivers on scsi_bus_type. Since the sd driver was already
removed from the bus list, the iterator fails to find it.

Does this cause the targeted mpath_remove_head callback to be missed,
permanently leaking the scsi_mpath_head and sd_mpath_disk structures?

> +}

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