2022-09-22 04:20:05

by Binglei Wang

[permalink] [raw]
Subject: [PATCH] rethook: add riscv rethook implementation.

From: Binglei Wang <[email protected]>

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 <[email protected]>
---
arch/riscv/Kconfig | 1 +
arch/riscv/kernel/probes/Makefile | 1 +
arch/riscv/kernel/probes/kprobes.c | 8 --
arch/riscv/kernel/probes/kprobes_trampoline.S | 73 +----------------
arch/riscv/kernel/probes/kprobes_trampoline.h | 79 +++++++++++++++++++
arch/riscv/kernel/probes/rethook.c | 26 ++++++
arch/riscv/kernel/probes/rethook_trampoline.S | 22 ++++++
include/linux/kprobes.h | 18 ++++-
8 files changed, 146 insertions(+), 82 deletions(-)
create mode 100644 arch/riscv/kernel/probes/kprobes_trampoline.h
create mode 100644 arch/riscv/kernel/probes/rethook.c
create mode 100644 arch/riscv/kernel/probes/rethook_trampoline.S

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index ed66c31e4..c5cae0825 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/kernel/probes/Makefile b/arch/riscv/kernel/probes/Makefile
index 7f0840dcc..ee345e7e9 100644
--- a/arch/riscv/kernel/probes/Makefile
+++ b/arch/riscv/kernel/probes/Makefile
@@ -3,4 +3,5 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o simulate-insn.o
obj-$(CONFIG_KPROBES) += kprobes_trampoline.o
obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o simulate-insn.o
+obj-$(CONFIG_RETHOOK) += rethook.o rethook_trampoline.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..04911cc42 100644
--- a/arch/riscv/kernel/probes/kprobes.c
+++ b/arch/riscv/kernel/probes/kprobes.c
@@ -350,14 +350,6 @@ 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/kprobes_trampoline.S b/arch/riscv/kernel/probes/kprobes_trampoline.S
index 7bdb09ded..54937f342 100644
--- a/arch/riscv/kernel/probes/kprobes_trampoline.S
+++ b/arch/riscv/kernel/probes/kprobes_trampoline.S
@@ -2,78 +2,7 @@
/*
* Author: Patrick Stählin <[email protected]>
*/
-#include <linux/linkage.h>
-
-#include <asm/asm.h>
-#include <asm/asm-offsets.h>
-
- .text
- .altmacro
-
- .macro save_all_base_regs
- REG_S x1, PT_RA(sp)
- REG_S x3, PT_GP(sp)
- REG_S x4, PT_TP(sp)
- REG_S x5, PT_T0(sp)
- REG_S x6, PT_T1(sp)
- REG_S x7, PT_T2(sp)
- REG_S x8, PT_S0(sp)
- REG_S x9, PT_S1(sp)
- REG_S x10, PT_A0(sp)
- REG_S x11, PT_A1(sp)
- REG_S x12, PT_A2(sp)
- REG_S x13, PT_A3(sp)
- REG_S x14, PT_A4(sp)
- REG_S x15, PT_A5(sp)
- REG_S x16, PT_A6(sp)
- REG_S x17, PT_A7(sp)
- REG_S x18, PT_S2(sp)
- REG_S x19, PT_S3(sp)
- REG_S x20, PT_S4(sp)
- REG_S x21, PT_S5(sp)
- REG_S x22, PT_S6(sp)
- REG_S x23, PT_S7(sp)
- REG_S x24, PT_S8(sp)
- REG_S x25, PT_S9(sp)
- REG_S x26, PT_S10(sp)
- REG_S x27, PT_S11(sp)
- REG_S x28, PT_T3(sp)
- REG_S x29, PT_T4(sp)
- REG_S x30, PT_T5(sp)
- REG_S x31, PT_T6(sp)
- .endm
-
- .macro restore_all_base_regs
- REG_L x3, PT_GP(sp)
- REG_L x4, PT_TP(sp)
- REG_L x5, PT_T0(sp)
- REG_L x6, PT_T1(sp)
- REG_L x7, PT_T2(sp)
- REG_L x8, PT_S0(sp)
- REG_L x9, PT_S1(sp)
- REG_L x10, PT_A0(sp)
- REG_L x11, PT_A1(sp)
- REG_L x12, PT_A2(sp)
- REG_L x13, PT_A3(sp)
- REG_L x14, PT_A4(sp)
- REG_L x15, PT_A5(sp)
- REG_L x16, PT_A6(sp)
- REG_L x17, PT_A7(sp)
- REG_L x18, PT_S2(sp)
- REG_L x19, PT_S3(sp)
- REG_L x20, PT_S4(sp)
- REG_L x21, PT_S5(sp)
- REG_L x22, PT_S6(sp)
- REG_L x23, PT_S7(sp)
- REG_L x24, PT_S8(sp)
- REG_L x25, PT_S9(sp)
- REG_L x26, PT_S10(sp)
- REG_L x27, PT_S11(sp)
- REG_L x28, PT_T3(sp)
- REG_L x29, PT_T4(sp)
- REG_L x30, PT_T5(sp)
- REG_L x31, PT_T6(sp)
- .endm
+#include "kprobes_trampoline.h"

ENTRY(__kretprobe_trampoline)
addi sp, sp, -(PT_SIZE_ON_STACK)
diff --git a/arch/riscv/kernel/probes/kprobes_trampoline.h b/arch/riscv/kernel/probes/kprobes_trampoline.h
new file mode 100644
index 000000000..48895a5e3
--- /dev/null
+++ b/arch/riscv/kernel/probes/kprobes_trampoline.h
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef _KPROBES_TRAMPOLINE_H
+#define _KPROBES_TRAMPOLINE_H
+/*
+ * Author: Patrick Stählin <[email protected]>
+ */
+#include <linux/linkage.h>
+
+#include <asm/asm.h>
+#include <asm/asm-offsets.h>
+
+ .text
+ .altmacro
+
+ .macro save_all_base_regs
+ REG_S x1, PT_RA(sp)
+ REG_S x3, PT_GP(sp)
+ REG_S x4, PT_TP(sp)
+ REG_S x5, PT_T0(sp)
+ REG_S x6, PT_T1(sp)
+ REG_S x7, PT_T2(sp)
+ REG_S x8, PT_S0(sp)
+ REG_S x9, PT_S1(sp)
+ REG_S x10, PT_A0(sp)
+ REG_S x11, PT_A1(sp)
+ REG_S x12, PT_A2(sp)
+ REG_S x13, PT_A3(sp)
+ REG_S x14, PT_A4(sp)
+ REG_S x15, PT_A5(sp)
+ REG_S x16, PT_A6(sp)
+ REG_S x17, PT_A7(sp)
+ REG_S x18, PT_S2(sp)
+ REG_S x19, PT_S3(sp)
+ REG_S x20, PT_S4(sp)
+ REG_S x21, PT_S5(sp)
+ REG_S x22, PT_S6(sp)
+ REG_S x23, PT_S7(sp)
+ REG_S x24, PT_S8(sp)
+ REG_S x25, PT_S9(sp)
+ REG_S x26, PT_S10(sp)
+ REG_S x27, PT_S11(sp)
+ REG_S x28, PT_T3(sp)
+ REG_S x29, PT_T4(sp)
+ REG_S x30, PT_T5(sp)
+ REG_S x31, PT_T6(sp)
+ .endm
+
+ .macro restore_all_base_regs
+ REG_L x3, PT_GP(sp)
+ REG_L x4, PT_TP(sp)
+ REG_L x5, PT_T0(sp)
+ REG_L x6, PT_T1(sp)
+ REG_L x7, PT_T2(sp)
+ REG_L x8, PT_S0(sp)
+ REG_L x9, PT_S1(sp)
+ REG_L x10, PT_A0(sp)
+ REG_L x11, PT_A1(sp)
+ REG_L x12, PT_A2(sp)
+ REG_L x13, PT_A3(sp)
+ REG_L x14, PT_A4(sp)
+ REG_L x15, PT_A5(sp)
+ REG_L x16, PT_A6(sp)
+ REG_L x17, PT_A7(sp)
+ REG_L x18, PT_S2(sp)
+ REG_L x19, PT_S3(sp)
+ REG_L x20, PT_S4(sp)
+ REG_L x21, PT_S5(sp)
+ REG_L x22, PT_S6(sp)
+ REG_L x23, PT_S7(sp)
+ REG_L x24, PT_S8(sp)
+ REG_L x25, PT_S9(sp)
+ REG_L x26, PT_S10(sp)
+ REG_L x27, PT_S11(sp)
+ REG_L x28, PT_T3(sp)
+ REG_L x29, PT_T4(sp)
+ REG_L x30, PT_T5(sp)
+ REG_L x31, PT_T6(sp)
+ .endm
+#endif
diff --git a/arch/riscv/kernel/probes/rethook.c b/arch/riscv/kernel/probes/rethook.c
new file mode 100644
index 000000000..4a41d5eb6
--- /dev/null
+++ b/arch/riscv/kernel/probes/rethook.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generic return hook for riscv.
+ */
+
+#include <linux/kprobes.h>
+#include <linux/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 = (u64)arch_rethook_trampoline;
+}
+NOKPROBE_SYMBOL(arch_rethook_prepare);
+
diff --git a/arch/riscv/kernel/probes/rethook_trampoline.S b/arch/riscv/kernel/probes/rethook_trampoline.S
new file mode 100644
index 000000000..e81c3d4e0
--- /dev/null
+++ b/arch/riscv/kernel/probes/rethook_trampoline.S
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Author: Patrick Stählin <[email protected]>
+ */
+#include "kprobes_trampoline.h"
+
+ENTRY(arch_rethook_trampoline)
+ addi sp, sp, -(PT_SIZE_ON_STACK)
+ save_all_base_regs
+
+ move a0, sp /* pt_regs */
+
+ call arch_rethook_trampoline_callback
+
+ /* use the result as the return-address */
+ move ra, a0
+
+ restore_all_base_regs
+ addi sp, sp, PT_SIZE_ON_STACK
+
+ ret
+ENDPROC(arch_rethook_trampoline)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 55041d2f8..a21562c11 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -210,9 +210,23 @@ static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_inst
{
return ri->node.ret_addr;
}
+
+static nokprobe_inline
+unsigned long kretprobe_trampoline_handler(struct pt_regs *regs,
+ void *frame_pointer)
+{
+ return 0;
+}
+
#else
-extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
- struct pt_regs *regs);
+static nokprobe_inline void 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;
+}
+
void arch_kretprobe_fixup_return(struct pt_regs *regs,
kprobe_opcode_t *correct_ret_addr);

