On Mon, Nov 03, 2025 at 11:47:37PM +0000, [email protected] wrote: > > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h > > index 93156ac4f..b08c95872 100644 > > --- a/arch/x86/include/asm/ftrace.h > > +++ b/arch/x86/include/asm/ftrace.h > > @@ -56,6 +56,11 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs) > > return &arch_ftrace_regs(fregs)->regs; > > } > > > > +#define arch_ftrace_partial_regs(regs) do { \ > > + regs->flags &= ~X86_EFLAGS_FIXED; \ > > + regs->cs = __KERNEL_CS; \ > > +} while (0) > > + > > Can arch_ftrace_partial_regs read uninitialized regs->flags? > > In return_to_handler, the ftrace_regs structure is allocated on the > stack but only a few fields are initialized (RAX, RDX, RBP, RSP). > The EFLAGS/flags field is never written before > arch_ftrace_partial_regs reads it: > > arch/x86/kernel/ftrace_64.S return_to_handler: > subq $(FRAME_SIZE), %rsp # Allocate ftrace_regs on stack > movq %rax, RAX(%rsp) > movq %rdx, RDX(%rsp) > movq %rbp, RBP(%rsp) > movq %rsp, RSP(%rsp) > # No initialization of EFLAGS field > > Then ftrace_partial_regs()->arch_ftrace_partial_regs() executes: > regs->flags &= ~X86_EFLAGS_FIXED; > > This reads the uninitialized flags field. Stack allocations contain > garbage, so regs->flags will have whatever data was previously on > the stack. The &= operation produces undefined results when operating > on uninitialized memory. > > For comparison, ftrace_regs_caller explicitly initializes EFLAGS: > movq MCOUNT_REG_SIZE(%rsp), %rcx > movq %rcx, EFLAGS(%rsp) > > Should return_to_handler initialize regs->flags to 0 (or another > appropriate value) before arch_ftrace_partial_regs modifies it?
yes, that was in the initial version [1] but Stephen did not like that, so I moved the flags init to later where it needs to take into account other callers (that have meaningful flags) and just zeros the X86_EFLAGS_FIXED, so we go through !perf_hw_regs leg in perf_callchain_kernel jirka [1] https://lore.kernel.org/bpf/aObSyt3qOnS_BMcy@krava/
