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

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

[Aaron Tomlin]

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

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

[Petr Pavlu]

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

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

[Aaron Tomlin]

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

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

[Petr Pavlu]

On 10/19/26 1:27 PM, Aaron Tomlin wrote:
> 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.

The trace_module_put() tracepoint is used only for debugging. What
tooling are you referring to that could be confused by changing the
recorded refcnt value?

> 
> 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'm not sure I follow. Unless panic_on_warn=1 is set, trace_module_put()
is still invoked after WARN_ON().

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

Normally, module_put() is not invoked concurrently with free_module()
and doesn't need to synchronize with it, so an RCU read-side critical
section only adds overhead.

The preempt_disable() call in module_put() was previously needed because
of the complex module_ref scheme, but that was replaced in commit
2f35c41f58a9 ("module: Replace module_ref with atomic_t refcnt"). Commit
cb0b50b813f6 ("module: Remove preempt_disable() from module reference
counting.") subsequently removed the no longer needed synchronization.

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

The fix is valid, but I don't see a strong argument to introduce an RCU
read-side critical section in module_put(), especially just for
a tracepoint that will be disabled 99% of the time.

-- 
Thanks,
Petr