--
2.27.0


2022-09-22 07:48:53

by Wangbinglei

[permalink] [raw]
Subject: Re: [PATCH] rethook: add riscv rethook implementation.

Thanks for your tips, I resended modified patch using another mail addr([email protected]).

-----?ʼ?ԭ??-----
??????: Conor Dooley [mailto:[email protected]]
????ʱ??: 2022??9??20?? 18:32
?ռ???: wangbinglei (RD) <[email protected]>
????: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
????: Re: [PATCH] rethook: add riscv rethook implementation.

On Tue, Sep 20, 2022 at 05:36:30PM +0800, Binglei Wang wrote:
> From: "wang.binglei" <[email protected]>
>
> Most of the code copied from
> arch/riscv/kernel/probes/kprobes_trampoline.S

Hey Wang Binglei,

Please use the commit log to explain the reasons behind the change you
are making:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes

>
> Signed-off-by: wang.binglei <[email protected]>

Unfortunately I don't know much about Asian naming, but I assume that
the . is not part of your name?

> diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
> index e6e950b7c..2c1847921 100644
> --- a/arch/riscv/kernel/probes/kprobes.c
> +++ b/arch/riscv/kernel/probes/kprobes.c
> @@ -345,6 +345,7 @@ int __init arch_populate_kprobe_blacklist(void)
> return ret;
> }
>
> +#ifndef CONFIG_KRETPROBE_ON_RETHOOK

This seems quite unusual, other archs don't seem to have ifdef-ery
using CONFIG_KRETPROBE_ON_RETHOOK in their arch code so why should
RISC-V?

> void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
> {
> return (void *)kretprobe_trampoline_handler(regs, NULL);
> @@ -357,6 +358,12 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
> ri->fp = NULL;
> regs->ra = (unsigned long) &__kretprobe_trampoline;
> }
> +#else
> +void __kprobes *trampoline_probe_handler(struct pt_regs *regs)
> +{
> + return NULL;
> +}
> +#endif


> diff --git a/arch/riscv/kernel/probes/rethook_trampoline.S b/arch/riscv/kernel/probes/rethook_trampoline.S
> new file mode 100644
> index 000000000..aa79630ac
> --- /dev/null
> +++ b/arch/riscv/kernel/probes/rethook_trampoline.S
> @@ -0,0 +1,94 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * rethook trampoline.
> + * Copied from arch/riscv/kernel/probes/kprobes_trampoline.S

Is this a 1:1 copy? If so, could the code be shared?

> This e-mail and its attachments contain confidential information from New H3C, which is
> intended only for the person or entity whose address is listed above. Any use of the
> information contained herein in any way (including, but not limited to, total or partial
> disclosure, reproduction, or dissemination) by persons other than the intended
> recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender
> by phone or email immediately and delete it!

Uh-oh! You'll have to work with your IT to get this removed before your
patches can be accepted:
https://lore.kernel.org/all/[email protected]/

The patch does not apply to -next for me either..

Thanks,
Conor.

-------------------------------------------------------------------------------------------------------------------------------------
???ʼ????丽???????»??????ŵı?????Ϣ???????ڷ??͸???????ַ???г?
?ĸ??˻?Ⱥ?顣??ֹ?κ??????????κ???ʽʹ?ã?????????????ȫ???????ֵ?й¶?????ơ?
??ɢ???????ʼ??е???Ϣ?????????????˱??ʼ????????????绰???ʼ?֪ͨ?????˲?ɾ????
?ʼ???
This e-mail and its attachments contain confidential information from New H3C, which is
intended only for the person or entity whose address is listed above. Any use of the
information contained herein in any way (including, but not limited to, total or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender
by phone or email immediately and delete it!

2022-09-22 09:20:21

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] rethook: add riscv rethook implementation.

Hi Binglei,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rostedt-trace/for-next]
[also build test ERROR on linus/master v6.0-rc6 next-20220921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Binglei-Wang/rethook-add-riscv-rethook-implementation/20220922-120748
base: https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: arc-defconfig (https://download.01.org/0day-ci/archive/20220922/[email protected]/config)
compiler: arc-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/3f86f610d114317cd7f7a7a1c9116fa0b916667a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Binglei-Wang/rethook-add-riscv-rethook-implementation/20220922-120748
git checkout 3f86f610d114317cd7f7a7a1c9116fa0b916667a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from arch/arc/kernel/kprobes.c:7:
include/linux/kprobes.h: In function 'arch_prepare_kretprobe':
include/linux/kprobes.h:225:49: error: 'struct pt_regs' has no member named 'ra'; did you mean 'r0'?
225 | ri->ret_addr = (kprobe_opcode_t *)regs->ra;
| ^~
| r0
include/linux/kprobes.h:227:15: error: 'struct pt_regs' has no member named 'ra'; did you mean 'r0'?
227 | regs->ra = (unsigned long) &__kretprobe_trampoline;
| ^~
| r0
arch/arc/kernel/kprobes.c: At top level:
arch/arc/kernel/kprobes.c:193:15: warning: no previous prototype for 'arc_kprobe_handler' [-Wmissing-prototypes]
193 | int __kprobes arc_kprobe_handler(unsigned long addr, struct pt_regs *regs)
| ^~~~~~~~~~~~~~~~~~
>> arch/arc/kernel/kprobes.c:371:16: error: redefinition of 'arch_prepare_kretprobe'
371 | void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/kprobes.h:222:29: note: previous definition of 'arch_prepare_kretprobe' with type 'void(struct kretprobe_instance *, struct pt_regs *)'
222 | static nokprobe_inline void arch_prepare_kretprobe(struct kretprobe_instance *ri,
| ^~~~~~~~~~~~~~~~~~~~~~


vim +/arch_prepare_kretprobe +371 arch/arc/kernel/kprobes.c

4d86dfbbda09b3 Vineet Gupta 2013-01-22 370
4d86dfbbda09b3 Vineet Gupta 2013-01-22 @371 void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
4d86dfbbda09b3 Vineet Gupta 2013-01-22 372 struct pt_regs *regs)
4d86dfbbda09b3 Vineet Gupta 2013-01-22 373 {
4d86dfbbda09b3 Vineet Gupta 2013-01-22 374
4d86dfbbda09b3 Vineet Gupta 2013-01-22 375 ri->ret_addr = (kprobe_opcode_t *) regs->blink;
f75dd136b65ccc Masami Hiramatsu 2020-08-29 376 ri->fp = NULL;
4d86dfbbda09b3 Vineet Gupta 2013-01-22 377
4d86dfbbda09b3 Vineet Gupta 2013-01-22 378 /* Replace the return addr with trampoline addr */
adf8a61a940c49 Masami Hiramatsu 2021-09-14 379 regs->blink = (unsigned long)&__kretprobe_trampoline;
4d86dfbbda09b3 Vineet Gupta 2013-01-22 380 }
4d86dfbbda09b3 Vineet Gupta 2013-01-22 381

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-09-22 11:27:01

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] rethook: add riscv rethook implementation.

Hi Binglei,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rostedt-trace/for-next]
[also build test ERROR on linus/master v6.0-rc6 next-20220921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Binglei-Wang/rethook-add-riscv-rethook-implementation/20220922-120748
base: https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20220922/[email protected]/config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/3f86f610d114317cd7f7a7a1c9116fa0b916667a
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Binglei-Wang/rethook-add-riscv-rethook-implementation/20220922-120748
git checkout 3f86f610d114317cd7f7a7a1c9116fa0b916667a
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash arch/s390/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from arch/s390/kernel/kprobes.c:13:
include/linux/kprobes.h: In function 'arch_prepare_kretprobe':
include/linux/kprobes.h:225:47: error: 'struct pt_regs' has no member named 'ra'
225 | ri->ret_addr = (kprobe_opcode_t *)regs->ra;
| ^~
include/linux/kprobes.h:227:13: error: 'struct pt_regs' has no member named 'ra'
227 | regs->ra = (unsigned long) &__kretprobe_trampoline;
| ^~
arch/s390/kernel/kprobes.c: At top level:
>> arch/s390/kernel/kprobes.c:285:6: error: redefinition of 'arch_prepare_kretprobe'
285 | void arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs)
| ^~~~~~~~~~~~~~~~~~~~~~
include/linux/kprobes.h:222:29: note: previous definition of 'arch_prepare_kretprobe' with type 'void(struct kretprobe_instance *, struct pt_regs *)'
222 | static nokprobe_inline void arch_prepare_kretprobe(struct kretprobe_instance *ri,
| ^~~~~~~~~~~~~~~~~~~~~~


vim +/arch_prepare_kretprobe +285 arch/s390/kernel/kprobes.c

4ba069b802c29e Michael Grundy 2006-09-20 284
7a5388de5c70f7 Heiko Carstens 2014-10-22 @285 void arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs)
4ba069b802c29e Michael Grundy 2006-09-20 286 {
4ba069b802c29e Michael Grundy 2006-09-20 287 ri->ret_addr = (kprobe_opcode_t *)regs->gprs[14];
09bc20c8fb35cf Vasily Gorbik 2022-03-05 288 ri->fp = (void *)regs->gprs[15];
4ba069b802c29e Michael Grundy 2006-09-20 289
4ba069b802c29e Michael Grundy 2006-09-20 290 /* Replace the return addr with trampoline addr */
adf8a61a940c49 Masami Hiramatsu 2021-09-14 291 regs->gprs[14] = (unsigned long)&__kretprobe_trampoline;
4ba069b802c29e Michael Grundy 2006-09-20 292 }
7a5388de5c70f7 Heiko Carstens 2014-10-22 293 NOKPROBE_SYMBOL(arch_prepare_kretprobe);
4ba069b802c29e Michael Grundy 2006-09-20 294

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-09-22 11:38:30

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH] rethook: add riscv rethook implementation.

On Thu, Sep 22, 2022 at 07:19:33PM +0800, Binglei Wang wrote:
> From: Binglei Wang <[email protected]>
>
> 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.

Hey Binglei,

This is now version 3 of the patch right? You need to run format-patch
with the -v option so you stop confusing tooling (and humans) as to
what they are reviewing. Also, your changelog from the other versions is
missing.

Please take a look at the docs for how to submit subsequent versions:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

> Signed-off-by: Binglei Wang <[email protected]>

Previously this was your work email - I suspect it still needs to be
your work email here. Your work address can still be the author even if
the mail is not send from that account.

Hope that helps,
Conor.

> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/kernel/probes/Makefile | 1 +
> arch/riscv/kernel/probes/kprobes.c | 8 --
> arch/riscv/kernel/probes/kprobes_trampoline.S | 73 +----------------
> arch/riscv/kernel/probes/kprobes_trampoline.h | 79 +++++++++++++++++++
> arch/riscv/kernel/probes/rethook.c | 26 ++++++
> arch/riscv/kernel/probes/rethook_trampoline.S | 22 ++++++
> include/linux/kprobes.h | 6 ++
> 8 files changed, 136 insertions(+), 80 deletions(-)
> create mode 100644 arch/riscv/kernel/probes/kprobes_trampoline.h
> create mode 100644 arch/riscv/kernel/probes/rethook.c
> create mode 100644 arch/riscv/kernel/probes/rethook_trampoline.S
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index ed66c31e4..c5cae0825 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/kernel/probes/Makefile b/arch/riscv/kernel/probes/Makefile
> index 7f0840dcc..ee345e7e9 100644
> --- a/arch/riscv/kernel/probes/Makefile
> +++ b/arch/riscv/kernel/probes/Makefile
> @@ -3,4 +3,5 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o simulate-insn.o
> obj-$(CONFIG_KPROBES) += kprobes_trampoline.o
> obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
> obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o simulate-insn.o
> +obj-$(CONFIG_RETHOOK) += rethook.o rethook_trampoline.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..04911cc42 100644
> --- a/arch/riscv/kernel/probes/kprobes.c
> +++ b/arch/riscv/kernel/probes/kprobes.c
> @@ -350,14 +350,6 @@ 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/kprobes_trampoline.S b/arch/riscv/kernel/probes/kprobes_trampoline.S
> index 7bdb09ded..54937f342 100644
> --- a/arch/riscv/kernel/probes/kprobes_trampoline.S
> +++ b/arch/riscv/kernel/probes/kprobes_trampoline.S
> @@ -2,78 +2,7 @@
> /*
> * Author: Patrick St?hlin <[email protected]>
> */
> -#include <linux/linkage.h>
> -
> -#include <asm/asm.h>
> -#include <asm/asm-offsets.h>
> -
> - .text
> - .altmacro
> -
> - .macro save_all_base_regs
> - REG_S x1, PT_RA(sp)
> - REG_S x3, PT_GP(sp)
> - REG_S x4, PT_TP(sp)
> - REG_S x5, PT_T0(sp)
> - REG_S x6, PT_T1(sp)
> - REG_S x7, PT_T2(sp)
> - REG_S x8, PT_S0(sp)
> - REG_S x9, PT_S1(sp)
> - REG_S x10, PT_A0(sp)
> - REG_S x11, PT_A1(sp)
> - REG_S x12, PT_A2(sp)
> - REG_S x13, PT_A3(sp)
> - REG_S x14, PT_A4(sp)
> - REG_S x15, PT_A5(sp)
> - REG_S x16, PT_A6(sp)
> - REG_S x17, PT_A7(sp)
> - REG_S x18, PT_S2(sp)
> - REG_S x19, PT_S3(sp)
> - REG_S x20, PT_S4(sp)
> - REG_S x21, PT_S5(sp)
> - REG_S x22, PT_S6(sp)
> - REG_S x23, PT_S7(sp)
> - REG_S x24, PT_S8(sp)
> - REG_S x25, PT_S9(sp)
> - REG_S x26, PT_S10(sp)
> - REG_S x27, PT_S11(sp)
> - REG_S x28, PT_T3(sp)
> - REG_S x29, PT_T4(sp)
> - REG_S x30, PT_T5(sp)
> - REG_S x31, PT_T6(sp)
> - .endm
> -
> - .macro restore_all_base_regs
> - REG_L x3, PT_GP(sp)
> - REG_L x4, PT_TP(sp)
> - REG_L x5, PT_T0(sp)
> - REG_L x6, PT_T1(sp)
> - REG_L x7, PT_T2(sp)
> - REG_L x8, PT_S0(sp)
> - REG_L x9, PT_S1(sp)
> - REG_L x10, PT_A0(sp)
> - REG_L x11, PT_A1(sp)
> - REG_L x12, PT_A2(sp)
> - REG_L x13, PT_A3(sp)
> - REG_L x14, PT_A4(sp)
> - REG_L x15, PT_A5(sp)
> - REG_L x16, PT_A6(sp)
> - REG_L x17, PT_A7(sp)
> - REG_L x18, PT_S2(sp)
> - REG_L x19, PT_S3(sp)
> - REG_L x20, PT_S4(sp)
> - REG_L x21, PT_S5(sp)
> - REG_L x22, PT_S6(sp)
> - REG_L x23, PT_S7(sp)
> - REG_L x24, PT_S8(sp)
> - REG_L x25, PT_S9(sp)
> - REG_L x26, PT_S10(sp)
> - REG_L x27, PT_S11(sp)
> - REG_L x28, PT_T3(sp)
> - REG_L x29, PT_T4(sp)
> - REG_L x30, PT_T5(sp)
> - REG_L x31, PT_T6(sp)
> - .endm
> +#include "kprobes_trampoline.h"
>
> ENTRY(__kretprobe_trampoline)
> addi sp, sp, -(PT_SIZE_ON_STACK)
> diff --git a/arch/riscv/kernel/probes/kprobes_trampoline.h b/arch/riscv/kernel/probes/kprobes_trampoline.h
> new file mode 100644
> index 000000000..48895a5e3
> --- /dev/null
> +++ b/arch/riscv/kernel/probes/kprobes_trampoline.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +#ifndef _KPROBES_TRAMPOLINE_H
> +#define _KPROBES_TRAMPOLINE_H
> +/*
> + * Author: Patrick St?hlin <[email protected]>
> + */
> +#include <linux/linkage.h>
> +
> +#include <asm/asm.h>
> +#include <asm/asm-offsets.h>
> +
> + .text
> + .altmacro
> +
> + .macro save_all_base_regs
> + REG_S x1, PT_RA(sp)
> + REG_S x3, PT_GP(sp)
> + REG_S x4, PT_TP(sp)
> + REG_S x5, PT_T0(sp)
> + REG_S x6, PT_T1(sp)
> + REG_S x7, PT_T2(sp)
> + REG_S x8, PT_S0(sp)
> + REG_S x9, PT_S1(sp)
> + REG_S x10, PT_A0(sp)
> + REG_S x11, PT_A1(sp)
> + REG_S x12, PT_A2(sp)
> + REG_S x13, PT_A3(sp)
> + REG_S x14, PT_A4(sp)
> + REG_S x15, PT_A5(sp)
> + REG_S x16, PT_A6(sp)
> + REG_S x17, PT_A7(sp)
> + REG_S x18, PT_S2(sp)
> + REG_S x19, PT_S3(sp)
> + REG_S x20, PT_S4(sp)
> + REG_S x21, PT_S5(sp)
> + REG_S x22, PT_S6(sp)
> + REG_S x23, PT_S7(sp)
> + REG_S x24, PT_S8(sp)
> + REG_S x25, PT_S9(sp)
> + REG_S x26, PT_S10(sp)
> + REG_S x27, PT_S11(sp)
> + REG_S x28, PT_T3(sp)
> + REG_S x29, PT_T4(sp)
> + REG_S x30, PT_T5(sp)
> + REG_S x31, PT_T6(sp)
> + .endm
> +
> + .macro restore_all_base_regs
> + REG_L x3, PT_GP(sp)
> + REG_L x4, PT_TP(sp)
> + REG_L x5, PT_T0(sp)
> + REG_L x6, PT_T1(sp)
> + REG_L x7, PT_T2(sp)
> + REG_L x8, PT_S0(sp)
> + REG_L x9, PT_S1(sp)
> + REG_L x10, PT_A0(sp)
> + REG_L x11, PT_A1(sp)
> + REG_L x12, PT_A2(sp)
> + REG_L x13, PT_A3(sp)
> + REG_L x14, PT_A4(sp)
> + REG_L x15, PT_A5(sp)
> + REG_L x16, PT_A6(sp)
> + REG_L x17, PT_A7(sp)
> + REG_L x18, PT_S2(sp)
> + REG_L x19, PT_S3(sp)
> + REG_L x20, PT_S4(sp)
> + REG_L x21, PT_S5(sp)
> + REG_L x22, PT_S6(sp)
> + REG_L x23, PT_S7(sp)
> + REG_L x24, PT_S8(sp)
> + REG_L x25, PT_S9(sp)
> + REG_L x26, PT_S10(sp)
> + REG_L x27, PT_S11(sp)
> + REG_L x28, PT_T3(sp)
> + REG_L x29, PT_T4(sp)
> + REG_L x30, PT_T5(sp)
> + REG_L x31, PT_T6(sp)
> + .endm
> +#endif
> diff --git a/arch/riscv/kernel/probes/rethook.c b/arch/riscv/kernel/probes/rethook.c
> new file mode 100644
> index 000000000..4a41d5eb6
> --- /dev/null
> +++ b/arch/riscv/kernel/probes/rethook.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Generic return hook for riscv.
> + */
> +
> +#include <linux/kprobes.h>
> +#include <linux/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 = (u64)arch_rethook_trampoline;
> +}
> +NOKPROBE_SYMBOL(arch_rethook_prepare);
> +
> diff --git a/arch/riscv/kernel/probes/rethook_trampoline.S b/arch/riscv/kernel/probes/rethook_trampoline.S
> new file mode 100644
> index 000000000..e81c3d4e0
> --- /dev/null
> +++ b/arch/riscv/kernel/probes/rethook_trampoline.S
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Author: Patrick St?hlin <[email protected]>
> + */
> +#include "kprobes_trampoline.h"
> +
> +ENTRY(arch_rethook_trampoline)
> + addi sp, sp, -(PT_SIZE_ON_STACK)
> + save_all_base_regs
> +
> + move a0, sp /* pt_regs */
> +
> + call arch_rethook_trampoline_callback
> +
> + /* use the result as the return-address */
> + move ra, a0
> +
> + restore_all_base_regs
> + addi sp, sp, PT_SIZE_ON_STACK
> +
> + ret
> +ENDPROC(arch_rethook_trampoline)
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 55041d2f8..a3805b5b2 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -210,6 +210,12 @@ static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_inst
> {
> return ri->node.ret_addr;
> }
> +static nokprobe_inline
> +unsigned long kretprobe_trampoline_handler(struct pt_regs *regs,
> + void *frame_pointer)
> +{
> + return 0;
> +}
> #else
> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> struct pt_regs *regs);
> --
> 2.27.0
>
>

