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

[PATCH v2] 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 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@vger.kernel.org
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
Changes since v1:

 - Drop guard(rcu)() and relocate trace_module_put() before the atomic
   decrement instead, avoiding RCU overhead on every module_put() while
   safely relying on the caller's held reference (Petr Pavlu)

 - Link to v1: https://lore.kernel.org/all/20261019034857.654569-1-atomlin@atomlin.com/
---
 kernel/module/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index c1b34dc1e89a..0c8df0984bc2 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -953,9 +953,9 @@ void module_put(struct module *module)
 	int ret;
 
 	if (module) {
+		trace_module_put(module, _RET_IP_);
 		ret = atomic_dec_if_positive(&module->refcnt);
 		WARN_ON(ret < 0);	/* Failed to put refcount */
-		trace_module_put(module, _RET_IP_);
 	}
 }
 EXPORT_SYMBOL(module_put);
-- 
2.55.0

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@vger.kernel.org
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>

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

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

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@atomlin.com/

-- 
Thanks,
Petr

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@vger.kernel.org
> > Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> 
> This version #2 looks ok to me. I plan to pick it up on the modules
> tree.
> 
> Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

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@atomlin.com/

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@atomlin.com/

Kind regards,
-- 
Aaron Tomlin

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

[Petr Pavlu]

On 11/9/26 3:28 PM, Aaron Tomlin wrote:
> 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@vger.kernel.org
>>> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
>>
>> This version #2 looks ok to me. I plan to pick it up on the modules
>> tree.
>>
>> Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
> 
> 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@atomlin.com/
> 
> 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.
Why is it important to preserve what this tracepoint reports? The value
isn't off by one, rather the tracepoint would now report the value
before the decrement.

I'm trying to understand why this case matters enough to justify the
additional complexity in version #3.

-- 
Cheers,
Petr