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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241213172947.GJ3387508@ZenIV>
Date: Fri, 13 Dec 2024 17:29:47 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: Michal Simek <monstr@...str.eu>
Cc: Steven Rostedt <rostedt@...dmis.org>, 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, 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;
}