2022-09-22 12:27:02

by Binglei Wang

[permalink] [raw]
Subject: [PATCH] rethook: add riscv rethook implementation.

From: Binglei Wang <[email protected]>

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 <[email protected]>
---
arch/riscv/Kconfig | 1 +
arch/riscv/kernel/probes/Makefile | 1 +
arch/riscv/kernel/probes/kprobes.c | 8 --
arch/riscv/kernel/probes/kprobes_trampoline.S | 73 +----------------
arch/riscv/kernel/probes/kprobes_trampoline.h | 79 +++++++++++++++++++
arch/riscv/kernel/probes/rethook.c | 26 ++++++
arch/riscv/kernel/probes/rethook_trampoline.S | 22 ++++++
include/linux/kprobes.h | 6 ++
8 files changed, 136 insertions(+), 80 deletions(-)
create mode 100644 arch/riscv/kernel/probes/kprobes_trampoline.h
create mode 100644 arch/riscv/kernel/probes/rethook.c
create mode 100644 arch/riscv/kernel/probes/rethook_trampoline.S

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index ed66c31e4..c5cae0825 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/kernel/probes/Makefile b/arch/riscv/kernel/probes/Makefile
index 7f0840dcc..ee345e7e9 100644
--- a/arch/riscv/kernel/probes/Makefile
+++ b/arch/riscv/kernel/probes/Makefile
@@ -3,4 +3,5 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o simulate-insn.o
obj-$(CONFIG_KPROBES) += kprobes_trampoline.o
obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o simulate-insn.o
+obj-$(CONFIG_RETHOOK) += rethook.o rethook_trampoline.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..04911cc42 100644
--- a/arch/riscv/kernel/probes/kprobes.c
+++ b/arch/riscv/kernel/probes/kprobes.c
@@ -350,14 +350,6 @@ 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/kprobes_trampoline.S b/arch/riscv/kernel/probes/kprobes_trampoline.S
index 7bdb09ded..54937f342 100644
--- a/arch/riscv/kernel/probes/kprobes_trampoline.S
+++ b/arch/riscv/kernel/probes/kprobes_trampoline.S
@@ -2,78 +2,7 @@
/*
* Author: Patrick Stählin <[email protected]>
*/
-#include <linux/linkage.h>
-
-#include <asm/asm.h>
-#include <asm/asm-offsets.h>
-
- .text
- .altmacro
-
- .macro save_all_base_regs
- REG_S x1, PT_RA(sp)
- REG_S x3, PT_GP(sp)
- REG_S x4, PT_TP(sp)
- REG_S x5, PT_T0(sp)
- REG_S x6, PT_T1(sp)
- REG_S x7, PT_T2(sp)
- REG_S x8, PT_S0(sp)
- REG_S x9, PT_S1(sp)
- REG_S x10, PT_A0(sp)
- REG_S x11, PT_A1(sp)
- REG_S x12, PT_A2(sp)
- REG_S x13, PT_A3(sp)
- REG_S x14, PT_A4(sp)
- REG_S x15, PT_A5(sp)
- REG_S x16, PT_A6(sp)
- REG_S x17, PT_A7(sp)
- REG_S x18, PT_S2(sp)
- REG_S x19, PT_S3(sp)
- REG_S x20, PT_S4(sp)
- REG_S x21, PT_S5(sp)
- REG_S x22, PT_S6(sp)
- REG_S x23, PT_S7(sp)
- REG_S x24, PT_S8(sp)
- REG_S x25, PT_S9(sp)
- REG_S x26, PT_S10(sp)
- REG_S x27, PT_S11(sp)
- REG_S x28, PT_T3(sp)
- REG_S x29, PT_T4(sp)
- REG_S x30, PT_T5(sp)
- REG_S x31, PT_T6(sp)
- .endm
-
- .macro restore_all_base_regs
- REG_L x3, PT_GP(sp)
- REG_L x4, PT_TP(sp)
- REG_L x5, PT_T0(sp)
- REG_L x6, PT_T1(sp)
- REG_L x7, PT_T2(sp)
- REG_L x8, PT_S0(sp)
- REG_L x9, PT_S1(sp)
- REG_L x10, PT_A0(sp)
- REG_L x11, PT_A1(sp)
- REG_L x12, PT_A2(sp)
- REG_L x13, PT_A3(sp)
- REG_L x14, PT_A4(sp)
- REG_L x15, PT_A5(sp)
- REG_L x16, PT_A6(sp)
- REG_L x17, PT_A7(sp)
- REG_L x18, PT_S2(sp)
- REG_L x19, PT_S3(sp)
- REG_L x20, PT_S4(sp)
- REG_L x21, PT_S5(sp)
- REG_L x22, PT_S6(sp)
- REG_L x23, PT_S7(sp)
- REG_L x24, PT_S8(sp)
- REG_L x25, PT_S9(sp)
- REG_L x26, PT_S10(sp)
- REG_L x27, PT_S11(sp)
- REG_L x28, PT_T3(sp)
- REG_L x29, PT_T4(sp)
- REG_L x30, PT_T5(sp)
- REG_L x31, PT_T6(sp)
- .endm
+#include "kprobes_trampoline.h"

