On Fri, 29 May 2026 19:51:34 -0400 Steven Rostedt <[email protected]> wrote:
> On Fri, 29 May 2026 22:08:26 +0200 > Peter Zijlstra <[email protected]> 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 <[email protected]> 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 <[email protected]> > > > > > 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. Ah, I got it. > > > > > 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. \ > + */ \ > +CFI_NOSEAL(__probestub_##_name); \ > DEFINE_STATIC_CALL(tp_func_##_name, __traceiter_##_name); This looks good to me. Eva, can you update the patch? Thanks for fix. > > > -- Steve -- Masami Hiramatsu (Google) <[email protected]>
