Re: [PATCH v7 01/27] libmultipath: Add initial framework

[John Garry <john.garry@linux.dev>]

On 9/1/26 12:03, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing synchronize_srcu() before cleanup_srcu_struct() in mpath_head_uninit() leads to SRCU leaks and use-after-free.
> --
> 
> commit 6f039c958c0ecba8ea01ef606d22d9dd4d06104b
> Author: John Garry <john.garry@linux.dev>
> 
> libmultipath: Add initial framework
>      
> This commit adds an initial framework for libmultipath, a library for
> multipath-capable block drivers. It introduces basic support to add and
> remove the mpath_head structure, which manages available paths and path
> selection.
> 
>> diff --git a/lib/multipath.c b/lib/multipath.c
>> new file mode 100644
>> index 0000000000000..f7b26c66eac7c
>> --- /dev/null
>> +++ b/lib/multipath.c
>> @@ -0,0 +1,67 @@
> [ ... ]
>> +void mpath_head_uninit(struct mpath_head *mpath_head)
>> +{
>> +	refcount_t *refcount = &mpath_head->refcount;
>> +
>> +	if (!refcount_dec_and_test(refcount))
>> +		wait_var_event(refcount, !refcount_read(refcount));
>> +	cleanup_srcu_struct(&mpath_head->srcu);
> 
> [Severity: High]
> Is it possible we need a synchronize_srcu() call before cleanup_srcu_struct()
> in mpath_head_uninit()?

synchronize_srcu() is called per-path as that path is removed - see 
sd_mpath_remove(). When all paths are gone, then mpath_head_uninit() 
would be called.

> 
> Active SRCU readers could trigger a WARN_ON(srcu_readers_active(ssp)) in
> cleanup_srcu_struct(), resulting in an SRCU leak. More critically, when the
> memory containing mpath_head is subsequently freed by the caller, any
> still-executing SRCU readers will experience a use-after-free crash when they
> call srcu_read_unlock() or access mpath_head fields.
> 
> During device removal, mpath_head_uninit() waits for mpath_head->refcount to
> reach 0. However, SRCU read-side critical sections do not increment this base
> refcount to maintain lockless fast-paths. Furthermore, del_gendisk() for
> bio-based queues does not automatically freeze the queue or wait for
> q_usage_counter to drain active submit_bio calls. Thus, cleanup_srcu_struct()
> might be called while readers are still executing.
> 
>> +}
>> +EXPORT_SYMBOL_GPL(mpath_head_uninit);
>