Live Patching
 help / color / mirror / Atom feed
* [PATCH] compiler_types: Do not mark inline functions notrace
@ 2026-10-21 22:22 Song Liu
  2026-10-21 23:09 ` sashiko-bot
  0 siblings, 1 reply; 5+ messages in thread
From: Song Liu @ 2026-10-21 22:22 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel, live-patching
  Cc: rostedt, mhiramat, mark.rutland, kernel-team, Song Liu

Commit 45959ee7aa64 ("ftrace: Do not function trace inlined functions")
added notrace to the inline macro. At the time, inline was only a hint
with CONFIG_OPTIMIZE_INLINING, and whether a given inline function got
an mcount call depended on the gcc version. Some functions that must
not be traced, such as the paravirt native_*() helpers, were marked
inline and relied on being inlined, so a different gcc could silently
break the function tracer. Making all inline functions notrace removed
that dependency on the compiler.

The reasons for that change no longer hold:

 - Code that must not be instrumented is now marked noinstr, and objtool
   validates that noinstr code only calls other noinstr code, see commit
   c4a33939a7eb ("objtool: Implement noinstr validation"). This check
   is based on sections, not on notrace, so an out-of-line inline
   function called from noinstr code is reported whether it is notrace
   or not. The paravirt helpers that the tracer itself depends on, such
   as pv_native_save_fl(), are now in .noinstr.text or __always_inline.

 - All ftrace callbacks are recursion safe since commit a25d036d939a
   ("ftrace: Reverse what the RECURSION flag means in the ftrace_ops"),
   so tracing a function that the tracer itself calls no longer causes
   recursion problems.

 - CONFIG_OPTIMIZE_INLINING was made always on by commit ac7c3e4ff401
   ("compiler: enable CONFIG_OPTIMIZE_INLINING forcibly") and removed
   by commit 889b3c1245de ("compiler: remove CONFIG_OPTIMIZE_INLINING
   entirely"). The compiler also inlines static functions that are not
   marked inline, and creates .constprop, .isra and .part clones, so
   the set of traceable functions already depends on the compiler.
   notrace on inline does not make it consistent. It only hides the
   inline functions that the compiler chose not to inline.

Hiding these functions has a cost. They cannot be traced with ftrace or
used as BPF fentry/fexit targets. They also cannot be livepatched,
because livepatch redirects functions through ftrace, so a fix to one
of them has to be shipped by livepatching its callers instead.

A function that really must not be traced should be marked notrace or
noinstr explicitly, instead of relying on the inline keyword.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Song Liu <song@kernel.org>
---
 include/linux/compiler_types.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index c5921f139007..ea7f2a928b4d 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -230,9 +230,8 @@ struct ftrace_likely_data {
  * externally visible function. This makes extern inline behave as per gnu89
  * semantics rather than c99. This prevents multiple symbol definition errors
  * of extern inline functions at link time.
- * A lot of inline functions can cause havoc with function tracing.
  */
-#define inline inline __gnu_inline __inline_maybe_unused notrace
+#define inline inline __gnu_inline __inline_maybe_unused
 
 /*
  * gcc provides both __inline__ and __inline as alternate spellings of
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] compiler_types: Do not mark inline functions notrace
  2026-10-21 22:22 [PATCH] compiler_types: Do not mark inline functions notrace Song Liu
@ 2026-10-21 23:09 ` sashiko-bot
  2026-10-21 23:38   ` Song Liu
  2026-10-22 14:27   ` Steven Rostedt
  0 siblings, 2 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-21 23:09 UTC (permalink / raw)
  To: Song Liu, linux-trace-kernel, live-patching
  Cc: rostedt, mhiramat, mark.rutland, kernel-team

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] x86/paravirt: removing notrace from inline instruments pv_ops and resume
- [High] tracing: removing notrace from inline breaks ftrace recursion protection
- [High] rcu: srcu_read_lock_fast_notrace() and tracepoints lose notrace attribute
- [High] bpf: removing notrace from inline exposes trampoline helpers to fentry
- [High] entry: removing notrace from inline instruments unvalidated entry helpers
--

commit 1eb6626c4a5ab70e5858a8a8f1c9edfca235db57
Author: Song Liu <song@kernel.org>

compiler_types: Do not mark inline functions notrace

This commit removes the notrace attribute from the inline macro definition
in include/linux/compiler_types.h so that inline functions emitted
out-of-line by the compiler can be traced by ftrace, attached to via BPF
fentry/fexit, and livepatched.

> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index c5921f1390079..ea7f2a928b4da 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -230,9 +230,8 @@ struct ftrace_likely_data {
>   * externally visible function. This makes extern inline behave as per gnu89
>   * semantics rather than c99. This prevents multiple symbol definition errors
>   * of extern inline functions at link time.
> - * A lot of inline functions can cause havoc with function tracing.
>   */
> -#define inline inline __gnu_inline __inline_maybe_unused notrace
> +#define inline inline __gnu_inline __inline_maybe_unused
>  
>  /*
>   * gcc provides both __inline__ and __inline as alternate spellings of

[Severity: Critical]
Could removing notrace from inline cause a triple-fault during x86 suspend
resume, secondary CPU bringup, or kexec when function tracing or call-depth
tracking is active?

In include/linux/compiler_attributes.h, __always_inline expands to inline:

    #define __always_inline inline __attribute__((__always_inline__))

Because __always_inline only inherits notrace through inline, removing
notrace here strips notrace from both inline and __always_inline functions
whenever their address is taken and the compiler emits an out-of-line copy.

In arch/x86/kernel/paravirt.c (compiled with CC_FLAGS_FTRACE when
CONFIG_PARAVIRT_XXL=y and CONFIG_FUNCTION_TRACER=y), pv_ops takes the
addresses of static inline and static __always_inline native_* helpers
(including native_load_idt, native_load_gdt, native_load_tr_desc,
native_set_ldt, native_write_gdt_entry, native_write_idt_entry,
native_write_ldt_entry, native_load_tls, native_load_gs_index,
native_load_sp0, native_store_tr, native_cpuid, native_read_cr0,
native_write_cr0, native_write_cr4, native_read_msr, native_write_msr,
native_read_msr_safe, native_write_msr_safe, native_read_pmc, and
native_halt):

    .cpu.read_cr0       = native_read_cr0,
    .cpu.write_cr0      = native_write_cr0,
    .cpu.write_cr4      = native_write_cr4,
    .cpu.read_msr       = native_read_msr,
    .cpu.write_msr      = native_write_msr,
    .cpu.read_msr_safe  = native_read_msr_safe,
    .cpu.write_msr_safe = native_write_msr_safe,
    .cpu.read_pmc       = native_read_pmc,
    .cpu.load_tr_desc   = native_load_tr_desc,
    .cpu.set_ldt        = native_set_ldt,
    .cpu.load_gdt       = native_load_gdt,
    .cpu.load_idt       = native_load_idt,

This forces the compiler to emit out-of-line bodies in paravirt.o with
__fentry__ callsites.

During S3 or hibernation resume, __restore_processor_state() in
arch/x86/power/cpu.c is marked notrace (rather than noinstr, so objtool does
not validate it) and calls wrmsrq(), __write_cr4(), write_cr3(),
write_cr0(), and load_idt(&ctxt->idt) -> pv_ops.cpu.load_idt ->
native_load_idt() before MSR_GS_BASE (%gs) is restored:

    static void notrace __restore_processor_state(struct saved_context *ctxt)
    {
        ...
        wrmsrq(MSR_EFER, ctxt->efer);
        __write_cr4(ctxt->cr4);
    #endif
        write_cr3(ctxt->cr3);
        write_cr2(ctxt->cr2);
        write_cr0(ctxt->cr0);

        /* Restore the IDT. */
        load_idt(&ctxt->idt);
        ...
        wrmsrq(MSR_GS_BASE, ctxt->gs_base);

Similarly, start_secondary() -> cpu_init_exception_handling() in
arch/x86/kernel/cpu/common.c calls write_gdt_entry(), load_TR_desc(), and
load_current_idt() -> load_idt() before the secondary CPU IDT is loaded, and
machine_kexec() in arch/x86/kernel/machine_kexec_64.c calls load_segments()
(which resets %gs to 0) followed by the static inline virt_to_phys() in
arch/x86/include/asm/io.h:

    load_segments();

    /* now call it */
    image->start = relocate_kernel_ptr((unsigned long)image->head,
                                       virt_to_phys(control_page),

Won't executing __fentry__ in these paths dereference %gs per-CPU state
(such as current->trace_recursion or call-depth tracking) before %gs or the
IDT is valid?

[Severity: High]
What prevents infinite ftrace recursion when out-of-line inline helpers are
called inside ftrace recursion protection itself?

Ftrace callbacks use ftrace_test_recursion_trylock() ->
trace_test_and_set_recursion() in include/linux/trace_recursion.h at entry
and ftrace_test_recursion_unlock() -> trace_clear_recursion() at exit:

    static __always_inline int trace_test_and_set_recursion(unsigned long ip,
                                                            unsigned long pip,
                                                            int start)
    {
        unsigned int val = READ_ONCE(current->trace_recursion);
        int bit;

        if (trace_warn_on_no_rcu(ip))
            return -1;

        bit = trace_get_context_bit() + start;

In translation units compiled with $(CC_FLAGS_FTRACE) (such as
kernel/livepatch/patch.c, fs/pstore/ftrace.c,
arch/x86/kernel/kprobes/ftrace.c, arch/powerpc/kernel/kprobes-ftrace.c,
kernel/rcu/tree.c, and kernel/trace/trace_events.c under
CONFIG_FUNCTION_SELF_TRACING=y), several functions called before
current->trace_recursion is set (or after it is cleared) are plain static
inline:

1. trace_test_and_set_recursion() evaluates current -> get_current() (plain
   static inline in arch/powerpc/include/asm/current.h and
   arch/m68k/include/asm/current.h), trace_warn_on_no_rcu() ->
   rcu_is_watching() (plain static inline in include/linux/rcutiny.h, and
   calls preempt_disable_notrace() / preempt_enable_notrace() in
   kernel/rcu/tree.c), and trace_get_context_bit() ->
   interrupt_context_level() -> preempt_count() (plain static inline in
   arch/arm64/include/asm/preempt.h).

2. In arch/arm64/include/asm/preempt.h, preempt_count(),
   __preempt_count_add(), __preempt_count_sub(), should_resched(), and
   __preempt_count_dec_and_test() are plain static inline without
   __always_inline or notrace, so preempt_disable_notrace() and
   preempt_enable_notrace() can be traced if emitted out-of-line.

3. In kernel/trace/trace_events.c (compiled with $(CC_FLAGS_FTRACE) when
   CONFIG_FUNCTION_SELF_TRACING=y), __init function_test_events_call() is
   registered without FTRACE_OPS_FL_RECURSION and calls static inline
   helpers tracing_gen_ctx(), tracing_gen_ctx_flags(),
   arch_local_save_flags(), preempt_disable_notrace(), and
   atomic_inc_return() before incrementing ftrace_test_event_disable, as
   well as atomic_dec() and preempt_enable_notrace() when decrementing it:

    trace_ctx = tracing_gen_ctx();
    preempt_disable_notrace();
    cpu = raw_smp_processor_id();
    disabled = atomic_inc_return(&per_cpu(ftrace_test_event_disable, cpu));

If the compiler emits any of these helpers out-of-line, won't tracing them
recurse infinitely before the recursion guard takes effect?

[Severity: High]
Should the _notrace SRCU helpers and tracepoint dispatch wrappers retain an
explicit notrace or __always_inline annotation?

In include/linux/srcu.h, srcu_read_lock_fast_notrace() and
srcu_read_unlock_fast_notrace() are documented as non-traceable, but unlike
srcu_read_lock_notrace() and srcu_read_unlock_notrace() (which are marked
static inline notrace), they rely on inline for notrace:

    /*
     * Used by tracing, cannot be traced and cannot call lockdep.
     * See srcu_read_lock_fast() for more information.
     */
    static inline struct srcu_ctr __percpu *srcu_read_lock_fast_notrace(struct srcu_struct *ssp)

Their callee srcu_check_read_flavor() in include/linux/srcutree.h and the
CONFIG_TINY_SRCU helpers in include/linux/srcutiny.h are also plain static
inline.

Similarly, in include/linux/tracepoint.h, __do_trace_##name (which uses
guard(srcu_fast_notrace)(&tracepoint_srcu) and is called on the cold
static_branch_unlikely() path as well as from notrace void
rust_do_trace_##name), trace_##name, and trace_call__##name are plain static
inline:

    static inline void __do_trace_##name(proto)                     \
    {                                                               \
        TRACEPOINT_CHECK(name)                                      \
        if (cond) {                                                 \
            guard(srcu_fast_notrace)(&tracepoint_srcu);             \
            __DO_TRACE_CALL(name, TP_ARGS(args));                   \
        }                                                           \
    }

Because __do_trace_##name is on the cold branch of trace_##name, compilers
frequently emit it out-of-line. Without notrace on inline, won't these
tracepoint and _notrace SRCU helpers become ftrace and fentry targets?

[Severity: High]
Can attaching a BPF fentry or fexit program to an uninlined helper in
kernel/bpf/trampoline.c or kernel/bpf/memalloc.c trigger infinite recursion
or llist corruption?

Unlike ftrace_ops callbacks, BPF trampolines do not use
ftrace_test_recursion_trylock(). In kernel/bpf/trampoline.c (compiled with
$(CC_FLAGS_FTRACE)), notrace __bpf_prog_enter*(), __bpf_prog_exit*(),
__bpf_tramp_enter(), and __bpf_tramp_exit() call plain static inline helpers
before prog->active recursion protection is acquired or after it is
released:

    static u64 notrace __bpf_prog_enter_recur(struct bpf_prog *prog, struct bpf_tramp_run_ctx *run_ctx)
        __acquires(RCU)
    {
        rcu_read_lock_dont_migrate();

        run_ctx->saved_run_ctx = bpf_set_run_ctx(&run_ctx->run_ctx);

        if (unlikely(!bpf_prog_get_recursion_context(prog))) {

and:

    static void notrace __bpf_prog_exit_recur(struct bpf_prog *prog, u64 start,
                                              struct bpf_tramp_run_ctx *run_ctx)
        __releases(RCU)
    {
        bpf_reset_run_ctx(run_ctx->saved_run_ctx);

        update_prog_stats(prog, start);
        bpf_prog_put_recursion_context(prog);
        rcu_read_unlock_migrate();
    }

The static inline helpers called here include __migrate_disable() and
__migrate_enable() (include/linux/sched.h), rcu_read_unlock_migrate(),
rcu_read_lock(), and rcu_read_unlock() (include/linux/rcupdate.h),
rcu_read_lock_trace() and rcu_read_unlock_trace()
(include/linux/rcupdate_trace.h), bpf_set_run_ctx(), bpf_reset_run_ctx(),
bpf_prog_get_recursion_context(), and bpf_prog_put_recursion_context()
(include/linux/bpf.h), percpu_ref_get() and percpu_ref_put()
(include/linux/percpu-refcount.h), and u64_stats_update_begin_irqsave() and
u64_stats_update_end_irqrestore() (include/linux/u64_stats_sync.h). In
kernel/bpf/verifier.c, btf_id_deny only blocks the out-of-line wrappers
migrate_disable, migrate_enable, ___migrate_enable, rcu_read_unlock_strict,
preempt_count_add, preempt_count_sub, __rcu_read_lock, and
__rcu_read_unlock, not these static inline functions.

In addition, unit_alloc(), unit_free(), and unit_free_rcu() in
kernel/bpf/memalloc.c are marked notrace specifically so BPF programs cannot
attach to them and corrupt free_llist or free_llist_extra, yet they call
plain static inline helpers __llist_add(), __llist_add_batch(), llist_add(),
llist_add_batch(), local_inc_return(), local_add_return(), and local_dec():

    static void notrace unit_free(struct bpf_mem_cache *c, void *ptr)
    {
        ...
        local_irq_save(flags);
        if (local_inc_return(&c->active) == 1) {
            __llist_add(llnode, &c->free_llist);
            cnt = ++c->free_cnt;
        } else {
            llist_add(llnode, &c->free_llist_extra);
        }
        local_dec(&c->active);

If any of these static inline helpers is emitted out-of-line and attached to
via fentry/fexit, won't the trampoline recurse infinitely or corrupt the
bpf_mem_alloc freelist?

[Severity: High]
How are entry, exit, and NMI inline helpers protected against ftrace
instrumentation on architectures without objtool noinstr validation?

CONFIG_HAVE_NOINSTR_VALIDATION is only selected on X86_64 (in
arch/x86/Kconfig, when CONFIG_DEBUG_ENTRY=y); powerpc, arm64, riscv, s390,
loongarch, and x86_32 do not have build-time objtool noinstr validation:

1. On PowerPC, arch/powerpc/include/asm/interrupt.h defines
   interrupt_handler with notrace rather than noinstr because of linker
   branch stub range limitations:

    #define interrupt_handler __visible noinline notrace __no_kcsan __no_sanitize_address

   The notrace interrupt, NMI, and syscall entry/exit paths in
   arch/powerpc/kernel/traps.c and arch/powerpc/kernel/interrupt.c call
   large static inline helpers in arch/powerpc/include/asm/entry-common.h,
   arch/powerpc/include/asm/ftrace.h, and arch/powerpc/include/asm/hw_irq.h,
   including check_return_regs_valid(), arch_interrupt_enter_prepare(),
   arch_interrupt_exit_prepare(), arch_interrupt_nmi_enter_prepare(),
   arch_interrupt_nmi_exit_prepare(), this_cpu_disable_ftrace(),
   this_cpu_enable_ftrace(), this_cpu_set_ftrace_enabled(),
   this_cpu_get_ftrace_enabled(), and (from prep_irq_for_enabled_exit() in
   arch/powerpc/kernel/interrupt.c) exit_must_hard_disable(),
   __hard_EE_RI_disable(), __hard_RI_enable(), __hard_irq_disable(),
   lazy_irq_pending_nocheck(), arch_local_irq_disable(), and
   arch_local_irq_save(). These run while MSR[RI]=0 (unrecoverable exception
   state), in real mode on Book3S-64 NMI entry, before
   this_cpu_set_ftrace_enabled(0) disables ftrace in NMI, with unreconciled
   irq_soft_mask, or after user_enter_irqoff() while RCU is not watching.

2. On arm64, arch/arm64/kernel/entry-common.c is compiled with
   $(CC_FLAGS_FTRACE), and its noinstr handlers
   (arm64_enter_from_kernel_mode(), __arm64_exit_to_kernel_mode(),
   arm64_exit_to_kernel_mode_preempt(), el0_pc(), and __sdei_handler()) call
   plain static inline helpers outside instrumentation_begin() /
   instrumentation_end(): irqentry_exit_to_kernel_mode_preempt()
   (include/linux/irq-entry-common.h), __kern_my_cpu_offset()
   (arch/arm64/include/asm/percpu.h), mte_check_tfsr_entry(),
   mte_check_tfsr_exit(), and mte_disable_tco_entry()
   (arch/arm64/include/asm/mte.h), sme_enter_from_user_mode() and
   sme_exit_to_user_mode() (arch/arm64/include/asm/fpsimd.h),
   instruction_pointer() (arch/arm64/include/asm/ptrace.h), and
   system_supports_mte(), system_has_prio_mask_debugging(),
   system_uses_hw_pan(), and cpu_has_pan()
   (arch/arm64/include/asm/cpufeature.h).

3. In generic entry code (include/linux/irq-entry-common.h),
   enter_from_user_mode() and exit_to_user_mode() call user_exit_irqoff()
   and user_enter_irqoff() (include/linux/context_tracking.h, which are
   plain static inline when !CONFIG_CONTEXT_TRACKING_USER) and
   lockdep_hardirqs_off() and lockdep_hardirqs_on()
   (include/linux/irqflags.h, which are plain static inline when
   !CONFIG_PROVE_LOCKING) outside instrumentation_begin() /
   instrumentation_end().

Won't an out-of-line copy of these helpers be instrumented by ftrace and
execute when MSR[RI]=0, in real mode, or while RCU is not watching?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021222255.3124940-1-song@kernel.org?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] compiler_types: Do not mark inline functions notrace
  2026-10-21 23:09 ` sashiko-bot
@ 2026-10-21 23:38   ` Song Liu
  2026-10-22 13:25     ` Steven Rostedt
  2026-10-22 14:27   ` Steven Rostedt
  1 sibling, 1 reply; 5+ messages in thread
From: Song Liu @ 2026-10-21 23:38 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: linux-trace-kernel, live-patching, rostedt, mhiramat,
	mark.rutland, kernel-team

On Wed, Oct 21, 2026 at 4:09 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [Critical] x86/paravirt: removing notrace from inline instruments pv_ops and resume
> - [High] tracing: removing notrace from inline breaks ftrace recursion protection
> - [High] rcu: srcu_read_lock_fast_notrace() and tracepoints lose notrace attribute
> - [High] bpf: removing notrace from inline exposes trampoline helpers to fentry
> - [High] entry: removing notrace from inline instruments unvalidated entry helpers

Wow. I didn't expect this from Sashiko.

> --
>
> commit 1eb6626c4a5ab70e5858a8a8f1c9edfca235db57
> Author: Song Liu <song@kernel.org>
>
> compiler_types: Do not mark inline functions notrace
>
> This commit removes the notrace attribute from the inline macro definition
> in include/linux/compiler_types.h so that inline functions emitted
> out-of-line by the compiler can be traced by ftrace, attached to via BPF
> fentry/fexit, and livepatched.
>
> > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> > index c5921f1390079..ea7f2a928b4da 100644
> > --- a/include/linux/compiler_types.h
> > +++ b/include/linux/compiler_types.h
> > @@ -230,9 +230,8 @@ struct ftrace_likely_data {
> >   * externally visible function. This makes extern inline behave as per gnu89
> >   * semantics rather than c99. This prevents multiple symbol definition errors
> >   * of extern inline functions at link time.
> > - * A lot of inline functions can cause havoc with function tracing.
> >   */
> > -#define inline inline __gnu_inline __inline_maybe_unused notrace
> > +#define inline inline __gnu_inline __inline_maybe_unused
> >
> >  /*
> >   * gcc provides both __inline__ and __inline as alternate spellings of
>
> [Severity: Critical]
> Could removing notrace from inline cause a triple-fault during x86 suspend
> resume, secondary CPU bringup, or kexec when function tracing or call-depth
> tracking is active?
>
> In include/linux/compiler_attributes.h, __always_inline expands to inline:
>
>     #define __always_inline inline __attribute__((__always_inline__))

I wonder whether we need __always_inline to be notrace, and whether
this resolves any of these concerns.

Thanks,
Song

>
> Because __always_inline only inherits notrace through inline, removing
> notrace here strips notrace from both inline and __always_inline functions
> whenever their address is taken and the compiler emits an out-of-line copy.
>

[...]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] compiler_types: Do not mark inline functions notrace
  2026-10-21 23:38   ` Song Liu
@ 2026-10-22 13:25     ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2026-10-22 13:25 UTC (permalink / raw)
  To: Song Liu
  Cc: sashiko-reviews, linux-trace-kernel, live-patching, mhiramat,
	mark.rutland, kernel-team

On Wed, 21 Oct 2026 16:38:03 -0700
Song Liu <song@kernel.org> wrote:

> On Wed, Oct 21, 2026 at 4:09 PM <sashiko-bot@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> > - [Critical] x86/paravirt: removing notrace from inline instruments pv_ops and resume
> > - [High] tracing: removing notrace from inline breaks ftrace recursion protection
> > - [High] rcu: srcu_read_lock_fast_notrace() and tracepoints lose notrace attribute
> > - [High] bpf: removing notrace from inline exposes trampoline helpers to fentry
> > - [High] entry: removing notrace from inline instruments unvalidated entry helpers  
> 
> Wow. I didn't expect this from Sashiko.

Yeah, I was pretty impressed too.

> 
> > --
> >
> > commit 1eb6626c4a5ab70e5858a8a8f1c9edfca235db57
> > Author: Song Liu <song@kernel.org>
> >
> > compiler_types: Do not mark inline functions notrace
> >
> > This commit removes the notrace attribute from the inline macro definition
> > in include/linux/compiler_types.h so that inline functions emitted
> > out-of-line by the compiler can be traced by ftrace, attached to via BPF
> > fentry/fexit, and livepatched.
> >  
> > > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> > > index c5921f1390079..ea7f2a928b4da 100644
> > > --- a/include/linux/compiler_types.h
> > > +++ b/include/linux/compiler_types.h
> > > @@ -230,9 +230,8 @@ struct ftrace_likely_data {
> > >   * externally visible function. This makes extern inline behave as per gnu89
> > >   * semantics rather than c99. This prevents multiple symbol definition errors
> > >   * of extern inline functions at link time.
> > > - * A lot of inline functions can cause havoc with function tracing.
> > >   */
> > > -#define inline inline __gnu_inline __inline_maybe_unused notrace
> > > +#define inline inline __gnu_inline __inline_maybe_unused
> > >
> > >  /*
> > >   * gcc provides both __inline__ and __inline as alternate spellings of  
> >
> > [Severity: Critical]
> > Could removing notrace from inline cause a triple-fault during x86 suspend
> > resume, secondary CPU bringup, or kexec when function tracing or call-depth
> > tracking is active?
> >
> > In include/linux/compiler_attributes.h, __always_inline expands to inline:
> >
> >     #define __always_inline inline __attribute__((__always_inline__))  
> 
> I wonder whether we need __always_inline to be notrace, and whether
> this resolves any of these concerns.

Yes, we should add "notrace" to the __always_inline for the reasons given
by sashiko. I'll look a the other comments to see if there's other issues.

-- Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] compiler_types: Do not mark inline functions notrace
  2026-10-21 23:09 ` sashiko-bot
  2026-10-21 23:38   ` Song Liu
@ 2026-10-22 14:27   ` Steven Rostedt
  1 sibling, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2026-10-22 14:27 UTC (permalink / raw)
  To: sashiko-bot
  Cc: sashiko-reviews, Song Liu, linux-trace-kernel, live-patching,
	mhiramat, mark.rutland, kernel-team

On Wed, 21 Oct 2026 23:09:21 +0000
sashiko-bot@kernel.org wrote:

> 
> > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> > index c5921f1390079..ea7f2a928b4da 100644
> > --- a/include/linux/compiler_types.h
> > +++ b/include/linux/compiler_types.h
> > @@ -230,9 +230,8 @@ struct ftrace_likely_data {
> >   * externally visible function. This makes extern inline behave as per gnu89
> >   * semantics rather than c99. This prevents multiple symbol definition errors
> >   * of extern inline functions at link time.
> > - * A lot of inline functions can cause havoc with function tracing.
> >   */
> > -#define inline inline __gnu_inline __inline_maybe_unused notrace
> > +#define inline inline __gnu_inline __inline_maybe_unused
> >  
> >  /*
> >   * gcc provides both __inline__ and __inline as alternate spellings of  
> 
> [Severity: Critical]
> Could removing notrace from inline cause a triple-fault during x86 suspend
> resume, secondary CPU bringup, or kexec when function tracing or call-depth
> tracking is active?
> 
> In include/linux/compiler_attributes.h, __always_inline expands to inline:
> 
>     #define __always_inline inline __attribute__((__always_inline__))
> 
> Because __always_inline only inherits notrace through inline, removing
> notrace here strips notrace from both inline and __always_inline functions
> whenever their address is taken and the compiler emits an out-of-line copy.

Correct. If we remove "notrace" from "inline" we must add it to
"__always_inline"

> 
> In arch/x86/kernel/paravirt.c (compiled with CC_FLAGS_FTRACE when
> CONFIG_PARAVIRT_XXL=y and CONFIG_FUNCTION_TRACER=y), pv_ops takes the
> addresses of static inline and static __always_inline native_* helpers
> (including native_load_idt, native_load_gdt, native_load_tr_desc,
> native_set_ldt, native_write_gdt_entry, native_write_idt_entry,
> native_write_ldt_entry, native_load_tls, native_load_gs_index,
> native_load_sp0, native_store_tr, native_cpuid, native_read_cr0,
> native_write_cr0, native_write_cr4, native_read_msr, native_write_msr,
> native_read_msr_safe, native_write_msr_safe, native_read_pmc, and
> native_halt):
> 
>     .cpu.read_cr0       = native_read_cr0,
>     .cpu.write_cr0      = native_write_cr0,
>     .cpu.write_cr4      = native_write_cr4,
>     .cpu.read_msr       = native_read_msr,
>     .cpu.write_msr      = native_write_msr,
>     .cpu.read_msr_safe  = native_read_msr_safe,
>     .cpu.write_msr_safe = native_write_msr_safe,
>     .cpu.read_pmc       = native_read_pmc,
>     .cpu.load_tr_desc   = native_load_tr_desc,
>     .cpu.set_ldt        = native_set_ldt,
>     .cpu.load_gdt       = native_load_gdt,
>     .cpu.load_idt       = native_load_idt,
> 
> This forces the compiler to emit out-of-line bodies in paravirt.o with
> __fentry__ callsites.
> 
> During S3 or hibernation resume, __restore_processor_state() in
> arch/x86/power/cpu.c is marked notrace (rather than noinstr, so objtool does
> not validate it) and calls wrmsrq(), __write_cr4(), write_cr3(),
> write_cr0(), and load_idt(&ctxt->idt) -> pv_ops.cpu.load_idt ->
> native_load_idt() before MSR_GS_BASE (%gs) is restored:

Seems that we need to find these and add "notrace" to each of them individually.

> 
>     static void notrace __restore_processor_state(struct saved_context *ctxt)
>     {
>         ...
>         wrmsrq(MSR_EFER, ctxt->efer);
>         __write_cr4(ctxt->cr4);
>     #endif
>         write_cr3(ctxt->cr3);
>         write_cr2(ctxt->cr2);
>         write_cr0(ctxt->cr0);
> 
>         /* Restore the IDT. */
>         load_idt(&ctxt->idt);
>         ...
>         wrmsrq(MSR_GS_BASE, ctxt->gs_base);
> 
> Similarly, start_secondary() -> cpu_init_exception_handling() in
> arch/x86/kernel/cpu/common.c calls write_gdt_entry(), load_TR_desc(), and
> load_current_idt() -> load_idt() before the secondary CPU IDT is loaded, and
> machine_kexec() in arch/x86/kernel/machine_kexec_64.c calls load_segments()
> (which resets %gs to 0) followed by the static inline virt_to_phys() in
> arch/x86/include/asm/io.h:

Anything that is compiled in common.c via headers is not a concern as the
Makefile has:

  CFLAGS_REMOVE_common.o = -pg

Which removes all fentry calls to common.o.

That doesn't stop fentry being added to functions that
cpu_init_exception_handling() calls that is defined in another C file that
is compiled with -pg.

But for those you mention:

  write_gdt_entry() is defined in headers (safe)
  load_TR_desc() is defined in headers (safe)

  load_current_idt() is itself currently traced. If what it calls is an
  issue then we have the issue now anyway.

The virt_to_phys() could be converted to __always_inline


> 
>     load_segments();
> 
>     /* now call it */
>     image->start = relocate_kernel_ptr((unsigned long)image->head,
>                                        virt_to_phys(control_page),
> 
> Won't executing __fentry__ in these paths dereference %gs per-CPU state
> (such as current->trace_recursion or call-depth tracking) before %gs or the
> IDT is valid?

Probably should test this.

> 
> [Severity: High]
> What prevents infinite ftrace recursion when out-of-line inline helpers are
> called inside ftrace recursion protection itself?
> 
> Ftrace callbacks use ftrace_test_recursion_trylock() ->
> trace_test_and_set_recursion() in include/linux/trace_recursion.h at entry
> and ftrace_test_recursion_unlock() -> trace_clear_recursion() at exit:
> 
>     static __always_inline int trace_test_and_set_recursion(unsigned long ip,
>                                                             unsigned long pip,
>                                                             int start)

Fixed by adding "notrace" to "__always_inline".

>     {
>         unsigned int val = READ_ONCE(current->trace_recursion);
>         int bit;
> 
>         if (trace_warn_on_no_rcu(ip))
>             return -1;
> 
>         bit = trace_get_context_bit() + start;
> 
> In translation units compiled with $(CC_FLAGS_FTRACE) (such as
> kernel/livepatch/patch.c, fs/pstore/ftrace.c,
> arch/x86/kernel/kprobes/ftrace.c, arch/powerpc/kernel/kprobes-ftrace.c,
> kernel/rcu/tree.c, and kernel/trace/trace_events.c under
> CONFIG_FUNCTION_SELF_TRACING=y), several functions called before
> current->trace_recursion is set (or after it is cleared) are plain static
> inline:
> 
> 1. trace_test_and_set_recursion() evaluates current -> get_current() (plain
>    static inline in arch/powerpc/include/asm/current.h and
>    arch/m68k/include/asm/current.h), trace_warn_on_no_rcu() ->
>    rcu_is_watching() (plain static inline in include/linux/rcutiny.h, and
>    calls preempt_disable_notrace() / preempt_enable_notrace() in
>    kernel/rcu/tree.c), and trace_get_context_bit() ->
>    interrupt_context_level() -> preempt_count() (plain static inline in
>    arch/arm64/include/asm/preempt.h).

Perhaps we should be more strict and convert them to __always_inline.

> 
> 2. In arch/arm64/include/asm/preempt.h, preempt_count(),
>    __preempt_count_add(), __preempt_count_sub(), should_resched(), and
>    __preempt_count_dec_and_test() are plain static inline without
>    __always_inline or notrace, so preempt_disable_notrace() and
>    preempt_enable_notrace() can be traced if emitted out-of-line.

Again, __always_inline having notrace will fix this.

> 
> 3. In kernel/trace/trace_events.c (compiled with $(CC_FLAGS_FTRACE) when
>    CONFIG_FUNCTION_SELF_TRACING=y), __init function_test_events_call() is
>    registered without FTRACE_OPS_FL_RECURSION and calls static inline
>    helpers tracing_gen_ctx(), tracing_gen_ctx_flags(),
>    arch_local_save_flags(), preempt_disable_notrace(), and
>    atomic_inc_return() before incrementing ftrace_test_event_disable, as
>    well as atomic_dec() and preempt_enable_notrace() when decrementing it:

All the above do need to be made to explicitly have notrace.

> 
>     trace_ctx = tracing_gen_ctx();
>     preempt_disable_notrace();
>     cpu = raw_smp_processor_id();
>     disabled = atomic_inc_return(&per_cpu(ftrace_test_event_disable, cpu));
> 
> If the compiler emits any of these helpers out-of-line, won't tracing them
> recurse infinitely before the recursion guard takes effect?
> 
> [Severity: High]
> Should the _notrace SRCU helpers and tracepoint dispatch wrappers retain an
> explicit notrace or __always_inline annotation?

Probably.

> 
> In include/linux/srcu.h, srcu_read_lock_fast_notrace() and
> srcu_read_unlock_fast_notrace() are documented as non-traceable, but unlike
> srcu_read_lock_notrace() and srcu_read_unlock_notrace() (which are marked
> static inline notrace), they rely on inline for notrace:
> 
>     /*
>      * Used by tracing, cannot be traced and cannot call lockdep.
>      * See srcu_read_lock_fast() for more information.
>      */
>     static inline struct srcu_ctr __percpu *srcu_read_lock_fast_notrace(struct srcu_struct *ssp)
> 
> Their callee srcu_check_read_flavor() in include/linux/srcutree.h and the
> CONFIG_TINY_SRCU helpers in include/linux/srcutiny.h are also plain static
> inline.
> 
> Similarly, in include/linux/tracepoint.h, __do_trace_##name (which uses
> guard(srcu_fast_notrace)(&tracepoint_srcu) and is called on the cold
> static_branch_unlikely() path as well as from notrace void
> rust_do_trace_##name), trace_##name, and trace_call__##name are plain static
> inline:
> 
>     static inline void __do_trace_##name(proto)                     \

This probably should be converted to __always_inline


>     {                                                               \
>         TRACEPOINT_CHECK(name)                                      \
>         if (cond) {                                                 \
>             guard(srcu_fast_notrace)(&tracepoint_srcu);             \
>             __DO_TRACE_CALL(name, TP_ARGS(args));                   \
>         }                                                           \
>     }
> 
> Because __do_trace_##name is on the cold branch of trace_##name, compilers
> frequently emit it out-of-line. Without notrace on inline, won't these
> tracepoint and _notrace SRCU helpers become ftrace and fentry targets?
> 
> [Severity: High]
> Can attaching a BPF fentry or fexit program to an uninlined helper in
> kernel/bpf/trampoline.c or kernel/bpf/memalloc.c trigger infinite recursion
> or llist corruption?

Hmm, perhaps we need to add to kernel/bpf/Makefile:

  CFLAGS_REMOVE_trampoline.o = $(CC_FLAGS_FTRACE)


> 
> Unlike ftrace_ops callbacks, BPF trampolines do not use
> ftrace_test_recursion_trylock(). In kernel/bpf/trampoline.c (compiled with
> $(CC_FLAGS_FTRACE)), notrace __bpf_prog_enter*(), __bpf_prog_exit*(),
> __bpf_tramp_enter(), and __bpf_tramp_exit() call plain static inline helpers
> before prog->active recursion protection is acquired or after it is
> released:
> 
>     static u64 notrace __bpf_prog_enter_recur(struct bpf_prog *prog, struct bpf_tramp_run_ctx *run_ctx)
>         __acquires(RCU)
>     {
>         rcu_read_lock_dont_migrate();
> 
>         run_ctx->saved_run_ctx = bpf_set_run_ctx(&run_ctx->run_ctx);
> 
>         if (unlikely(!bpf_prog_get_recursion_context(prog))) {
> 
> and:
> 
>     static void notrace __bpf_prog_exit_recur(struct bpf_prog *prog, u64 start,
>                                               struct bpf_tramp_run_ctx *run_ctx)
>         __releases(RCU)
>     {
>         bpf_reset_run_ctx(run_ctx->saved_run_ctx);
> 
>         update_prog_stats(prog, start);
>         bpf_prog_put_recursion_context(prog);
>         rcu_read_unlock_migrate();
>     }
> 
> The static inline helpers called here include __migrate_disable() and
> __migrate_enable() (include/linux/sched.h), rcu_read_unlock_migrate(),
> rcu_read_lock(), and rcu_read_unlock() (include/linux/rcupdate.h),
> rcu_read_lock_trace() and rcu_read_unlock_trace()

Yeah, this patch will likely need to turn into a series where we have to
convert several "inline" to "__always_inline" to prevent from tracing. Or
add explicit "notrace".

> (include/linux/rcupdate_trace.h), bpf_set_run_ctx(), bpf_reset_run_ctx(),
> bpf_prog_get_recursion_context(), and bpf_prog_put_recursion_context()
> (include/linux/bpf.h), percpu_ref_get() and percpu_ref_put()
> (include/linux/percpu-refcount.h), and u64_stats_update_begin_irqsave() and
> u64_stats_update_end_irqrestore() (include/linux/u64_stats_sync.h). In
> kernel/bpf/verifier.c, btf_id_deny only blocks the out-of-line wrappers
> migrate_disable, migrate_enable, ___migrate_enable, rcu_read_unlock_strict,
> preempt_count_add, preempt_count_sub, __rcu_read_lock, and
> __rcu_read_unlock, not these static inline functions.
> 
> In addition, unit_alloc(), unit_free(), and unit_free_rcu() in
> kernel/bpf/memalloc.c are marked notrace specifically so BPF programs cannot
> attach to them and corrupt free_llist or free_llist_extra, yet they call
> plain static inline helpers __llist_add(), __llist_add_batch(), llist_add(),
> llist_add_batch(), local_inc_return(), local_add_return(), and local_dec():
> 
>     static void notrace unit_free(struct bpf_mem_cache *c, void *ptr)
>     {
>         ...
>         local_irq_save(flags);
>         if (local_inc_return(&c->active) == 1) {
>             __llist_add(llnode, &c->free_llist);
>             cnt = ++c->free_cnt;
>         } else {
>             llist_add(llnode, &c->free_llist_extra);
>         }
>         local_dec(&c->active);
> 
> If any of these static inline helpers is emitted out-of-line and attached to
> via fentry/fexit, won't the trampoline recurse infinitely or corrupt the
> bpf_mem_alloc freelist?
> 
> [Severity: High]
> How are entry, exit, and NMI inline helpers protected against ftrace
> instrumentation on architectures without objtool noinstr validation?
> 
> CONFIG_HAVE_NOINSTR_VALIDATION is only selected on X86_64 (in
> arch/x86/Kconfig, when CONFIG_DEBUG_ENTRY=y); powerpc, arm64, riscv, s390,
> loongarch, and x86_32 do not have build-time objtool noinstr validation:
> 
> 1. On PowerPC, arch/powerpc/include/asm/interrupt.h defines
>    interrupt_handler with notrace rather than noinstr because of linker
>    branch stub range limitations:
> 
>     #define interrupt_handler __visible noinline notrace __no_kcsan __no_sanitize_address
> 
>    The notrace interrupt, NMI, and syscall entry/exit paths in
>    arch/powerpc/kernel/traps.c and arch/powerpc/kernel/interrupt.c call
>    large static inline helpers in arch/powerpc/include/asm/entry-common.h,
>    arch/powerpc/include/asm/ftrace.h, and arch/powerpc/include/asm/hw_irq.h,
>    including check_return_regs_valid(), arch_interrupt_enter_prepare(),
>    arch_interrupt_exit_prepare(), arch_interrupt_nmi_enter_prepare(),
>    arch_interrupt_nmi_exit_prepare(), this_cpu_disable_ftrace(),
>    this_cpu_enable_ftrace(), this_cpu_set_ftrace_enabled(),
>    this_cpu_get_ftrace_enabled(), and (from prep_irq_for_enabled_exit() in
>    arch/powerpc/kernel/interrupt.c) exit_must_hard_disable(),
>    __hard_EE_RI_disable(), __hard_RI_enable(), __hard_irq_disable(),
>    lazy_irq_pending_nocheck(), arch_local_irq_disable(), and
>    arch_local_irq_save(). These run while MSR[RI]=0 (unrecoverable exception
>    state), in real mode on Book3S-64 NMI entry, before
>    this_cpu_set_ftrace_enabled(0) disables ftrace in NMI, with unreconciled
>    irq_soft_mask, or after user_enter_irqoff() while RCU is not watching.
> 
> 2. On arm64, arch/arm64/kernel/entry-common.c is compiled with
>    $(CC_FLAGS_FTRACE), and its noinstr handlers
>    (arm64_enter_from_kernel_mode(), __arm64_exit_to_kernel_mode(),
>    arm64_exit_to_kernel_mode_preempt(), el0_pc(), and __sdei_handler()) call
>    plain static inline helpers outside instrumentation_begin() /
>    instrumentation_end(): irqentry_exit_to_kernel_mode_preempt()
>    (include/linux/irq-entry-common.h), __kern_my_cpu_offset()
>    (arch/arm64/include/asm/percpu.h), mte_check_tfsr_entry(),
>    mte_check_tfsr_exit(), and mte_disable_tco_entry()
>    (arch/arm64/include/asm/mte.h), sme_enter_from_user_mode() and
>    sme_exit_to_user_mode() (arch/arm64/include/asm/fpsimd.h),
>    instruction_pointer() (arch/arm64/include/asm/ptrace.h), and
>    system_supports_mte(), system_has_prio_mask_debugging(),
>    system_uses_hw_pan(), and cpu_has_pan()
>    (arch/arm64/include/asm/cpufeature.h).

These will need to be looked through.

> 
> 3. In generic entry code (include/linux/irq-entry-common.h),
>    enter_from_user_mode() and exit_to_user_mode() call user_exit_irqoff()
>    and user_enter_irqoff() (include/linux/context_tracking.h, which are
>    plain static inline when !CONFIG_CONTEXT_TRACKING_USER) and
>    lockdep_hardirqs_off() and lockdep_hardirqs_on()
>    (include/linux/irqflags.h, which are plain static inline when
>    !CONFIG_PROVE_LOCKING) outside instrumentation_begin() /
>    instrumentation_end().
> 
> Won't an out-of-line copy of these helpers be instrumented by ftrace and
> execute when MSR[RI]=0, in real mode, or while RCU is not watching?

I think most of those are stubs. I would hope a compiler would never
out-of-line them.

-- Steve

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-22 14:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-21 22:22 [PATCH] compiler_types: Do not mark inline functions notrace Song Liu
2026-10-21 23:09 ` sashiko-bot
2026-10-21 23:38   ` Song Liu
2026-10-22 13:25     ` Steven Rostedt
2026-10-22 14:27   ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox