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

[Aaron Tomlin]

On Mon, Nov 09, 2026 at 02:47:31PM +0100, Petr Pavlu wrote:
> 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>

Hi Petr,

> 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/

Thank you.

However, I do prefer the next iteration [1], since the use-after-free is
resolved without altering existing tracepoint semantics or introducing an
off-by-one discrepancy in the recorded reference count. Finally, the fast
code path is fully preserved when the tracepoint is disabled.

I found an example in dma_fence_wait_timeout(). The following is an excerpt
of drivers/dma-buf/dma-fence.c:555-559:

    if (trace_dma_fence_wait_end_enabled()) {
        rcu_read_lock();
        trace_dma_fence_wait_end(fence);
        rcu_read_unlock();
    }

Uses trace_dma_fence_wait_end_enabled() to avoid entering an RCU read-side
critical section unless the tracepoint is active, protecting access to
RCU-freed dma_fence state inside the tracepoint.

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

Kind regards,
-- 
Aaron Tomlin