ENTRY(__kretprobe_trampoline)
addi sp, sp, -(PT_SIZE_ON_STACK)
diff --git a/arch/riscv/kernel/probes/kprobes_trampoline.h b/arch/riscv/kernel/probes/kprobes_trampoline.h
new file mode 100644
index 000000000..48895a5e3
--- /dev/null
+++ b/arch/riscv/kernel/probes/kprobes_trampoline.h
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef _KPROBES_TRAMPOLINE_H
+#define _KPROBES_TRAMPOLINE_H
+/*
+ * Author: Patrick Stählin <[email protected]>
+ */
+#include <linux/linkage.h>
+
+#include <asm/asm.h>
+#include <asm/asm-offsets.h>
+
+ .text
+ .altmacro
+
+ .macro save_all_base_regs
+ REG_S x1, PT_RA(sp)
+ REG_S x3, PT_GP(sp)
+ REG_S x4, PT_TP(sp)
+ REG_S x5, PT_T0(sp)
+ REG_S x6, PT_T1(sp)
+ REG_S x7, PT_T2(sp)
+ REG_S x8, PT_S0(sp)
+ REG_S x9, PT_S1(sp)
+ REG_S x10, PT_A0(sp)
+ REG_S x11, PT_A1(sp)
+ REG_S x12, PT_A2(sp)
+ REG_S x13, PT_A3(sp)
+ REG_S x14, PT_A4(sp)
+ REG_S x15, PT_A5(sp)
+ REG_S x16, PT_A6(sp)
+ REG_S x17, PT_A7(sp)
+ REG_S x18, PT_S2(sp)
+ REG_S x19, PT_S3(sp)
+ REG_S x20, PT_S4(sp)
+ REG_S x21, PT_S5(sp)
+ REG_S x22, PT_S6(sp)
+ REG_S x23, PT_S7(sp)
+ REG_S x24, PT_S8(sp)
+ REG_S x25, PT_S9(sp)
+ REG_S x26, PT_S10(sp)
+ REG_S x27, PT_S11(sp)
+ REG_S x28, PT_T3(sp)
+ REG_S x29, PT_T4(sp)
+ REG_S x30, PT_T5(sp)
+ REG_S x31, PT_T6(sp)
+ .endm
+
+ .macro restore_all_base_regs
+ REG_L x3, PT_GP(sp)
+ REG_L x4, PT_TP(sp)
+ REG_L x5, PT_T0(sp)
+ REG_L x6, PT_T1(sp)
+ REG_L x7, PT_T2(sp)
+ REG_L x8, PT_S0(sp)
+ REG_L x9, PT_S1(sp)
+ REG_L x10, PT_A0(sp)
+ REG_L x11, PT_A1(sp)
+ REG_L x12, PT_A2(sp)
+ REG_L x13, PT_A3(sp)
+ REG_L x14, PT_A4(sp)
+ REG_L x15, PT_A5(sp)
+ REG_L x16, PT_A6(sp)
+ REG_L x17, PT_A7(sp)
+ REG_L x18, PT_S2(sp)
+ REG_L x19, PT_S3(sp)
+ REG_L x20, PT_S4(sp)
+ REG_L x21, PT_S5(sp)
+ REG_L x22, PT_S6(sp)
+ REG_L x23, PT_S7(sp)
+ REG_L x24, PT_S8(sp)
+ REG_L x25, PT_S9(sp)
+ REG_L x26, PT_S10(sp)
+ REG_L x27, PT_S11(sp)
+ REG_L x28, PT_T3(sp)
+ REG_L x29, PT_T4(sp)
+ REG_L x30, PT_T5(sp)
+ REG_L x31, PT_T6(sp)
+ .endm
+#endif
diff --git a/arch/riscv/kernel/probes/rethook.c b/arch/riscv/kernel/probes/rethook.c
new file mode 100644
index 000000000..4a41d5eb6
--- /dev/null
+++ b/arch/riscv/kernel/probes/rethook.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generic return hook for riscv.
+ */
+
+#include <linux/kprobes.h>
+#include <linux/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 = (u64)arch_rethook_trampoline;
+}
+NOKPROBE_SYMBOL(arch_rethook_prepare);
+
diff --git a/arch/riscv/kernel/probes/rethook_trampoline.S b/arch/riscv/kernel/probes/rethook_trampoline.S
new file mode 100644
index 000000000..e81c3d4e0
--- /dev/null
+++ b/arch/riscv/kernel/probes/rethook_trampoline.S
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Author: Patrick Stählin <[email protected]>
+ */
+#include "kprobes_trampoline.h"
+
+ENTRY(arch_rethook_trampoline)
+ addi sp, sp, -(PT_SIZE_ON_STACK)
+ save_all_base_regs
+
+ move a0, sp /* pt_regs */
+
+ call arch_rethook_trampoline_callback
+
+ /* use the result as the return-address */
+ move ra, a0
+
+ restore_all_base_regs
+ addi sp, sp, PT_SIZE_ON_STACK
+
+ ret
+ENDPROC(arch_rethook_trampoline)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 55041d2f8..a3805b5b2 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -210,6 +210,12 @@ static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_inst
{
return ri->node.ret_addr;
}
+static nokprobe_inline
+unsigned long kretprobe_trampoline_handler(struct pt_regs *regs,
+ void *frame_pointer)
+{
+ return 0;
+}
#else
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);
--
2.27.0

2022-09-22 14:02:06

by Binglei Wang

[permalink] [raw]
Subject: [PATCH v4] rethook: add riscv rethook implementation.

From: Binglei Wang <[email protected]>

Implement the kretprobes function on riscv arch by
using rethook machenism which abstracts general kretprobe info
into the struct rethook_node
to be embedded in the struct kretprobe_instance

Signed-off-by: Binglei Wang <[email protected]>
---
arch/riscv/Kconfig | 1 +
arch/riscv/kernel/probes/Makefile | 1 +
arch/riscv/kernel/probes/kprobes.c | 8 --
arch/riscv/kernel/probes/kprobes_trampoline.S | 73 +----------------
arch/riscv/kernel/probes/kprobes_trampoline.h | 79 +++++++++++++++++++
arch/riscv/kernel/probes/rethook.c | 24 ++++++
arch/riscv/kernel/probes/rethook_trampoline.S | 22 ++++++
include/linux/kprobes.h | 6 ++
8 files changed, 134 insertions(+), 80 deletions(-)
create mode 100644 arch/riscv/kernel/probes/kprobes_trampoline.h
create mode 100644 arch/riscv/kernel/probes/rethook.c
create mode 100644 arch/riscv/kernel/probes/rethook_trampoline.S

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index ed66c31e4..c5cae0825 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/kernel/probes/Makefile b/arch/riscv/kernel/probes/Makefile
index 7f0840dcc..ee345e7e9 100644
--- a/arch/riscv/kernel/probes/Makefile
+++ b/arch/riscv/kernel/probes/Makefile
@@ -3,4 +3,5 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o simulate-insn.o
obj-$(CONFIG_KPROBES) += kprobes_trampoline.o
obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o simulate-insn.o
+obj-$(CONFIG_RETHOOK) += rethook.o rethook_trampoline.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..04911cc42 100644
--- a/arch/riscv/kernel/probes/kprobes.c
+++ b/arch/riscv/kernel/probes/kprobes.c
@@ -350,14 +350,6 @@ 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/kprobes_trampoline.S b/arch/riscv/kernel/probes/kprobes_trampoline.S
index 7bdb09ded..54937f342 100644
--- a/arch/riscv/kernel/probes/kprobes_trampoline.S
+++ b/arch/riscv/kernel/probes/kprobes_trampoline.S
@@ -2,78 +2,7 @@
/*
* Author: Patrick Stählin <[email protected]>
*/
-#include <linux/linkage.h>
-
-#include <asm/asm.h>
-#include <asm/asm-offsets.h>
-
- .text
- .altmacro
-
- .macro save_all_base_regs
- REG_S x1, PT_RA(sp)
- REG_S x3, PT_GP(sp)
- REG_S x4, PT_TP(sp)
- REG_S x5, PT_T0(sp)
- REG_S x6, PT_T1(sp)
- REG_S x7, PT_T2(sp)
- REG_S x8, PT_S0(sp)
- REG_S x9, PT_S1(sp)
- REG_S x10, PT_A0(sp)
- REG_S x11, PT_A1(sp)
- REG_S x12, PT_A2(sp)
- REG_S x13, PT_A3(sp)
- REG_S x14, PT_A4(sp)
- REG_S x15, PT_A5(sp)
- REG_S x16, PT_A6(sp)
- REG_S x17, PT_A7(sp)
- REG_S x18, PT_S2(sp)
- REG_S x19, PT_S3(sp)
- REG_S x20, PT_S4(sp)
- REG_S x21, PT_S5(sp)
- REG_S x22, PT_S6(sp)
- REG_S x23, PT_S7(sp)
- REG_S x24, PT_S8(sp)
- REG_S x25, PT_S9(sp)
- REG_S x26, PT_S10(sp)
- REG_S x27, PT_S11(sp)
- REG_S x28, PT_T3(sp)
- REG_S x29, PT_T4(sp)
- REG_S x30, PT_T5(sp)
- REG_S x31, PT_T6(sp)
- .endm
-
- .macro restore_all_base_regs
- REG_L x3, PT_GP(sp)
- REG_L x4, PT_TP(sp)
- REG_L x5, PT_T0(sp)
- REG_L x6, PT_T1(sp)
- REG_L x7, PT_T2(sp)
- REG_L x8, PT_S0(sp)
- REG_L x9, PT_S1(sp)
- REG_L x10, PT_A0(sp)
- REG_L x11, PT_A1(sp)
- REG_L x12, PT_A2(sp)
- REG_L x13, PT_A3(sp)
- REG_L x14, PT_A4(sp)
- REG_L x15, PT_A5(sp)
- REG_L x16, PT_A6(sp)
- REG_L x17, PT_A7(sp)
- REG_L x18, PT_S2(sp)
- REG_L x19, PT_S3(sp)
- REG_L x20, PT_S4(sp)
- REG_L x21, PT_S5(sp)
- REG_L x22, PT_S6(sp)
- REG_L x23, PT_S7(sp)
- REG_L x24, PT_S8(sp)
- REG_L x25, PT_S9(sp)
- REG_L x26, PT_S10(sp)
- REG_L x27, PT_S11(sp)
- REG_L x28, PT_T3(sp)
- REG_L x29, PT_T4(sp)
- REG_L x30, PT_T5(sp)
- REG_L x31, PT_T6(sp)
- .endm
+#include "kprobes_trampoline.h"

