Re: [PATCH] fprobe: Protect fprobe_return() with guard(rcu)()

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

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>