Re: [PATCH v8] rethook: add riscv rethook implementation

binglei wang
2022-10-17T14:31:45Z

Hi Conor,
could you please help me to push forward merging process of this patch
into the 6.1 ? Thank you!

Binglei Wang
Best wishes.

On 2022-10-03 00:39, Conor Dooley wrote:
> Hey Binglei,
> I am not qualified to give you an actual R-b on this patch, but I see
> you did get one from Masami who very much is!
>
> That said, the patch looks a lot better (and much simpler!) now. Thanks
> for sticking with it despite the initial issues with your email setup
> and the submission process.
>
> Thanks,
> Conor.
>
> On Fri, Sep 30, 2022 at 04:13:57PM +0800, Binglei Wang wrote:
>> From: Binglei Wang <l3b2w1@gmail.com>
>>
>> Implement the kretprobes on riscv arch by using rethook machenism
>> which abstracts general kretprobe info into a struct rethook_node
>> to be embedded in the struct kretprobe_instance.
>>
>> Signed-off-by: Binglei Wang <l3b2w1@gmail.com>
>> ---
>>
>> Notes:
>> v8: Add the omitted rethook.h
>> v7: Add the changelog.
>> v6: Remove the kretprobes trampoline.
>> v5: Trt to fix robot compiling error and warnings.
>> v4: Add patch version number.
>> v3: Trt to fix robot compiling error and warnings.
>> v2: Add comit log to explain reasons behind changes.
>> Use my personal email instead of work email
>> to avoid the attachments of company informaton.
>> Make the kprobes_trampoline.S code to be shared.
>> v1: Add riscv rethook implementation.
>>
>> arch/riscv/Kconfig | 1 +
>> arch/riscv/include/asm/kprobes.h | 2 --
>> arch/riscv/kernel/probes/Makefile | 2 +-
>> arch/riscv/kernel/probes/kprobes.c | 13 ---------
>> arch/riscv/kernel/probes/rethook.c | 27 +++++++++++++++++++
>> arch/riscv/kernel/probes/rethook.h | 8 ++++++
>> ...obes_trampoline.S => rethook_trampoline.S} | 6 ++---
>> 7 files changed, 40 insertions(+), 19 deletions(-)
>> create mode 100644 arch/riscv/kernel/probes/rethook.c
>> create mode 100644 arch/riscv/kernel/probes/rethook.h
>> rename arch/riscv/kernel/probes/{kprobes_trampoline.S => rethook_trampoline.S} (94%)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 59d18881f..bfb66cdc5 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -97,6 +97,7 @@ config RISCV
>> select HAVE_KPROBES if !XIP_KERNEL
>> select HAVE_KPROBES_ON_FTRACE if !XIP_KERNEL
>> select HAVE_KRETPROBES if !XIP_KERNEL
>> + select HAVE_RETHOOK if !XIP_KERNEL
>> select HAVE_MOVE_PMD
>> select HAVE_MOVE_PUD
>> select HAVE_PCI
>> diff --git a/arch/riscv/include/asm/kprobes.h b/arch/riscv/include/asm/kprobes.h
>> index 217ef89f2..e7882ccb0 100644
>> --- a/arch/riscv/include/asm/kprobes.h
>> +++ b/arch/riscv/include/asm/kprobes.h
>> @@ -40,8 +40,6 @@ void arch_remove_kprobe(struct kprobe *p);
>> int kprobe_fault_handler(struct pt_regs *regs, unsigned int trapnr);
>> bool kprobe_breakpoint_handler(struct pt_regs *regs);
>> bool kprobe_single_step_handler(struct pt_regs *regs);
>> -void __kretprobe_trampoline(void);
>> -void __kprobes *trampoline_probe_handler(struct pt_regs *regs);
>>
>> #endif /* CONFIG_KPROBES */
>> #endif /* _ASM_RISCV_KPROBES_H */
>> diff --git a/arch/riscv/kernel/probes/Makefile b/arch/riscv/kernel/probes/Makefile
>> index 7f0840dcc..c40139e9c 100644
>> --- a/arch/riscv/kernel/probes/Makefile
>> +++ b/arch/riscv/kernel/probes/Makefile
>> @@ -1,6 +1,6 @@
>> # SPDX-License-Identifier: GPL-2.0
>> obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o simulate-insn.o
>> -obj-$(CONFIG_KPROBES) += kprobes_trampoline.o
>> +obj-$(CONFIG_RETHOOK) += rethook.o rethook_trampoline.o
>> obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
>> obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o simulate-insn.o
>> CFLAGS_REMOVE_simulate-insn.o = $(CC_FLAGS_FTRACE)
>> diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
>> index e6e950b7c..f21592d20 100644
>> --- a/arch/riscv/kernel/probes/kprobes.c
>> +++ b/arch/riscv/kernel/probes/kprobes.c
>> @@ -345,19 +345,6 @@ int __init arch_populate_kprobe_blacklist(void)
>> return ret;
>> }
>>
>> -void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
>> -{
>> - return (void *)kretprobe_trampoline_handler(regs, NULL);
>> -}
>> -
>> -void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
>> - struct pt_regs *regs)
>> -{
>> - ri->ret_addr = (kprobe_opcode_t *)regs->ra;
>> - ri->fp = NULL;
>> - regs->ra = (unsigned long) &__kretprobe_trampoline;
>> -}
>> -
>> int __kprobes arch_trampoline_kprobe(struct kprobe *p)
>> {
>> return 0;
>> diff --git a/arch/riscv/kernel/probes/rethook.c b/arch/riscv/kernel/probes/rethook.c
>> new file mode 100644
>> index 000000000..cbd0da059
>> --- /dev/null
>> +++ b/arch/riscv/kernel/probes/rethook.c
>> @@ -0,0 +1,27 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Generic return hook for riscv.
>> + */
>> +
>> +#include <linux/kprobes.h>
>> +#include <linux/rethook.h>
>> +#include "rethook.h"
>> +
>> +/* This is called from arch_rethook_trampoline() */
>> +unsigned long __used arch_rethook_trampoline_callback(struct pt_regs *regs)
>> +{
>> + return rethook_trampoline_handler(regs, regs->s0);
>> +}
>> +NOKPROBE_SYMBOL(arch_rethook_trampoline_callback);
>> +
>> +
>> +void arch_rethook_prepare(struct rethook_node *rhn, struct pt_regs *regs, bool mcount)
>> +{
>> + rhn->ret_addr = regs->ra;
>> + rhn->frame = regs->s0;
>> +
>> + /* replace return addr with trampoline */
>> + regs->ra = (unsigned long)arch_rethook_trampoline;
>> +}
>> +NOKPROBE_SYMBOL(arch_rethook_prepare);
>> +
>> diff --git a/arch/riscv/kernel/probes/rethook.h b/arch/riscv/kernel/probes/rethook.h
>> new file mode 100644
>> index 000000000..cc573d701
>> --- /dev/null
>> +++ b/arch/riscv/kernel/probes/rethook.h
>> @@ -0,0 +1,8 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +#ifndef __RISCV_RETHOOK_H
>> +#define __RISCV_RETHOOK_H
>> +
>> +unsigned long arch_rethook_trampoline_callback(struct pt_regs *regs);
>> +void arch_rethook_prepare(struct rethook_node *rhn, struct pt_regs *regs, bool mcount);
>> +
>> +#endif
>> diff --git a/arch/riscv/kernel/probes/kprobes_trampoline.S b/arch/riscv/kernel/probes/rethook_trampoline.S
>> similarity index 94%
>> rename from arch/riscv/kernel/probes/kprobes_trampoline.S
>> rename to arch/riscv/kernel/probes/rethook_trampoline.S
>> index 7bdb09ded..21bac92a1 100644
>> --- a/arch/riscv/kernel/probes/kprobes_trampoline.S
>> +++ b/arch/riscv/kernel/probes/rethook_trampoline.S
>> @@ -75,13 +75,13 @@
>> REG_L x31, PT_T6(sp)
>> .endm
>>
>> -ENTRY(__kretprobe_trampoline)
>> +ENTRY(arch_rethook_trampoline)
>> addi sp, sp, -(PT_SIZE_ON_STACK)
>> save_all_base_regs
>>
>> move a0, sp /* pt_regs */
>>
>> - call trampoline_probe_handler
>> + call arch_rethook_trampoline_callback
>>
>> /* use the result as the return-address */
>> move ra, a0
>> @@ -90,4 +90,4 @@ ENTRY(__kretprobe_trampoline)
>> addi sp, sp, PT_SIZE_ON_STACK
>>
>> ret
>> -ENDPROC(__kretprobe_trampoline)
>> +ENDPROC(arch_rethook_trampoline)
>> --
>> 2.27.0
>>