Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] tracing: fprobe UAF and lockdep splat due to invalid guard(rcu) --
commit 4b472c8849c5257c65478348507af6f9fee7c3af Author: Masami Hiramatsu (Google) <[email protected]> fprobe: Protect fprobe_return() with guard(rcu)() In fprobe_return(), the shadow-stack iteration and exit_handler invocations were protected by preempt_disable_notrace(). However, unregister_fprobe() and unregister_fprobe_async() (used by BPF kprobe-multi) rely on standard RCU grace periods (synchronize_rcu() and call_rcu()) to wait until the fprobe is no longer in use before freeing it. In preemptible kernels (CONFIG_PREEMPT_RCU=y), standard RCU grace periods do not wait for pure preempt_disable_notrace() critical sections. Consequently, an unregistered fprobe may be freed while a concurrent CPU executing fprobe_return() is still running fp->exit_handler(), causing a use-after-free. To resolve this, protect fprobe_return() with guard(rcu)() matching fprobe_fgraph_entry(). This ensures both synchronous unregister_fprobe() and asynchronous unregister_fprobe_async() safely wait for in-flight exit_handlers to complete via standard RCU grace periods. Reported-by: Sashiko <[email protected]> Closes: https://sashiko.dev/#/bug/linux-e46bcd68-4a56-4f19-a255-e3772980e5e3 Fixes: 657b594b2084 ("fprobe: Fix unregister_fprobe() to wait for RCU grace period") Cc: [email protected] Assisted-by: LLM Signed-off-by: Masami Hiramatsu (Google) <[email protected]> > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c > index 1e9b00997ff27..e34c3f7667848 100644 > --- a/kernel/trace/fprobe.c > +++ b/kernel/trace/fprobe.c > @@ -656,7 +656,7 @@ static void fprobe_return(struct ftrace_graph_ret *trace, > size_words = SIZE_IN_LONG(size); > ret_ip = ftrace_regs_get_instruction_pointer(fregs); > > - preempt_disable_notrace(); > + guard(rcu)(); [Severity: High] Since ftrace operations lacking the FTRACE_OPS_FL_RCU flag can execute in idle contexts where RCU is not watching, does adding guard(rcu)() here trigger an illegal RCU lock usage splat? Additionally, since RCU doesn't track read-side critical sections that start when RCU isn't watching, does this mean synchronize_rcu() during unregistration will fail to wait for these handlers, leaving the fprobe vulnerable to the same use-after-free this patch attempts to fix? [ ... ] Looking at fprobe_fgraph_entry() in the same file: kernel/trace/fprobe.c:fprobe_fgraph_entry() { ... if (WARN_ON_ONCE(!fregs)) return 0; guard(rcu)(); head = rhltable_lookup(&fprobe_ip_table, &func, fprobe_rht_params); ... } [Severity: High] This isn't a bug introduced by this patch, but fprobe_fgraph_entry() also uses guard(rcu)(). Could this pre-existing usage cause the same lockdep splats in idle contexts? Looking at how fgraph operations are initialized: kernel/trace/fgraph.c:fgraph_init_ops() { dst_ops->flags = FTRACE_OPS_FL_PID | FTRACE_OPS_GRAPH_STUB; #ifdef CONFIG_DYNAMIC_FTRACE ... } [Severity: High] This isn't a bug introduced by this patch, but does the lack of FTRACE_OPS_FL_RCU in these global fgraph operations expose all fgraph users to execution when RCU is not watching? Should this flag be added here to prevent tracing callbacks from being invoked in contexts where RCU is offline? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/179055575009.241711.6358052647499787191.stgit@devnote2?part=1
