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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] module: commit message in module_put() fix is a raw email reply without Signed-off-by
--

commit 37bb54444936a36339279161bf4d23fea92a7c7a
Author: Aaron Tomlin <atomlin@atomlin.com>

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

This commit updates module_put() in kernel/module/main.c to wrap
__module_put() and trace_module_put() in an RCU read-side critical section
when trace_module_put_enabled() is true.

>     module: Fix use-after-free in module_put() tracepoint
>     
>     On Tue, Oct 20, 2026 at 05:22:18PM +0200, Petr Pavlu wrote:
>     > 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?
>     
>     Fair point. Please ignore. I have another idea to avoid a reference count
>     that is off by one.
>     
>     > 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.
>     
>     Agreed.
>     
>     > 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.
>     
>     How about the following to ensure the fast path incurs zero RCU overhead
>     when tracing is disabled, gate the RCU critical section behind
>     trace_module_put_enabled():

[Severity: Low]
This isn't a runtime bug, but the commit message body consists entirely of an
inline mailing-list reply rather than a self-contained changelog, and it is
missing a Signed-off-by tag.

Could the commit message be replaced with a standalone explanation of the
use-after-free race in module_put() (where atomic_dec_if_positive() drops the
final reference on module->refcnt, allowing a concurrent delete_module() ->
free_module() path to free struct module before trace_module_put() reads
module->refcnt and module->name, exposed after commit cb0b50b813f6 removed
preempt_disable()) and how gating guard(rcu)() behind
trace_module_put_enabled() pairs with synchronize_rcu() in free_module(),
along with a Signed-off-by tag?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/an3o4dzjwt6p3wt45twtqsv2t3x527srauhcqv6u7uxtt7zz3k@22ye3vb5tf4e?part=1