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

[Peter Zijlstra <peterz@infradead.org>]

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@virtuozzo.com> 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@virtuozzo.com>
> 
> Wait! The probestub is not in the __tracepoints section. At least it
> shouldn't be. Are you sure there's not another issue here?
> 
> #define __DEFINE_TRACE_EXT(_name, _ext, proto, args)			\
> 	static const char __tpstrtab_##_name[]				\
> 	__section("__tracepoints_strings") = #_name;			\
> 	extern struct static_call_key STATIC_CALL_KEY(tp_func_##_name);	\
> 	int __traceiter_##_name(void *__data, proto);			\
> 	void __probestub_##_name(void *__data, proto);			\
> 	struct tracepoint __tracepoint_##_name	__used			\
> 	__section("__tracepoints") = {					\
> 
>  Here the structure __tracepoint_##name is in the __tracepoints section.
> 
> 		.name = __tpstrtab_##_name,				\
> 		.key = STATIC_KEY_FALSE_INIT,				\
> 		.static_call_key = &STATIC_CALL_KEY(tp_func_##_name),	\
> 		.static_call_tramp = STATIC_CALL_TRAMP_ADDR(tp_func_##_name), \
> 		.iterator = &__traceiter_##_name,			\
> 		.probestub = &__probestub_##_name,			\

                    ^^^^^^^^^^ this

> 		.funcs = NULL,						\
> 		.ext = _ext,						\
> 	};								\
> 	__TRACEPOINT_ENTRY(_name);					\
> 	int __traceiter_##_name(void *__data, proto)			\
> 	{								\
> 		struct tracepoint_func *it_func_ptr;			\
> 		void *it_func;						\
> 									\
> 		it_func_ptr =						\
> 			rcu_dereference_raw((&__tracepoint_##_name)->funcs); \
> 		if (it_func_ptr) {					\
> 			do {						\
> 				it_func = READ_ONCE((it_func_ptr)->func); \
> 				__data = (it_func_ptr)->data;		\
> 				((void(*)(void *, proto))(it_func))(__data, args); \
> 			} while ((++it_func_ptr)->func);		\
> 		}							\
> 		return 0;						\
> 	}								\
> 	void __probestub_##_name(void *__data, proto)			\
> 	{								\
> 	}
> 
> But above, probestub is just a function defined wherever the tracepoint is
> created.
> 
> In fact, it's just there for fprobes to work. It doesn't get called if you
> add more than one callback to the tracepoint. So your explanation is totally
> bogus.

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*.

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.