Re: [PATCH v6 21/27] scsi-multipath: add delayed disk removal support

On 29/07/2026 17:26, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The unchecked return value of `scsi_mpath_get_head()` in `scsi_mpath_remove_head_work()` can lead to a Use-After-Free (UAF) and double-free.
> --
> 
> --- Patch [21]: [PATCH v6 21/27] scsi-multipath: add delayed disk removal support ---
> commit 10f5e133107a91a4b4df48e21559fe51a6b82a5e
> Author: John Garry <[email protected]>
> 
> scsi-multipath: add delayed disk removal support
> 
> This commit adds core code support for delayed disk removal, introducing a
> worker function that iterates over the scsi_bus_type to invoke a newly
> added mpath_remove_head callback. It also attempts to manage reference
> counting for scsi_mpath_head during this removal process.
> 
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> index 0adc5adb4b234..a42e20d33e422 100644
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
> [ ... ]
>> @@ -367,7 +367,34 @@ static bool scsi_mpath_available_path(struct mpath_device *mpath_device)
>> +static void scsi_mpath_remove_head_work(struct mpath_head *mpath_head)
>> +{
>> +	struct scsi_mpath_head *scsi_mpath_head = to_scsi_mpath_head(mpath_head);
>> +
>> +	/*
>> +	 * scsi_mpath_remove_head_drv() could drop the final reference to
>> +	 * scsi_mpath_head, meaning that we pass a free'd pointer further
>> +	 * drivers - take a reference so that won't happen.
>> +	 */
>> +	scsi_mpath_get_head(scsi_mpath_head);
>> +	bus_for_each_drv(&scsi_bus_type, NULL, scsi_mpath_head,
>> +		scsi_mpath_remove_head_drv);
>> +	scsi_mpath_put_head(scsi_mpath_head);
>> +}
> 
> [Severity: High]
> Can ignoring the return value of scsi_mpath_get_head() cause a use-after-free
> regression?

It should not fail as we already must have a reference

> 
> Looking at scsi_mpath_get_head(), it uses kref_get_unless_zero() which can
> fail and return -ENXIO. If the reference count drops to zero concurrently
> just before scsi_mpath_get_head() is called, the acquisition will fail.
> 
> By proceeding unconditionally when it fails, bus_for_each_drv() could
> operate on an already freed scsi_mpath_head pointer.
> 
> Furthermore, scsi_mpath_put_head() would then invoke kref_put() on an
> object that had a zero reference count, causing a double-free regression.
> 
> Should this code check the return value of scsi_mpath_get_head() and exit
> early if the reference could not be acquired?
>