mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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; 4+ 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] 4+ 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 16:10   ` Paul E. McKenney
  0 siblings, 1 reply; 4+ 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	[flat|nested] 4+ messages in thread

* Re: [PATCH] fprobe: Protect fprobe_return() with guard(rcu)()
  2026-09-28  0:35 ` Masami Hiramatsu (Google)
@ 2026-09-28 16:10   ` Paul E. McKenney
  2026-09-29  0:13     ` Masami Hiramatsu
  0 siblings, 1 reply; 4+ 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] 4+ 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; 4+ 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] 4+ messages in thread

end of thread, other threads:[~2026-09-29  0:13 UTC | newest]

Thread overview: 4+ 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 16:10   ` Paul E. McKenney
2026-09-29  0:13     ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®