ENTRY(__kretprobe_trampoline)
addi sp, sp, -(PT_SIZE_ON_STACK)
diff --git a/arch/riscv/kernel/probes/kprobes_trampoline.h b/arch/riscv/kernel/probes/kprobes_trampoline.h
new file mode 100644
index 000000000..48895a5e3
--- /dev/null
+++ b/arch/riscv/kernel/probes/kprobes_trampoline.h
@@ -0,0 +1,79 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef _KPROBES_TRAMPOLINE_H
+#define _KPROBES_TRAMPOLINE_H
+/*
+ * Author: Patrick Stählin <[email protected]>
+ */
+#include <linux/linkage.h>
+
+#include <asm/asm.h>
+#include <asm/asm-offsets.h>
+
+ .text
+ .altmacro
+
+ .macro save_all_base_regs
+ REG_S x1, PT_RA(sp)
+ REG_S x3, PT_GP(sp)
+ REG_S x4, PT_TP(sp)
+ REG_S x5, PT_T0(sp)
+ REG_S x6, PT_T1(sp)
+ REG_S x7, PT_T2(sp)
+ REG_S x8, PT_S0(sp)
+ REG_S x9, PT_S1(sp)
+ REG_S x10, PT_A0(sp)
+ REG_S x11, PT_A1(sp)
+ REG_S x12, PT_A2(sp)
+ REG_S x13, PT_A3(sp)
+ REG_S x14, PT_A4(sp)
+ REG_S x15, PT_A5(sp)
+ REG_S x16, PT_A6(sp)
+ REG_S x17, PT_A7(sp)
+ REG_S x18, PT_S2(sp)
+ REG_S x19, PT_S3(sp)
+ REG_S x20, PT_S4(sp)
+ REG_S x21, PT_S5(sp)
+ REG_S x22, PT_S6(sp)
+ REG_S x23, PT_S7(sp)
+ REG_S x24, PT_S8(sp)
+ REG_S x25, PT_S9(sp)
+ REG_S x26, PT_S10(sp)
+ REG_S x27, PT_S11(sp)
+ REG_S x28, PT_T3(sp)
+ REG_S x29, PT_T4(sp)
+ REG_S x30, PT_T5(sp)
+ REG_S x31, PT_T6(sp)
+ .endm
+
+ .macro restore_all_base_regs
+ REG_L x3, PT_GP(sp)
+ REG_L x4, PT_TP(sp)
+ REG_L x5, PT_T0(sp)
+ REG_L x6, PT_T1(sp)
+ REG_L x7, PT_T2(sp)
+ REG_L x8, PT_S0(sp)
+ REG_L x9, PT_S1(sp)
+ REG_L x10, PT_A0(sp)
+ REG_L x11, PT_A1(sp)
+ REG_L x12, PT_A2(sp)
+ REG_L x13, PT_A3(sp)
+ REG_L x14, PT_A4(sp)
+ REG_L x15, PT_A5(sp)
+ REG_L x16, PT_A6(sp)
+ REG_L x17, PT_A7(sp)
+ REG_L x18, PT_S2(sp)
+ REG_L x19, PT_S3(sp)
+ REG_L x20, PT_S4(sp)
+ REG_L x21, PT_S5(sp)
+ REG_L x22, PT_S6(sp)
+ REG_L x23, PT_S7(sp)
+ REG_L x24, PT_S8(sp)
+ REG_L x25, PT_S9(sp)
+ REG_L x26, PT_S10(sp)
+ REG_L x27, PT_S11(sp)
+ REG_L x28, PT_T3(sp)
+ REG_L x29, PT_T4(sp)
+ REG_L x30, PT_T5(sp)
+ REG_L x31, PT_T6(sp)
+ .endm
+#endif
diff --git a/arch/riscv/kernel/probes/rethook.c b/arch/riscv/kernel/probes/rethook.c
new file mode 100644
index 000000000..47853bc36
--- /dev/null
+++ b/arch/riscv/kernel/probes/rethook.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Generic return hook for riscv.
+ */
+
+#include <linux/kprobes.h>
+#include <linux/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 = (u64)arch_rethook_trampoline;
+}
+NOKPROBE_SYMBOL(arch_rethook_prepare);
diff --git a/arch/riscv/kernel/probes/rethook_trampoline.S b/arch/riscv/kernel/probes/rethook_trampoline.S
new file mode 100644
index 000000000..e81c3d4e0
--- /dev/null
+++ b/arch/riscv/kernel/probes/rethook_trampoline.S
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Author: Patrick Stählin <[email protected]>
+ */
+#include "kprobes_trampoline.h"
+
+ENTRY(arch_rethook_trampoline)
+ addi sp, sp, -(PT_SIZE_ON_STACK)
+ save_all_base_regs
+
+ move a0, sp /* pt_regs */
+
+ call arch_rethook_trampoline_callback
+
+ /* use the result as the return-address */
+ move ra, a0
+
+ restore_all_base_regs
+ addi sp, sp, PT_SIZE_ON_STACK
+
+ ret
+ENDPROC(arch_rethook_trampoline)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 55041d2f8..a3805b5b2 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -210,6 +210,12 @@ static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_inst
{
return ri->node.ret_addr;
}
+static nokprobe_inline
+unsigned long kretprobe_trampoline_handler(struct pt_regs *regs,
+ void *frame_pointer)
+{
+ return 0;
+}
#else
extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
struct pt_regs *regs);

base-commit: f30480cdeafc79d1f017d41335a96d8e3b973c91
--
2.25.1

2022-09-23 17:55:59

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4] rethook: add riscv rethook implementation.


Hey Binglei Wang,

I don't know anything about rethooks, so just more minor comments from
me. Don't respin until someone more knowledgable comes back with a
review of the patch itself.

Please send new versions of the patch as a new mail thread & not as
a reply to the existing one.

On Thu, Sep 22, 2022 at 09:44:28PM +0800, [email protected] wrote:
> From: Binglei Wang <[email protected]>
>
> Implement the kretprobes function on riscv arch by
> using rethook machenism which abstracts general kretprobe info
^mechanism
> into the struct rethook_node
> to be embedded in the struct kretprobe_instance

Whenever you resend, please fix the line wrapping here too.

> Signed-off-by: Binglei Wang <[email protected]>

Again, your first version of the patch used your work email as the
commit author/signer. The sending email does not need to match your
authorship/signature email so is there a reason not to have your h3c
email here?
Thanks,
Conor.

> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/kernel/probes/Makefile | 1 +
> arch/riscv/kernel/probes/kprobes.c | 8 --
> arch/riscv/kernel/probes/kprobes_trampoline.S | 73 +----------------
> arch/riscv/kernel/probes/kprobes_trampoline.h | 79 +++++++++++++++++++
> arch/riscv/kernel/probes/rethook.c | 24 ++++++
> arch/riscv/kernel/probes/rethook_trampoline.S | 22 ++++++
> include/linux/kprobes.h | 6 ++
> 8 files changed, 134 insertions(+), 80 deletions(-)
> create mode 100644 arch/riscv/kernel/probes/kprobes_trampoline.h
> create mode 100644 arch/riscv/kernel/probes/rethook.c
> create mode 100644 arch/riscv/kernel/probes/rethook_trampoline.S
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index ed66c31e4..c5cae0825 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/kernel/probes/Makefile b/arch/riscv/kernel/probes/Makefile
> index 7f0840dcc..ee345e7e9 100644
> --- a/arch/riscv/kernel/probes/Makefile
> +++ b/arch/riscv/kernel/probes/Makefile
> @@ -3,4 +3,5 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o simulate-insn.o
> obj-$(CONFIG_KPROBES) += kprobes_trampoline.o
> obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
> obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o simulate-insn.o
> +obj-$(CONFIG_RETHOOK) += rethook.o rethook_trampoline.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..04911cc42 100644
> --- a/arch/riscv/kernel/probes/kprobes.c
> +++ b/arch/riscv/kernel/probes/kprobes.c
> @@ -350,14 +350,6 @@ 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/kprobes_trampoline.S b/arch/riscv/kernel/probes/kprobes_trampoline.S
> index 7bdb09ded..54937f342 100644
> --- a/arch/riscv/kernel/probes/kprobes_trampoline.S
> +++ b/arch/riscv/kernel/probes/kprobes_trampoline.S
> @@ -2,78 +2,7 @@
> /*
> * Author: Patrick St?hlin <[email protected]>
> */
> -#include <linux/linkage.h>
> -
> -#include <asm/asm.h>
> -#include <asm/asm-offsets.h>
> -
> - .text
> - .altmacro
> -
> - .macro save_all_base_regs
> - REG_S x1, PT_RA(sp)
> - REG_S x3, PT_GP(sp)
> - REG_S x4, PT_TP(sp)
> - REG_S x5, PT_T0(sp)
> - REG_S x6, PT_T1(sp)
> - REG_S x7, PT_T2(sp)
> - REG_S x8, PT_S0(sp)
> - REG_S x9, PT_S1(sp)
> - REG_S x10, PT_A0(sp)
> - REG_S x11, PT_A1(sp)
> - REG_S x12, PT_A2(sp)
> - REG_S x13, PT_A3(sp)
> - REG_S x14, PT_A4(sp)
> - REG_S x15, PT_A5(sp)
> - REG_S x16, PT_A6(sp)
> - REG_S x17, PT_A7(sp)
> - REG_S x18, PT_S2(sp)
> - REG_S x19, PT_S3(sp)
> - REG_S x20, PT_S4(sp)
> - REG_S x21, PT_S5(sp)
> - REG_S x22, PT_S6(sp)
> - REG_S x23, PT_S7(sp)
> - REG_S x24, PT_S8(sp)
> - REG_S x25, PT_S9(sp)
> - REG_S x26, PT_S10(sp)
> - REG_S x27, PT_S11(sp)
> - REG_S x28, PT_T3(sp)
> - REG_S x29, PT_T4(sp)
> - REG_S x30, PT_T5(sp)
> - REG_S x31, PT_T6(sp)
> - .endm
> -
> - .macro restore_all_base_regs
> - REG_L x3, PT_GP(sp)
> - REG_L x4, PT_TP(sp)
> - REG_L x5, PT_T0(sp)
> - REG_L x6, PT_T1(sp)
> - REG_L x7, PT_T2(sp)
> - REG_L x8, PT_S0(sp)
> - REG_L x9, PT_S1(sp)
> - REG_L x10, PT_A0(sp)
> - REG_L x11, PT_A1(sp)
> - REG_L x12, PT_A2(sp)
> - REG_L x13, PT_A3(sp)
> - REG_L x14, PT_A4(sp)
> - REG_L x15, PT_A5(sp)
> - REG_L x16, PT_A6(sp)
> - REG_L x17, PT_A7(sp)
> - REG_L x18, PT_S2(sp)
> - REG_L x19, PT_S3(sp)
> - REG_L x20, PT_S4(sp)
> - REG_L x21, PT_S5(sp)
> - REG_L x22, PT_S6(sp)
> - REG_L x23, PT_S7(sp)
> - REG_L x24, PT_S8(sp)
> - REG_L x25, PT_S9(sp)
> - REG_L x26, PT_S10(sp)
> - REG_L x27, PT_S11(sp)
> - REG_L x28, PT_T3(sp)
> - REG_L x29, PT_T4(sp)
> - REG_L x30, PT_T5(sp)
> - REG_L x31, PT_T6(sp)
> - .endm
> +#include "kprobes_trampoline.h"
>
> ENTRY(__kretprobe_trampoline)
> addi sp, sp, -(PT_SIZE_ON_STACK)
> diff --git a/arch/riscv/kernel/probes/kprobes_trampoline.h b/arch/riscv/kernel/probes/kprobes_trampoline.h
> new file mode 100644
> index 000000000..48895a5e3
> --- /dev/null
> +++ b/arch/riscv/kernel/probes/kprobes_trampoline.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +#ifndef _KPROBES_TRAMPOLINE_H
> +#define _KPROBES_TRAMPOLINE_H
> +/*
> + * Author: Patrick St?hlin <[email protected]>
> + */
> +#include <linux/linkage.h>
> +
> +#include <asm/asm.h>
> +#include <asm/asm-offsets.h>
> +
> + .text
> + .altmacro
> +
> + .macro save_all_base_regs
> + REG_S x1, PT_RA(sp)
> + REG_S x3, PT_GP(sp)
> + REG_S x4, PT_TP(sp)
> + REG_S x5, PT_T0(sp)
> + REG_S x6, PT_T1(sp)
> + REG_S x7, PT_T2(sp)
> + REG_S x8, PT_S0(sp)
> + REG_S x9, PT_S1(sp)
> + REG_S x10, PT_A0(sp)
> + REG_S x11, PT_A1(sp)
> + REG_S x12, PT_A2(sp)
> + REG_S x13, PT_A3(sp)
> + REG_S x14, PT_A4(sp)
> + REG_S x15, PT_A5(sp)
> + REG_S x16, PT_A6(sp)
> + REG_S x17, PT_A7(sp)
> + REG_S x18, PT_S2(sp)
> + REG_S x19, PT_S3(sp)
> + REG_S x20, PT_S4(sp)
> + REG_S x21, PT_S5(sp)
> + REG_S x22, PT_S6(sp)
> + REG_S x23, PT_S7(sp)
> + REG_S x24, PT_S8(sp)
> + REG_S x25, PT_S9(sp)
> + REG_S x26, PT_S10(sp)
> + REG_S x27, PT_S11(sp)
> + REG_S x28, PT_T3(sp)
> + REG_S x29, PT_T4(sp)
> + REG_S x30, PT_T5(sp)
> + REG_S x31, PT_T6(sp)
> + .endm
> +
> + .macro restore_all_base_regs
> + REG_L x3, PT_GP(sp)
> + REG_L x4, PT_TP(sp)
> + REG_L x5, PT_T0(sp)
> + REG_L x6, PT_T1(sp)
> + REG_L x7, PT_T2(sp)
> + REG_L x8, PT_S0(sp)
> + REG_L x9, PT_S1(sp)
> + REG_L x10, PT_A0(sp)
> + REG_L x11, PT_A1(sp)
> + REG_L x12, PT_A2(sp)
> + REG_L x13, PT_A3(sp)
> + REG_L x14, PT_A4(sp)
> + REG_L x15, PT_A5(sp)
> + REG_L x16, PT_A6(sp)
> + REG_L x17, PT_A7(sp)
> + REG_L x18, PT_S2(sp)
> + REG_L x19, PT_S3(sp)
> + REG_L x20, PT_S4(sp)
> + REG_L x21, PT_S5(sp)
> + REG_L x22, PT_S6(sp)
> + REG_L x23, PT_S7(sp)
> + REG_L x24, PT_S8(sp)
> + REG_L x25, PT_S9(sp)
> + REG_L x26, PT_S10(sp)
> + REG_L x27, PT_S11(sp)
> + REG_L x28, PT_T3(sp)
> + REG_L x29, PT_T4(sp)
> + REG_L x30, PT_T5(sp)
> + REG_L x31, PT_T6(sp)
> + .endm
> +#endif
> diff --git a/arch/riscv/kernel/probes/rethook.c b/arch/riscv/kernel/probes/rethook.c
> new file mode 100644
> index 000000000..47853bc36
> --- /dev/null
> +++ b/arch/riscv/kernel/probes/rethook.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Generic return hook for riscv.
> + */
> +
> +#include <linux/kprobes.h>
> +#include <linux/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 = (u64)arch_rethook_trampoline;
> +}
> +NOKPROBE_SYMBOL(arch_rethook_prepare);
> diff --git a/arch/riscv/kernel/probes/rethook_trampoline.S b/arch/riscv/kernel/probes/rethook_trampoline.S
> new file mode 100644
> index 000000000..e81c3d4e0
> --- /dev/null
> +++ b/arch/riscv/kernel/probes/rethook_trampoline.S
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Author: Patrick St?hlin <[email protected]>
> + */
> +#include "kprobes_trampoline.h"
> +
> +ENTRY(arch_rethook_trampoline)
> + addi sp, sp, -(PT_SIZE_ON_STACK)
> + save_all_base_regs
> +
> + move a0, sp /* pt_regs */
> +
> + call arch_rethook_trampoline_callback
> +
> + /* use the result as the return-address */
> + move ra, a0
> +
> + restore_all_base_regs
> + addi sp, sp, PT_SIZE_ON_STACK
> +
> + ret
> +ENDPROC(arch_rethook_trampoline)
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 55041d2f8..a3805b5b2 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -210,6 +210,12 @@ static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_inst
> {
> return ri->node.ret_addr;
> }
> +static nokprobe_inline
> +unsigned long kretprobe_trampoline_handler(struct pt_regs *regs,
> + void *frame_pointer)
> +{
> + return 0;
> +}
> #else
> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> struct pt_regs *regs);
>
> base-commit: f30480cdeafc79d1f017d41335a96d8e3b973c91
> --
> 2.25.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2022-09-24 09:19:28

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH v4] rethook: add riscv rethook implementation.

Hi Binglei,

On Thu, 22 Sep 2022 21:44:28 +0800
[email protected] wrote:

> From: Binglei Wang <[email protected]>
>
> Implement the kretprobes function on riscv arch by
> using rethook machenism which abstracts general kretprobe info
> into the struct rethook_node
> to be embedded in the struct kretprobe_instance

Thanks for working on it.

BTW, you might work on older kernel. Could you check your kernel
has commit 73f9b911faa7 ("kprobes: Use rethook for kretprobe if possible") ?

It replaces the kretprobe implementation in kernel/kprobes.c with
rethook. This means your patch also must remove the arch dependent
kretprobe implementation (arch_prepare_kretprobe(), __kretprobe_trampoline,
trampoline_handler() and arch_kretprobe_fixup_return()), and you
don't need to create kprobes_trampoline.h, but just rename
kprobes_trampoline.S to rethook_trampoline.S.

Thank you,

