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 <[email protected]> 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
