Message-Id: <20241105181752.74a3d6fa2f06d0adfdf85322@kernel.org>
Date: Tue, 5 Nov 2024 18:17:51 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Segher Boessenkool <segher@...nel.crashing.org>
Cc: Hari Bathini <hbathini@...ux.ibm.com>, Shuah Khan <shuah@...nel.org>,
 linux-kselftest@...r.kernel.org, Steven Rostedt <rostedt@...dmis.org>,
 Masami Hiramatsu <mhiramat@...nel.org>, Michael Ellerman
 <mpe@...erman.id.au>, Madhavan Srinivasan <maddy@...ux.ibm.com>,
 "Naveen N. Rao" <naveen@...nel.org>, lkml <linux-kernel@...r.kernel.org>,
 linux-trace-kernel@...r.kernel.org, linuxppc-dev
 <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH] selftests/ftrace: update kprobe syntax error test for
 ppc64le

Hi,

On Tue, 5 Nov 2024 02:20:18 -0600
Segher Boessenkool <segher@...nel.crashing.org> wrote:

> Hi!
> 
> On Mon, Nov 04, 2024 at 11:06:23PM +0530, Hari Bathini wrote:
> > Seems like a bit of misunderstanding there. Function entry here intends
> > to mean the actual start of function code (function prologue) - after
> > GEP and function profiling sequence (mflr r0; bl mcount).
> 
> What you call "function entry" here simply does not exist.  The compiler
> can -- and ***WILL***, ***DOES*** -- mix up all of that.

Here is the "function entry" means the function address.
Not the prologue. On some architecture, we are sure fixed sequences
right after the function address for ftrace/security. For example,
x86 has an `ENDBR` for security. Thus, even if we tend to put a
probe on the "function entry", kprobes shifts the probe point
forcibly skipping the `ENDBR`. So from the probe callback, the
probed address does not look like the function address (shift
the sizeof(ENDBR)).

However, the ENDBR does nothing from the program point of view, we
can still think of that address as the address of the function.
That is the reason why I introduced arch_kprobe_on_func_entry().

For the other architecture, it might be misunderstood and
could be miss-implemented. In that case, we should fix that.

>  In particular,
> "function prologue" does not exist at all (on any architecture worth
> its salt, including PowerPC), and all instructions you consider part of
> a function prologue might end up anywhere.  The "profiling sequence" is
> part of that btw, and that typically ends up *not* the first thing in
> the function, not the first thing after the LEP (register saves are
> earlier often, they are generated in that order in the first place,
> but they can (and will) be moved if that schedules better).
> 
> > Function arguments can be accessed with kprobe only while setting a
> > probe at an address the kernel treats as function start address.
> 
> That is a silly assumption to make.  There is no guarantee you can
> access function arguments *at all*, we're not in 1975 anymore.  You
> *need* to look at debug information if you want to deal with anything
> about your high-level language program.  Looking at the machine code
> can only tell you about the machine state, whatever is in registers
> etc.

Yeah, understood. So the `$arg*` here does not guarantee to access
arguments, but the best effort to do that. And it fully depends on
regs_get_kernel_argument(). Thus `$arg*` works only where the
regs_get_kernel_argument() can return most likely function argument
value from `pt_regs`. That is where we call "function entry" in
this context.

And since it checks the function entry by arch_kprobe_on_func_entry()
this test fails on powerpc because it returns true if the offset from
the kallsyms symbol address is less than 8/16 bytes.


> 
> > Note that the test case pass criteria here is setting probe to fail by
> > providing an address (sym+offset) beyond the function start address.
> > 
> > And in this specific test case (with "vfs_read+8", where vfs_read is
> > the symbol and '8' is the offset), the test case was failing on powerpc
> > because setting the probe at 'sym+8' was succeeding, as anywhere between
> > 'sym' to 'sym+16' is treated as function start address on powerpc:
> 
> Yeah, fragile tests sometimes break.  Changing a randomly chosen number
> to some other randomly chosen number will not fix the problem (but you
> can postpone having to deal with it, sure!)

Yeah, sorry about the test case. Actually `+8` is also not a good number
for x86 too since we are not sure whether the address is an instruction
boundary or not. In that case it may report another error and failed.

Thank you,

> 
> 
> Segher


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>

Powered by blists - more mailing lists