>
> Signed-off-by: Binglei Wang <[email protected]>
> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/kernel/probes/Makefile | 1 +
> arch/riscv/kernel/probes/kprobes.c | 8 --
> arch/riscv/kernel/probes/kprobes_trampoline.S | 73 +----------------
> arch/riscv/kernel/probes/kprobes_trampoline.h | 79 +++++++++++++++++++
> arch/riscv/kernel/probes/rethook.c | 24 ++++++
> arch/riscv/kernel/probes/rethook_trampoline.S | 22 ++++++
> include/linux/kprobes.h | 6 ++
> 8 files changed, 134 insertions(+), 80 deletions(-)
> create mode 100644 arch/riscv/kernel/probes/kprobes_trampoline.h
> create mode 100644 arch/riscv/kernel/probes/rethook.c
> create mode 100644 arch/riscv/kernel/probes/rethook_trampoline.S
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index ed66c31e4..c5cae0825 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/kernel/probes/Makefile b/arch/riscv/kernel/probes/Makefile
> index 7f0840dcc..ee345e7e9 100644
> --- a/arch/riscv/kernel/probes/Makefile
> +++ b/arch/riscv/kernel/probes/Makefile
> @@ -3,4 +3,5 @@ obj-$(CONFIG_KPROBES) += kprobes.o decode-insn.o simulate-insn.o
> obj-$(CONFIG_KPROBES) += kprobes_trampoline.o
> obj-$(CONFIG_KPROBES_ON_FTRACE) += ftrace.o
> obj-$(CONFIG_UPROBES) += uprobes.o decode-insn.o simulate-insn.o
> +obj-$(CONFIG_RETHOOK) += rethook.o rethook_trampoline.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..04911cc42 100644
> --- a/arch/riscv/kernel/probes/kprobes.c
> +++ b/arch/riscv/kernel/probes/kprobes.c
> @@ -350,14 +350,6 @@ 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/kprobes_trampoline.S b/arch/riscv/kernel/probes/kprobes_trampoline.S
> index 7bdb09ded..54937f342 100644
> --- a/arch/riscv/kernel/probes/kprobes_trampoline.S
> +++ b/arch/riscv/kernel/probes/kprobes_trampoline.S
> @@ -2,78 +2,7 @@
> /*
> * Author: Patrick Stählin <[email protected]>
> */
> -#include <linux/linkage.h>
> -
> -#include <asm/asm.h>
> -#include <asm/asm-offsets.h>
> -
> - .text
> - .altmacro
> -
> - .macro save_all_base_regs
> - REG_S x1, PT_RA(sp)
> - REG_S x3, PT_GP(sp)
> - REG_S x4, PT_TP(sp)
> - REG_S x5, PT_T0(sp)
> - REG_S x6, PT_T1(sp)
> - REG_S x7, PT_T2(sp)
> - REG_S x8, PT_S0(sp)
> - REG_S x9, PT_S1(sp)
> - REG_S x10, PT_A0(sp)
> - REG_S x11, PT_A1(sp)
> - REG_S x12, PT_A2(sp)
> - REG_S x13, PT_A3(sp)
> - REG_S x14, PT_A4(sp)
> - REG_S x15, PT_A5(sp)
> - REG_S x16, PT_A6(sp)
> - REG_S x17, PT_A7(sp)
> - REG_S x18, PT_S2(sp)
> - REG_S x19, PT_S3(sp)
> - REG_S x20, PT_S4(sp)
> - REG_S x21, PT_S5(sp)
> - REG_S x22, PT_S6(sp)
> - REG_S x23, PT_S7(sp)
> - REG_S x24, PT_S8(sp)
> - REG_S x25, PT_S9(sp)
> - REG_S x26, PT_S10(sp)
> - REG_S x27, PT_S11(sp)
> - REG_S x28, PT_T3(sp)
> - REG_S x29, PT_T4(sp)
> - REG_S x30, PT_T5(sp)
> - REG_S x31, PT_T6(sp)
> - .endm
> -
> - .macro restore_all_base_regs
> - REG_L x3, PT_GP(sp)
> - REG_L x4, PT_TP(sp)
> - REG_L x5, PT_T0(sp)
> - REG_L x6, PT_T1(sp)
> - REG_L x7, PT_T2(sp)
> - REG_L x8, PT_S0(sp)
> - REG_L x9, PT_S1(sp)
> - REG_L x10, PT_A0(sp)
> - REG_L x11, PT_A1(sp)
> - REG_L x12, PT_A2(sp)
> - REG_L x13, PT_A3(sp)
> - REG_L x14, PT_A4(sp)
> - REG_L x15, PT_A5(sp)
> - REG_L x16, PT_A6(sp)
> - REG_L x17, PT_A7(sp)
> - REG_L x18, PT_S2(sp)
> - REG_L x19, PT_S3(sp)
> - REG_L x20, PT_S4(sp)
> - REG_L x21, PT_S5(sp)
> - REG_L x22, PT_S6(sp)
> - REG_L x23, PT_S7(sp)
> - REG_L x24, PT_S8(sp)
> - REG_L x25, PT_S9(sp)
> - REG_L x26, PT_S10(sp)
> - REG_L x27, PT_S11(sp)
> - REG_L x28, PT_T3(sp)
> - REG_L x29, PT_T4(sp)
> - REG_L x30, PT_T5(sp)
> - REG_L x31, PT_T6(sp)
> - .endm
> +#include "kprobes_trampoline.h"
>
> ENTRY(__kretprobe_trampoline)
> addi sp, sp, -(PT_SIZE_ON_STACK)
> diff --git a/arch/riscv/kernel/probes/kprobes_trampoline.h b/arch/riscv/kernel/probes/kprobes_trampoline.h
> new file mode 100644
> index 000000000..48895a5e3
> --- /dev/null
> +++ b/arch/riscv/kernel/probes/kprobes_trampoline.h
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +#ifndef _KPROBES_TRAMPOLINE_H
> +#define _KPROBES_TRAMPOLINE_H
> +/*
> + * Author: Patrick Stählin <[email protected]>
> + */
> +#include <linux/linkage.h>
> +
> +#include <asm/asm.h>
> +#include <asm/asm-offsets.h>
> +
> + .text
> + .altmacro
> +
> + .macro save_all_base_regs
> + REG_S x1, PT_RA(sp)
> + REG_S x3, PT_GP(sp)
> + REG_S x4, PT_TP(sp)
> + REG_S x5, PT_T0(sp)
> + REG_S x6, PT_T1(sp)
> + REG_S x7, PT_T2(sp)
> + REG_S x8, PT_S0(sp)
> + REG_S x9, PT_S1(sp)
> + REG_S x10, PT_A0(sp)
> + REG_S x11, PT_A1(sp)
> + REG_S x12, PT_A2(sp)
> + REG_S x13, PT_A3(sp)
> + REG_S x14, PT_A4(sp)
> + REG_S x15, PT_A5(sp)
> + REG_S x16, PT_A6(sp)
> + REG_S x17, PT_A7(sp)
> + REG_S x18, PT_S2(sp)
> + REG_S x19, PT_S3(sp)
> + REG_S x20, PT_S4(sp)
> + REG_S x21, PT_S5(sp)
> + REG_S x22, PT_S6(sp)
> + REG_S x23, PT_S7(sp)
> + REG_S x24, PT_S8(sp)
> + REG_S x25, PT_S9(sp)
> + REG_S x26, PT_S10(sp)
> + REG_S x27, PT_S11(sp)
> + REG_S x28, PT_T3(sp)
> + REG_S x29, PT_T4(sp)
> + REG_S x30, PT_T5(sp)
> + REG_S x31, PT_T6(sp)
> + .endm
> +
> + .macro restore_all_base_regs
> + REG_L x3, PT_GP(sp)
> + REG_L x4, PT_TP(sp)
> + REG_L x5, PT_T0(sp)
> + REG_L x6, PT_T1(sp)
> + REG_L x7, PT_T2(sp)
> + REG_L x8, PT_S0(sp)
> + REG_L x9, PT_S1(sp)
> + REG_L x10, PT_A0(sp)
> + REG_L x11, PT_A1(sp)
> + REG_L x12, PT_A2(sp)
> + REG_L x13, PT_A3(sp)
> + REG_L x14, PT_A4(sp)
> + REG_L x15, PT_A5(sp)
> + REG_L x16, PT_A6(sp)
> + REG_L x17, PT_A7(sp)
> + REG_L x18, PT_S2(sp)
> + REG_L x19, PT_S3(sp)
> + REG_L x20, PT_S4(sp)
> + REG_L x21, PT_S5(sp)
> + REG_L x22, PT_S6(sp)
> + REG_L x23, PT_S7(sp)
> + REG_L x24, PT_S8(sp)
> + REG_L x25, PT_S9(sp)
> + REG_L x26, PT_S10(sp)
> + REG_L x27, PT_S11(sp)
> + REG_L x28, PT_T3(sp)
> + REG_L x29, PT_T4(sp)
> + REG_L x30, PT_T5(sp)
> + REG_L x31, PT_T6(sp)
> + .endm
> +#endif
> diff --git a/arch/riscv/kernel/probes/rethook.c b/arch/riscv/kernel/probes/rethook.c
> new file mode 100644
> index 000000000..47853bc36
> --- /dev/null
> +++ b/arch/riscv/kernel/probes/rethook.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Generic return hook for riscv.
> + */
> +
> +#include <linux/kprobes.h>
> +#include <linux/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 = (u64)arch_rethook_trampoline;
> +}
> +NOKPROBE_SYMBOL(arch_rethook_prepare);
> diff --git a/arch/riscv/kernel/probes/rethook_trampoline.S b/arch/riscv/kernel/probes/rethook_trampoline.S
> new file mode 100644
> index 000000000..e81c3d4e0
> --- /dev/null
> +++ b/arch/riscv/kernel/probes/rethook_trampoline.S
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Author: Patrick Stählin <[email protected]>
> + */
> +#include "kprobes_trampoline.h"
> +
> +ENTRY(arch_rethook_trampoline)
> + addi sp, sp, -(PT_SIZE_ON_STACK)
> + save_all_base_regs
> +
> + move a0, sp /* pt_regs */
> +
> + call arch_rethook_trampoline_callback
> +
> + /* use the result as the return-address */
> + move ra, a0
> +
> + restore_all_base_regs
> + addi sp, sp, PT_SIZE_ON_STACK
> +
> + ret
> +ENDPROC(arch_rethook_trampoline)
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 55041d2f8..a3805b5b2 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -210,6 +210,12 @@ static nokprobe_inline unsigned long get_kretprobe_retaddr(struct kretprobe_inst
> {
> return ri->node.ret_addr;
> }
> +static nokprobe_inline
> +unsigned long kretprobe_trampoline_handler(struct pt_regs *regs,
> + void *frame_pointer)
> +{
> + return 0;
> +}
> #else
> extern void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> struct pt_regs *regs);
>
> base-commit: f30480cdeafc79d1f017d41335a96d8e3b973c91
> --
> 2.25.1
>


--
Masami Hiramatsu (Google) <[email protected]>