linux-kernel - Re: [for-linus][PATCH 3/3] ftrace/microblaze: Do not find "true

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241213152004.1e74ca81@gandalf.local.home>
Date: Fri, 13 Dec 2024 15:20:04 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: Michal Simek <monstr@...str.eu>, linux-kernel@...r.kernel.org, Masami
 Hiramatsu <mhiramat@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Andrew Morton
 <akpm@...ux-foundation.org>
Subject: Re: [for-linus][PATCH 3/3] ftrace/microblaze: Do not find
 "true_parent" for return address

On Fri, 13 Dec 2024 17:29:47 +0000
Al Viro <viro@...iv.linux.org.uk> wrote:

> On Fri, Dec 13, 2024 at 04:39:29PM +0100, Michal Simek wrote:
> 
> > > diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> > > index 74c353164ca1..a75d107a45f8 100644
> > > --- a/kernel/trace/trace_functions.c
> > > +++ b/kernel/trace/trace_functions.c
> > > @@ -176,7 +176,8 @@ static void function_trace_start(struct trace_array *tr)
> > >   	tracing_reset_online_cpus(&tr->array_buffer);
> > >   }
> > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > > +/* Microblaze currently doesn't implement kernel_stack_pointer() */  
> > 
> > Does it mean that this function should depends on
> > ARCH_HAS_CURRENT_STACK_POINTER instead of name the architecture?  
> 
> Nope.  ARCH_HAS_CURRENT_STACK_POINTER == "there's a current_stack_pointer variable"
> (presumably something like register unsigned long current_stack_pointer asm("r1");
> in case of microblaze).  kernel_stack_pointer() is "here's pt_regs, give me the
> kernel stack pointer stored in it (assuming it _is_ stored there)".
> 
> And what ftrace code really want is "here's the structure formed by _mcount();
> give me the kernel stack pointer at the time of _mcount() entry".  _IF_ that
> structure is pt_regs (fairly common) and if there's kernel_stack_pointer(),
> we get the default implementation of that helper in linux/ftrace_regs.h:
> 
> #define ftrace_regs_get_stack_pointer(fregs) \
>         kernel_stack_pointer(&arch_ftrace_regs(fregs)->regs)
> 
> If it's not pt_regs, you are expected to define HAVE_ARCH_FTRACE_REGS, define
> struct __arch_ftrace_regs to match whatever layout you are using and provide
> the set of ftrace_regs_...() helpers.
> 
> >From my reading of your mcount.S, the layout is, indeed, different and  
> r1 is not stored there at all - something like
> 
> struct __arch_ftrace_regs {
> 	unsigned long r2, r3, r4, r6;
> 	unsigned long r7, r8, r9, r10;
> 	unsigned long r11, r12, r13, r14;
> 	unsigned long r16, r17, r18, r19;
> 	unsigned long r20, r21, r22, r23;
> 	unsigned long r24, r25, r26, r27;
> 	unsigned long r28, r29, r30, r31;
> 	unsigned long r5;
> }
> 
> static inline unsigned long ftrace_regs_get_stack_pointer(struct ftrace_regs *regs)
> {
> 	return (unsigned long)regs + sizeof(struct __arch_ftrace_regs) + 4;
> }

OK, so this is still unique for Microblaze. I'll keep the patch, but fix
the typo in the change log.

Michal,

Any objections?

-- Steve