* [PATCH] module: Fix use-after-free in module_put() tracepoint
@ 2026-10-19 3:48 Aaron Tomlin
2026-10-19 8:48 ` Petr Pavlu
0 siblings, 1 reply; 3+ messages in thread
From: Aaron Tomlin @ 2026-10-19 3:48 UTC (permalink / raw)
To: mcgrof, petr.pavlu, da.gomez, samitolvanen
Cc: atomlin, linux-modules, linux-kernel
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: stable@vger.kernel.org
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
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_);
--
2.55.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] module: Fix use-after-free in module_put() tracepoint 2026-10-19 3:48 [PATCH] module: Fix use-after-free in module_put() tracepoint Aaron Tomlin @ 2026-10-19 8:48 ` Petr Pavlu 2026-10-19 11:27 ` Aaron Tomlin 0 siblings, 1 reply; 3+ messages in thread From: Petr Pavlu @ 2026-10-19 8:48 UTC (permalink / raw) To: Aaron Tomlin; +Cc: mcgrof, da.gomez, samitolvanen, linux-modules, linux-kernel 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: stable@vger.kernel.org > Signed-off-by: Aaron Tomlin <atomlin@atomlin.com> > --- > 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_); 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. -- Thanks, Petr ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] module: Fix use-after-free in module_put() tracepoint 2026-10-19 8:48 ` Petr Pavlu @ 2026-10-19 11:27 ` Aaron Tomlin 0 siblings, 0 replies; 3+ messages in thread From: Aaron Tomlin @ 2026-10-19 11:27 UTC (permalink / raw) To: Petr Pavlu; +Cc: mcgrof, da.gomez, samitolvanen, linux-modules, linux-kernel 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: stable@vger.kernel.org > > Signed-off-by: Aaron Tomlin <atomlin@atomlin.com> > > --- > > 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-19 11:27 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-19 3:48 [PATCH] module: Fix use-after-free in module_put() tracepoint Aaron Tomlin 2026-10-19 8:48 ` Petr Pavlu 2026-10-19 11:27 ` Aaron Tomlin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®