Re: [PATCH] tracing: fix CFI violation in probestub helper

[David Laight]

On Fri, 29 May 2026 19:51:34 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> On Fri, 29 May 2026 22:08:26 +0200
> Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> 
> > On Thu, May 28, 2026 at 04:49:02PM -0400, Steven Rostedt wrote:  
> > > On Sun, 24 May 2026 18:43:01 +0300
> > > Eva Kurchatova <eva.kurchatova@xxxxxxxxxxxxx> wrote:
> > >     
> > > > When multiple callbacks are registered on the same tracepoint, probestub
> > > > will be indirectly called via traceiter helper.
> > > > 
> > > > Pointer to probestub callback resides in __tracepoints section, which is
> > > > excluded from ENDBR checks in objtool. Pointers to regfunc/unregfunc
> > > > callbacks reside in extended structure however, which is not affected.
> > > > 
> > > > Registering multiple callbacks will result in a #CP exception due to
> > > > missed ENDBR in __probestub helper on a CFI-enabled machine.
> > > > 
> > > > Fix this by adding CFI_NOSEAL annotation to probestub declaration.
> > > > 
> > > > Fixes: d5173f753750 ("objtool: Exclude __tracepoints data from ENDBR checks")
> > > > Signed-off-by: Eva Kurchatova <eva.kurchatova@xxxxxxxxxxxxx>    
> 
> > 
> > The only place the function address lives is in that __tracepoint
> > section. Since that is explicitly excluded by objtool, it figures there
> > are no actual references to __probestub and the function goes on the
> > seal list and the kernel explicitly scribbles the ENDBR on boot.
> > 
> > Then, if it ever gets used on an IBT enabled host, *boom*.  
> 
> That makes much more sense.
> 
> > 
> > I agree it would've perhaps been clearer if there was part of a splat in
> > the changelog, but the issue is real afaict.
> > 
> > Also, I do think this:
> >   
> > > > @@ -356,6 +357,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> > > >  	void __probestub_##_name(void *__data, proto)			\
> > > >  	{								\
> > > >  	}								\
> > > > +	CFI_NOSEAL(__probestub_##_name);				\
> > > >  	DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name);
> > > >  
> > > >  #define DEFINE_TRACE_FN(_name, _reg, _unreg, _proto, _args)		\    
> > 
> > could do with a comment, explaining why it wants the NOSEAL.  
> 
> Yes.
> 
> Thus, the above change log is totally incorrect and should be updated to:
> 
>    tprobes uses a stub function of the tracepoint to allow fprobes to
>    attach to the tracepoint call site and have access to its arguments.
>    The stub function is called __probestub_##_name() and is only
>    referenced as a pointer in the tracepoint structure so that the
>    tprobe can have access to it.
> 
>    The issue is that the probstub function is only referenced in the
>    __tracepoint section and objtool thinks nothing calls it. Since it
>    explicitly excludes the __tracepoint section, objtool thinks there
>    are no callers so it puts the probstub function into the seal list
>    and then the kernel scrubs its ENDBR on boot.
> 
>    This becomes an issue if someone were to use a tprobe which will
>    register the probestub as a callback to the tracepoint so that a
>    fprobe may attach to it and get access to the arguments. Without the
>    ENDBR it will make the kernel go BOOM!
> 
> 
> Then have a comment in the patch with:
> 
>  void __probestub_##_name(void *__data, proto)			\
>  {								\
>  }								\
> +/*								\
> + * The probestub is only used for tprobes and not referenced	\
> + * anywhere else. This causes objtool to think it's not called	\
> + * at all and will add it to the seal list which will remove	\
> + * the ENDBR causing issues if a tprobe is ever used.		\
> + */								\

Isn't the sense of that all wrong?
Maybe:
	Annotate the protosub 'CFI_NOSEAL' to stop objtool requesting the
	kernel remove the ENDBR because the only references to the
	function are in the __tracepoint section that objtool doesn't scan.

-- David
 
> +CFI_NOSEAL(__probestub_##_name);				\
>  DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name);
> 
> 
> -- Steve
>