On Mon, Oct 19, 2026 at 10:48:44AM +0200, Petr Pavlu wrote:
> On 10/19/26 5:48 AM, 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 acquiring an RCU read-side critical section via
> > guard(rcu)() in module_put(). This ensures that synchronize_rcu() in
> > free_module() blocks until trace_module_put() has run to completion, even
> > across preemption under PREEMPT_RCU configurations, while avoiding the need
> > to disable preemption and preserving the post-decrement reference count
> > tracing semantics.
> >
> > Fixes: cb0b50b813f6 ("module: Remove preempt_disable() from module
> > reference counting.")
> > Cc: [email protected]
> > Signed-off-by: Aaron Tomlin <[email protected]>
> > ---
> > kernel/module/main.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index d0e1e0bd2ad0..8e2d0ab96e6e 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -953,6 +953,7 @@ void module_put(struct module *module)
> > int ret;
> >
> > if (module) {
> > + guard(rcu)();
> > ret = atomic_dec_if_positive(&module->refcnt);
> > WARN_ON(ret < 0); /* Failed to put refcount */
> > trace_module_put(module, _RET_IP_);
Hi Petr,
Thank you for your feedback.
Moving the tracepoint prior to the decrement is certainly a more concise
delta and resolves the use-after-free, but I opted for guard(rcu)() for a
couple of reasons.
Firstly, symmetry in the tracepoints: trace_module_get() records the
resulting reference count post-increment. Moving trace_module_put() would
introduce an asymmetry where puts record the pre-decrement value. When
relinquishing the final active reference, trace_module_put() would log
refcnt=2 rather than refcnt=1 (MODULE_REF_BASE). For scripts or tooling
monitoring reference transitions, this off-by-one difference can be
confusing.
Secondly, keeping the tracepoint after the atomic operation ensures we do
not emit a trace event or dereference mod->name should an unbalanced or
erroneous put occur when refcnt is already non-positive (which triggers
WARN_ON(ret < 0)).
>
> I think a simpler fix is to move the trace_module_put() call before
> atomic_dec_if_positive(). It would change the refcnt recorded in the
> trace from the post-decrement count to the pre-decrement value, but that
> isn't particularly important. For instance, xfs_group_put() and
> objagg_obj_put() take the same approach.
In the case of objagg_obj_put(), the caller invokes the destructor
synchronously within that very same thread once the count hits zero
(i.e. __objagg_obj_put() -> objagg_obj_destroy()), without an RCU grace
period. Placing the tracepoint prior to the decrement is therefore
mandatory there.
As we know, with modules, destruction is asynchronous: module_put() never
frees the module directly. Instead, free_module() already executes
synchronize_rcu() specifically to ensure in-flight operations conclude
before releasing module memory. Prior to commit cb0b50b813f6,
preempt_disable() served as that RCU critical section; guard(rcu)() merely
restores this intended synchronisation contract without disabling
preemption, incurring negligible overhead and fitting naturally with
existing patterns in kernel/module/main.c.
That said, if you feel strongly that avoiding guard(rcu)() and keeping the
diff strictly to a reordering is preferable notwithstanding the tracepoint
asymmetry, I am happy to adapt the patch accordingly.
Kind regards,
--
Aaron Tomlin
