Re: [PATCH v4 05/28] libmultipath: Add support for mpath_device management

[John Garry <john.g.garry@oracle.com>]

On 23/07/2026 10:58, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] mpath_delete_device fails to clear the device from the current_path array before calling mpath_synchronize, guaranteeing an RCU Use-After-Free.
> - [Low] Duplicate function declaration of `mpath_synchronize`.
> --
> 
> --- Patch [5]: [PATCH v4 05/28] libmultipath: Add support for mpath_device management ---
> commit 729c9713b67dacb1c3c3c943a84db42ae105702d
> Author: John Garry <john.g.garry@oracle.com>
> 
> libmultipath: Add support for mpath_device management
> 
> Add support to add or remove a mpath_device as a path. This implements
> functionality similar to NVMe equivalents for path management.
> 
>> diff --git a/include/linux/multipath.h b/include/linux/multipath.h
>> index 57d13eb1450cd..6915c2a979642 100644
>> --- a/include/linux/multipath.h
>> +++ b/include/linux/multipath.h
> [ ... ]
>> @@ -89,6 +92,17 @@ static inline enum mpath_iopolicy_e mpath_read_iopolicy(
>>   void mpath_synchronize(struct mpath_head *mpath_head);
>>   int mpath_set_iopolicy(const char *str, enum mpath_iopolicy_e *iopolicy);
>>   int mpath_get_iopolicy(char *buf, int iopolicy);
>> +bool mpath_clear_current_path(struct mpath_device *mpath_device);
>> +void mpath_synchronize(struct mpath_head *mpath_head);
> 
> [Severity: Low]
> This isn't a bug, but should we remove this duplicate declaration of
> mpath_synchronize() since it is already declared just a few lines above?

will fix

> 
>> +int mpath_add_device(struct mpath_device *mpath_device,
> [ ... ]
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> index 448d879abb354..20c633ad7e1df 100644
>> --- a/lib/multipath.c
>> +++ b/lib/multipath.c
> [ ... ]
>> @@ -50,6 +50,87 @@ void mpath_synchronize(struct mpath_head *mpath_head)
> [ ... ]
>> +bool mpath_delete_device(struct mpath_device *mpath_device)
>> +{
>> +	bool empty;
>> +
>> +	mutex_lock(&mpath_device->mpath_head->lock);
>> +	list_del_rcu(&mpath_device->siblings);
>> +	empty = list_empty(&mpath_device->mpath_head->dev_list);
>> +	mutex_unlock(&mpath_device->mpath_head->lock);
>> +
>> +	mpath_synchronize(mpath_device->mpath_head);
> 
> [Severity: High]
> Does this leave a dangling pointer in the current_path array that could
> lead to a use-after-free?
> 
> Since the deleted device isn't cleared from mpath_head->current_path[],
> new I/O submissions starting after mpath_synchronize() returns could still
> read the device from current_path and use it after it's been freed.
> 
> If a caller tries to avoid this by calling mpath_clear_current_path()
> before mpath_delete_device(), concurrent I/O could potentially re-select
> the device from dev_list and place it back into current_path before the
> deletion acquires the list lock.
> 
> Should mpath_clear_current_path() be called here, exactly between
> list_del_rcu() and mpath_synchronize()?

sd_mpath_remove() calls mpath_clear_current_path() and then 
mpath_delete_device()

> 
>> +
>> +	return empty;
>> +}
>