[PATCH v2] PowerPC: Replace kretprobe with rethook

[Naveen N Rao]

On Tue, Jun 18, 2024 at 06:43:06AM GMT, Masami Hiramatsu wrote:
> On Mon, 17 Jun 2024 18:28:07 +0530
> Naveen N Rao <naveen at kernel.org> wrote:
> 
> > Hi Abhishek,
> > 
> > On Mon, Jun 10, 2024 at 11:45:09AM GMT, Abhishek Dubey wrote:
> > > This is an adaptation of commit f3a112c0c40d ("x86,rethook,kprobes:
> > > Replace kretprobe with rethook on x86") to PowerPC.
> > > 
> > > Replaces the kretprobe code with rethook on Power. With this patch,
> > > kretprobe on Power uses the rethook instead of kretprobe specific
> > > trampoline code.
> > > 
> > > Reference to other archs:
> > > commit b57c2f124098 ("riscv: add riscv rethook implementation")
> > > commit 7b0a096436c2 ("LoongArch: Replace kretprobe with rethook")
> > > 
> > > Signed-off-by: Abhishek Dubey <adubey at linux.ibm.com>
> > > ---
> > >  arch/powerpc/Kconfig             |  1 +
> > >  arch/powerpc/kernel/Makefile     |  1 +
> > >  arch/powerpc/kernel/kprobes.c    | 65 +----------------------------
> > >  arch/powerpc/kernel/optprobes.c  |  2 +-
> > >  arch/powerpc/kernel/rethook.c    | 71 ++++++++++++++++++++++++++++++++
> > >  arch/powerpc/kernel/stacktrace.c | 10 +++--
> > >  6 files changed, 81 insertions(+), 69 deletions(-)
> > >  create mode 100644 arch/powerpc/kernel/rethook.c
...
> > > +
> > > +	return 0;
> > > +}
> > > +NOKPROBE_SYMBOL(trampoline_rethook_handler);
> > > +
> > > +void arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mcount)
> > > +{
> > > +	rh->ret_addr = regs->link;
> > > +	rh->frame = 0;
> > 
> > There is additional code to validate our assumption with a frame pointer 
> > set, so I think we should set this to regs->gpr[1].
> 
> Additonal note: If this sets regs->gpr[1], pass it to rethook_trampoline_handler()
> too, so that it can find correct frame.
> 
> BTW, it seems powerpc does not use kretprobe/rethook shadow stack for
> stack unwinding yet, is that right?

Yes, you are right. That would be a good addition. I suppose we could 
add something in show_stack() to show the actual function name rather 
than the rethook trampoline. It can be a separate patch though.


Thanks,
Naveen