Re: [RFC PATCH 03/14] arm: kprobes: Use generic kretprobe trampoline handler

[Masami Hiramatsu <mhiramat@kernel.org>]

On Wed, 26 Aug 2020 16:10:25 +0200
peterz@infradead.org wrote:

> On Wed, Aug 26, 2020 at 04:08:52PM +0200, peterz@infradead.org wrote:
> > On Wed, Aug 26, 2020 at 10:46:43PM +0900, Masami Hiramatsu wrote:
> > >  static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
> > >  {
> > > +	return (void *)kretprobe_trampoline_handler(regs,
> > > +				(unsigned long)&kretprobe_trampoline,
> > > +				regs->ARM_fp);
> > >  }
> > 
> > Does it make sense to have the generic code have a weak
> > trampoline_handler() implemented like the above? It looks like a number
> > of architectures have this trivial variant and it seems pointless to
> > duplicate this.
> 
> Argh, I replied to the wrong variant, I mean the one that uses
> kernel_stack_pointer(regs).

Would you mean using kernel_stack_pointer() for the frame_pointer?
Some arch will be OK, but others can not get the framepointer by that.
(that is because the stack layout is different on the function prologue
and returned address, e.g. x86...)

> 
> Then the architecture only needs to implement kernel_stack_pointer() if
> there is nothing else to do.

There are 2 patterns of kretprobe trampoline handling, one is using
a kprobe which hooks the trampoline code. In this case, the
trampoline handler is a kprobe pre_handler. And another is not
using kprobe, but trampoline code saves (a part of)pt_regs and call
the trampoline handler. In this case the trampoline handler will get
the (maybe incomplete) pt_regs. Actually, arm kretprobe handler doesn't
save the sp register for some reason...

Thank you,
-- 
Masami Hiramatsu <mhiramat@kernel.org>