Re: [PATCH v2] module: Fix use-after-free in module_put() tracepoint

[Petr Pavlu]

On 10/22/26 8:45 PM, Aaron Tomlin wrote:
> A use-after-free condition can arise in module_put() should a thread be
> preempted immediately after decrementing the module's reference count
> while the module is concurrently being unloaded.
> 
> In module_put(), the module's reference count is decremented via
> atomic_dec_if_positive() before the module pointer is passed to
> trace_module_put(). Consider a scenario where CPU 0 decrements the
> reference count without reaching the baseline MODULE_REF_BASE level and
> is immediately preempted: it relinquishes its reference while still
> retaining a local pointer to the module. If a concurrent CPU 1
> subsequently drops the final active reference, the module's reference
> count reaches MODULE_REF_BASE. This permits the module unload sequence
> (via delete_module() or the initialisation-failure tear-down path) to
> proceed and invoke free_module().
> 
> Function free_module() invokes synchronize_rcu() and subsequently
> releases the memory backing struct module via free_mod_mem(). Because
> CPU 0 was preempted outside an RCU read-side critical section,
> synchronize_rcu() does not await CPU 0. When CPU 0 eventually resumes
> execution, it passes the dangling module pointer to trace_module_put(),
> which dereferences mod->name and mod->refcnt, culminating in a
> use-after-free. This sequence of events is illustrated below:
> 
>     CPU 0                                       CPU 1
>     -----                                       -----
> 
>     module_put(module)
>       atomic_dec_if_positive()
>       [preempted]
>                                         module_put(module)
>                                           atomic_dec_if_positive()
>                                           // drops final reference
>                                         free_module(module)
>                                           synchronize_rcu()
>                                           free_mod_mem(module)
>       trace_module_put(module, ...)
>         mod->name
> 
> Resolve this by moving trace_module_put() prior to
> atomic_dec_if_positive(). Because the caller holds an active reference
> to the module at this point, the module structure and its name are
> guaranteed to remain valid throughout the tracepoint invocation without
> requiring preemption disabling or RCU synchronisation.
> 
> Fixes: cb0b50b813f6 ("module: Remove preempt_disable() from module reference counting.")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Aaron Tomlin <atomlin@xxxxxxxxxxx>

This version #2 looks ok to me. I plan to pick it up on the modules
tree.

Reviewed-by: Petr Pavlu <petr.pavlu@xxxxxxxx>

The additional version #3 [1] seems unnecessarily complicated to me. It
is ok to report the value of module->refcnt before it is decremented. As
noted earlier, other code, such as xfs_group_put() and objagg_obj_put(),
takes the same approach.

[1] https://lore.kernel.org/linux-modules/20261103163103.13195-1-atomlin@xxxxxxxxxxx/

-- 
Thanks,
Petr