On 6/2/26 6:17 AM, Naveen Kumar Chaudhary wrote:
> kmod_dup_request_delete() calls synchronize_rcu() while holding
> kmod_dup_mutex. This is unnecessary since list_del_rcu() only needs
> the mutex for list manipulation, and the RCU grace period just needs
> to complete before kfree(). Holding the mutex across the grace period
> blocks all other kmod_dup_mutex users for the duration, causing
> unnecessary latency.
> 
> Move synchronize_rcu() after the mutex is released.
> 
> Signed-off-by: Naveen Kumar Chaudhary <[email protected]>
> ---
>  kernel/module/dups.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/module/dups.c b/kernel/module/dups.c
> index 1d720a5311ba..46e74c06ed52 100644
> --- a/kernel/module/dups.c
> +++ b/kernel/module/dups.c
> @@ -88,8 +88,8 @@ static void kmod_dup_request_delete(struct work_struct 
> *work)
>        */
>       mutex_lock(&kmod_dup_mutex);
>       list_del_rcu(&kmod_req->list);
> -     synchronize_rcu();
>       mutex_unlock(&kmod_dup_mutex);
> +     synchronize_rcu();
>       kfree(kmod_req);
>  }
>  

I think the way the module dups code works with kmod_dup_req entries is
generally off and should be properly reworked.

This synchronize_rcu() call doesn't actually synchronize with anything
because the code has no RCU read-side critical section. All lookups
performed by kmod_dup_request_lookup() are protected by kmod_dup_mutex.

On the other hand, I suspect that there is a use-after-free in
kmod_dup_request_exists_wait(), where the code releases kmod_dup_mutex
but continues using the obtained kmod_dup_req.

-- 
Thanks,
Petr

Reply via email to