* [PATCH] fprobe: Protect fprobe_return() with guard(rcu)()
@ 2026-09-28 0:35 Masami Hiramatsu (Google)
2026-09-28 0:35 ` Masami Hiramatsu (Google)
0 siblings, 1 reply; 8+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-09-28 0:35 UTC (permalink / raw)
To: Steven Rostedt, Paul E . McKenney, Frederic Weisbecker,
Neeraj Upadhyay
Cc: Mathieu Desnoyers, Josef Bacik, Masami Hiramatsu, linux-kernel,
linux-trace-kernel, rcu
Hi,
Here is a bugfix (possible UAF) for fprobe found by Sashiko[1].
[1] https://sashiko.dev/#/bug/linux-e46bcd68-4a56-4f19-a255-e3772980e5e3
I think this fix is a short-term fix to make it safer. Eventually
I would like to replace all guard(rcu)() from fprobe with
preempt_disable_notrace(), because currently it introduces unneeded
overhead to fprobe.
- Introduce new call_rcu_tasks_rude() for async call.
- Add special non-preempt mode flag to rhashtable, which
uses call_rcu_tasks_rude() instead of call_rcu()
- Switching to use synchronize_rcu_tasks_rude() for unregistering.
- Replace call_rcu() with call_rcu_tasks_rude() in BPF.
But this is heavy depends on Tasks RCU, so I would like to check
with the RCU maintainers whether this idea aligns with the concept
behind Tasks RCU updates.
Thanks,
---
Masami Hiramatsu (Google) (1):
fprobe: Protect fprobe_return() with guard(rcu)()
kernel/trace/fprobe.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] fprobe: Protect fprobe_return() with guard(rcu)() 2026-09-28 0:35 [PATCH] fprobe: Protect fprobe_return() with guard(rcu)() Masami Hiramatsu (Google) @ 2026-09-28 0:35 ` Masami Hiramatsu (Google) 2026-09-28 0:51 ` sashiko-bot 2026-09-28 16:10 ` Paul E. McKenney 0 siblings, 2 replies; 8+ messages in thread From: Masami Hiramatsu (Google) @ 2026-09-28 0:35 UTC (permalink / raw) To: Steven Rostedt, Paul E . McKenney, Frederic Weisbecker, Neeraj Upadhyay Cc: Mathieu Desnoyers, Josef Bacik, Masami Hiramatsu, linux-kernel, linux-trace-kernel, rcu From: Masami Hiramatsu (Google) <mhiramat@kernel.org> 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 <sashiko-bot@kernel.org> Closes: https://sashiko.dev/#/bug/linux-e46bcd68-4a56-4f19-a255-e3772980e5e3 Fixes: 657b594b2084 ("fprobe: Fix unregister_fprobe() to wait for RCU grace period") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> --- kernel/trace/fprobe.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c index 9f2d98181779..c3e1580bc648 100644 --- a/kernel/trace/fprobe.c +++ b/kernel/trace/fprobe.c @@ -671,7 +671,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)(); curr = 0; while (size_words > curr) { @@ -687,7 +687,6 @@ static void fprobe_return(struct ftrace_graph_ret *trace, } curr += size; } - preempt_enable_notrace(); } NOKPROBE_SYMBOL(fprobe_return); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] fprobe: Protect fprobe_return() with guard(rcu)() 2026-09-28 0:35 ` Masami Hiramatsu (Google) @ 2026-09-28 0:51 ` sashiko-bot 2026-09-28 12:22 ` Masami Hiramatsu 2026-09-28 16:10 ` Paul E. McKenney 1 sibling, 1 reply; 8+ messages in thread From: sashiko-bot @ 2026-09-28 0:51 UTC (permalink / raw) To: Masami Hiramatsu (Google); +Cc: linux-trace-kernel 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) <mhiramat@kernel.org> 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 <sashiko-bot@kernel.org> Closes: https://sashiko.dev/#/bug/linux-e46bcd68-4a56-4f19-a255-e3772980e5e3 Fixes: 657b594b2084 ("fprobe: Fix unregister_fprobe() to wait for RCU grace period") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fprobe: Protect fprobe_return() with guard(rcu)() 2026-09-28 0:51 ` sashiko-bot @ 2026-09-28 12:22 ` Masami Hiramatsu 2026-09-29 19:39 ` Steven Rostedt 0 siblings, 1 reply; 8+ messages in thread From: Masami Hiramatsu @ 2026-09-28 12:22 UTC (permalink / raw) To: sashiko-reviews; +Cc: sashiko-bot, linux-trace-kernel On Mon, 28 Sep 2026 00:51:14 +0000 sashiko-bot@kernel.org wrote: > > 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? Hm, OK. Maybe we need to add something like checking rcu_watching, but FTRACE_OPS_FL_RCU does not work correctly because fgraph_init_ops() only configures subops (gops->ops). ftrace_startup_subops() does not propagete that flag to graph_ops. (I guess that is yet another problem?) To guarantee no illegal RCU usage or lockdep splats if an instrumented function returns while RCU is offline, we can check if (unlikely(!rcu_is_watching())) return; before acquiring guard(rcu)(). > > 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? As I said above, if we add the rcu_is_watching() checks, we can avoid this issue. > > [ ... ] > > > 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? Yeah, so we also need !rcu_is_watching() check. BTW for x86-64, it seems those call path are removed from ftrace by objtool. > > > 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? Yeah, but the fgraph_ops in fprobe becomes a subops, whose flags are ignored. So fprobe needs to check the rcu_is_watching() by itself. Let me update it. However, I think this is fundamentally unneeded check if we replace guard(rcu)() with preempt-disabled. Thank you, -- Masami Hiramatsu (Google) <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fprobe: Protect fprobe_return() with guard(rcu)() 2026-09-28 12:22 ` Masami Hiramatsu @ 2026-09-29 19:39 ` Steven Rostedt 2026-09-29 23:30 ` Masami Hiramatsu 0 siblings, 1 reply; 8+ messages in thread From: Steven Rostedt @ 2026-09-29 19:39 UTC (permalink / raw) To: Masami Hiramatsu (Google) Cc: sashiko-reviews, sashiko-bot, linux-trace-kernel On Mon, 28 Sep 2026 21:22:51 +0900 Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > Hm, OK. Maybe we need to add something like checking rcu_watching, but > FTRACE_OPS_FL_RCU does not work correctly because fgraph_init_ops() only > configures subops (gops->ops). ftrace_startup_subops() does not propagete > that flag to graph_ops. (I guess that is yet another problem?) > > To guarantee no illegal RCU usage or lockdep splats if an instrumented > function returns while RCU is offline, we can check > > if (unlikely(!rcu_is_watching())) > return; If rcu isn't watching then that is a bug. If you enable CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING then the ftrace_test_recursion_trylock() tests for rcu_is_watching(). We actually had that as default for some time to trigger any ftrace calls that were outside of RCU watching and was fixed. Perhaps we need to enable that on more test machines to see if anything came back and allowed ftrace to trigger without it watching? -- Steve ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fprobe: Protect fprobe_return() with guard(rcu)() 2026-09-29 19:39 ` Steven Rostedt @ 2026-09-29 23:30 ` Masami Hiramatsu 0 siblings, 0 replies; 8+ messages in thread From: Masami Hiramatsu @ 2026-09-29 23:30 UTC (permalink / raw) To: Steven Rostedt; +Cc: sashiko-reviews, sashiko-bot, linux-trace-kernel On Tue, 29 Sep 2026 15:39:06 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Mon, 28 Sep 2026 21:22:51 +0900 > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > > > > Hm, OK. Maybe we need to add something like checking rcu_watching, but > > FTRACE_OPS_FL_RCU does not work correctly because fgraph_init_ops() only > > configures subops (gops->ops). ftrace_startup_subops() does not propagete > > that flag to graph_ops. (I guess that is yet another problem?) > > > > To guarantee no illegal RCU usage or lockdep splats if an instrumented > > function returns while RCU is offline, we can check > > > > if (unlikely(!rcu_is_watching())) > > return; > > If rcu isn't watching then that is a bug. Yeah, I made another fix for this bug. https://lore.kernel.org/all/179064115227.394389.16910234241400391996.stgit@devnote2/ > > If you enable CONFIG_FTRACE_VALIDATE_RCU_IS_WATCHING then the > ftrace_test_recursion_trylock() tests for rcu_is_watching(). We > actually had that as default for some time to trigger any ftrace calls > that were outside of RCU watching and was fixed. > > Perhaps we need to enable that on more test machines to see if anything > came back and allowed ftrace to trigger without it watching? What about adding that config to tools/testing/selftests/ftrace/config? Thank you, > > -- Steve -- Masami Hiramatsu (Google) <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fprobe: Protect fprobe_return() with guard(rcu)() 2026-09-28 0:35 ` Masami Hiramatsu (Google) 2026-09-28 0:51 ` sashiko-bot @ 2026-09-28 16:10 ` Paul E. McKenney 2026-09-29 0:13 ` Masami Hiramatsu 1 sibling, 1 reply; 8+ messages in thread From: Paul E. McKenney @ 2026-09-28 16:10 UTC (permalink / raw) To: Masami Hiramatsu (Google) Cc: Steven Rostedt, Frederic Weisbecker, Neeraj Upadhyay, Mathieu Desnoyers, Josef Bacik, linux-kernel, linux-trace-kernel, rcu On Mon, Sep 28, 2026 at 09:35:59AM +0900, Masami Hiramatsu (Google) wrote: > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > 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. Actually, standard RCU grace periods wait for preemption-disabled regions of code regardless of kernel configuration. So if the original code below was broken, that indicates a bug in RCU. So do you have a reproducer for this? Thanx, Paul > 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 <sashiko-bot@kernel.org> > Closes: https://sashiko.dev/#/bug/linux-e46bcd68-4a56-4f19-a255-e3772980e5e3 > Fixes: 657b594b2084 ("fprobe: Fix unregister_fprobe() to wait for RCU grace period") > Cc: stable@vger.kernel.org > Assisted-by: LLM > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > --- > kernel/trace/fprobe.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c > index 9f2d98181779..c3e1580bc648 100644 > --- a/kernel/trace/fprobe.c > +++ b/kernel/trace/fprobe.c > @@ -671,7 +671,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)(); > > curr = 0; > while (size_words > curr) { > @@ -687,7 +687,6 @@ static void fprobe_return(struct ftrace_graph_ret *trace, > } > curr += size; > } > - preempt_enable_notrace(); > } > NOKPROBE_SYMBOL(fprobe_return); > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fprobe: Protect fprobe_return() with guard(rcu)() 2026-09-28 16:10 ` Paul E. McKenney @ 2026-09-29 0:13 ` Masami Hiramatsu 0 siblings, 0 replies; 8+ messages in thread From: Masami Hiramatsu @ 2026-09-29 0:13 UTC (permalink / raw) To: paulmck Cc: Steven Rostedt, Frederic Weisbecker, Neeraj Upadhyay, Mathieu Desnoyers, Josef Bacik, linux-kernel, linux-trace-kernel, rcu On Mon, 28 Sep 2026 09:10:10 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote: > On Mon, Sep 28, 2026 at 09:35:59AM +0900, Masami Hiramatsu (Google) wrote: > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > 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. > > Actually, standard RCU grace periods wait for preemption-disabled regions > of code regardless of kernel configuration. So if the original code > below was broken, that indicates a bug in RCU. Thanks for pointing, this was my mistake. Sorry about that. And I still think we need a fix to add rcu_is_watching() check. Thank you, > > So do you have a reproducer for this? > > Thanx, Paul > > > 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 <sashiko-bot@kernel.org> > > Closes: https://sashiko.dev/#/bug/linux-e46bcd68-4a56-4f19-a255-e3772980e5e3 > > Fixes: 657b594b2084 ("fprobe: Fix unregister_fprobe() to wait for RCU grace period") > > Cc: stable@vger.kernel.org > > Assisted-by: LLM > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > --- > > kernel/trace/fprobe.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c > > index 9f2d98181779..c3e1580bc648 100644 > > --- a/kernel/trace/fprobe.c > > +++ b/kernel/trace/fprobe.c > > @@ -671,7 +671,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)(); > > > > curr = 0; > > while (size_words > curr) { > > @@ -687,7 +687,6 @@ static void fprobe_return(struct ftrace_graph_ret *trace, > > } > > curr += size; > > } > > - preempt_enable_notrace(); > > } > > NOKPROBE_SYMBOL(fprobe_return); > > > > -- Masami Hiramatsu (Google) <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-29 23:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-28 0:35 [PATCH] fprobe: Protect fprobe_return() with guard(rcu)() Masami Hiramatsu (Google) 2026-09-28 0:35 ` Masami Hiramatsu (Google) 2026-09-28 0:51 ` sashiko-bot 2026-09-28 12:22 ` Masami Hiramatsu 2026-09-29 19:39 ` Steven Rostedt 2026-09-29 23:30 ` Masami Hiramatsu 2026-09-28 16:10 ` Paul E. McKenney 2026-09-29 0:13 ` Masami Hiramatsu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.