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

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

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>