[PATCH v3] LoongArch/rethook: Replace kretprobe with rethook on LoongArch

Haoran Jiang posted 1 patch 2 years, 8 months ago
There is a newer version of this series
arch/loongarch/Kconfig                        |  1 +
arch/loongarch/include/asm/kprobes.h          |  3 ---
arch/loongarch/kernel/Makefile                |  7 ++++-
arch/loongarch/kernel/kprobes.c               | 20 --------------
arch/loongarch/kernel/rethook.c               | 27 +++++++++++++++++++
arch/loongarch/kernel/rethook.h               |  8 ++++++
...obes_trampoline.S => rethook_trampoline.S} |  6 ++---
7 files changed, 45 insertions(+), 27 deletions(-)
create mode 100644 arch/loongarch/kernel/rethook.c
create mode 100644 arch/loongarch/kernel/rethook.h
rename arch/loongarch/kernel/{kprobes_trampoline.S => rethook_trampoline.S} (93%)
[PATCH v3] LoongArch/rethook: Replace kretprobe with rethook on LoongArch
Posted by Haoran Jiang 2 years, 8 months ago
That's an adaptation of commit f3a112c0c40d ("x86,rethook,kprobes:
Replace kretprobe with rethook on x86")and commit b57c2f124098
("riscv: add riscv rethook implementation") to LoongArch.
Mainly refer to this commit
b57c2f124098 ("riscv: add riscv rethook implementation").

Replaces the kretprobe code with rethook on LoongArch. With this patch,
kretprobe on LoongArch uses the rethook instead of kretprobe specific
trampoline code.

Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>

---
v3:
1,Fixed some format and spelling issues
2,According to this commit 571a2a50a8fc
(rethook, fprobe: do not trace rethook related functions),
the problem described in this commit Should be modified on LoongArch.

v2:
Modified the patch format and commit message.
---
 arch/loongarch/Kconfig                        |  1 +
 arch/loongarch/include/asm/kprobes.h          |  3 ---
 arch/loongarch/kernel/Makefile                |  7 ++++-
 arch/loongarch/kernel/kprobes.c               | 20 --------------
 arch/loongarch/kernel/rethook.c               | 27 +++++++++++++++++++
 arch/loongarch/kernel/rethook.h               |  8 ++++++
 ...obes_trampoline.S => rethook_trampoline.S} |  6 ++---
 7 files changed, 45 insertions(+), 27 deletions(-)
 create mode 100644 arch/loongarch/kernel/rethook.c
 create mode 100644 arch/loongarch/kernel/rethook.h
 rename arch/loongarch/kernel/{kprobes_trampoline.S => rethook_trampoline.S} (93%)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index d38b066fc931..5800aa52c82f 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -120,6 +120,7 @@ config LOONGARCH
 	select HAVE_PERF_REGS
 	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_REGS_AND_STACK_ACCESS_API
+	select HAVE_RETHOOK
 	select HAVE_RSEQ
 	select HAVE_SAMPLE_FTRACE_DIRECT
 	select HAVE_SAMPLE_FTRACE_DIRECT_MULTI
diff --git a/arch/loongarch/include/asm/kprobes.h b/arch/loongarch/include/asm/kprobes.h
index 798020ae02c6..7b9fc3ed71c3 100644
--- a/arch/loongarch/include/asm/kprobes.h
+++ b/arch/loongarch/include/asm/kprobes.h
@@ -49,9 +49,6 @@ bool kprobe_fault_handler(struct pt_regs *regs, int trapnr);
 bool kprobe_breakpoint_handler(struct pt_regs *regs);
 bool kprobe_singlestep_handler(struct pt_regs *regs);
 
-void __kretprobe_trampoline(void);
-void *trampoline_probe_handler(struct pt_regs *regs);
-
 #else /* !CONFIG_KPROBES */
 
 static inline bool kprobe_breakpoint_handler(struct pt_regs *regs) { return false; }
diff --git a/arch/loongarch/kernel/Makefile b/arch/loongarch/kernel/Makefile
index 9a72d91cd104..7106ca996e55 100644
--- a/arch/loongarch/kernel/Makefile
+++ b/arch/loongarch/kernel/Makefile
@@ -52,6 +52,11 @@ obj-$(CONFIG_UNWINDER_PROLOGUE) += unwind_prologue.o
 obj-$(CONFIG_PERF_EVENTS)	+= perf_event.o perf_regs.o
 obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
 
-obj-$(CONFIG_KPROBES)		+= kprobes.o kprobes_trampoline.o
+obj-$(CONFIG_KPROBES)		+= kprobes.o
+obj-$(CONFIG_RETHOOK)		+= rethook.o rethook_trampoline.o
+
+CFLAGS_REMOVE_rethook.o = $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_rethook_trampoline.o = $(CC_FLAGS_FTRACE)
+
 
 CPPFLAGS_vmlinux.lds		:= $(KBUILD_CFLAGS)
diff --git a/arch/loongarch/kernel/kprobes.c b/arch/loongarch/kernel/kprobes.c
index 56c8c4b09a42..dbce23ba9970 100644
--- a/arch/loongarch/kernel/kprobes.c
+++ b/arch/loongarch/kernel/kprobes.c
@@ -378,26 +378,6 @@ int __init arch_init_kprobes(void)
 	return 0;
 }
 
-/* ASM function that handles the kretprobes must not be probed */
-NOKPROBE_SYMBOL(__kretprobe_trampoline);
-
-/* Called from __kretprobe_trampoline */
-void __used *trampoline_probe_handler(struct pt_regs *regs)
-{
-	return (void *)kretprobe_trampoline_handler(regs, NULL);
-}
-NOKPROBE_SYMBOL(trampoline_probe_handler);
-
-void arch_prepare_kretprobe(struct kretprobe_instance *ri,
-			    struct pt_regs *regs)
-{
-	ri->ret_addr = (kprobe_opcode_t *)regs->regs[1];
-	ri->fp = NULL;
-
-	/* Replace the return addr with trampoline addr */
-	regs->regs[1] = (unsigned long)&__kretprobe_trampoline;
-}
-NOKPROBE_SYMBOL(arch_prepare_kretprobe);
 
 int arch_trampoline_kprobe(struct kprobe *p)
 {
diff --git a/arch/loongarch/kernel/rethook.c b/arch/loongarch/kernel/rethook.c
new file mode 100644
index 000000000000..b5e7f62f7dea
--- /dev/null
+++ b/arch/loongarch/kernel/rethook.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Generic return hook for LoongArch.
+ */
+
+#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, 0);
+}
+
+NOKPROBE_SYMBOL(arch_rethook_trampoline_callback);
+
+void arch_rethook_prepare(struct rethook_node *rhn, struct pt_regs *regs, bool mcount)
+{
+	rhn->ret_addr = regs->regs[1];
+	rhn->frame = 0;
+
+	/* replace return addr with trampoline */
+	regs->regs[1] = (unsigned long)arch_rethook_trampoline;
+}
+
+NOKPROBE_SYMBOL(arch_rethook_prepare);
diff --git a/arch/loongarch/kernel/rethook.h b/arch/loongarch/kernel/rethook.h
new file mode 100644
index 000000000000..3f1c1edf0d0b
--- /dev/null
+++ b/arch/loongarch/kernel/rethook.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LOONGARCH_RETHOOK_H
+#define __LOONGARCH_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/loongarch/kernel/kprobes_trampoline.S b/arch/loongarch/kernel/rethook_trampoline.S
similarity index 93%
rename from arch/loongarch/kernel/kprobes_trampoline.S
rename to arch/loongarch/kernel/rethook_trampoline.S
index af94b0d213fa..bd5772c96338 100644
--- a/arch/loongarch/kernel/kprobes_trampoline.S
+++ b/arch/loongarch/kernel/rethook_trampoline.S
@@ -75,7 +75,7 @@
 	csrxchg t0, t1, LOONGARCH_CSR_CRMD
 	.endm
 
-SYM_CODE_START(__kretprobe_trampoline)
+SYM_CODE_START(arch_rethook_trampoline)
 	addi.d	sp, sp, -PT_SIZE
 	save_all_base_regs
 
@@ -84,7 +84,7 @@ SYM_CODE_START(__kretprobe_trampoline)
 
 	move a0, sp /* pt_regs */
 
-	bl trampoline_probe_handler
+	bl arch_rethook_trampoline_callback
 
 	/* use the result as the return-address */
 	move ra, a0
@@ -93,4 +93,4 @@ SYM_CODE_START(__kretprobe_trampoline)
 	addi.d	sp, sp, PT_SIZE
 
 	jr ra
-SYM_CODE_END(__kretprobe_trampoline)
+SYM_CODE_END(arch_rethook_trampoline)
-- 
2.27.0
Re: [PATCH v3] LoongArch/rethook: Replace kretprobe with rethook on LoongArch
Posted by Huacai Chen 2 years, 8 months ago
On Tue, Jun 13, 2023 at 2:09 PM Haoran Jiang <jianghaoran@kylinos.cn> wrote:
>
> That's an adaptation of commit f3a112c0c40d ("x86,rethook,kprobes:
> Replace kretprobe with rethook on x86")and commit b57c2f124098
> ("riscv: add riscv rethook implementation") to LoongArch.
> Mainly refer to this commit
> b57c2f124098 ("riscv: add riscv rethook implementation").
>
> Replaces the kretprobe code with rethook on LoongArch. With this patch,
> kretprobe on LoongArch uses the rethook instead of kretprobe specific
> trampoline code.
>
> Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>
>
> ---
> v3:
> 1,Fixed some format and spelling issues
> 2,According to this commit 571a2a50a8fc
> (rethook, fprobe: do not trace rethook related functions),
> the problem described in this commit Should be modified on LoongArch.
>
> v2:
> Modified the patch format and commit message.
> ---
>  arch/loongarch/Kconfig                        |  1 +
>  arch/loongarch/include/asm/kprobes.h          |  3 ---
>  arch/loongarch/kernel/Makefile                |  7 ++++-
>  arch/loongarch/kernel/kprobes.c               | 20 --------------
>  arch/loongarch/kernel/rethook.c               | 27 +++++++++++++++++++
>  arch/loongarch/kernel/rethook.h               |  8 ++++++
>  ...obes_trampoline.S => rethook_trampoline.S} |  6 ++---
>  7 files changed, 45 insertions(+), 27 deletions(-)
>  create mode 100644 arch/loongarch/kernel/rethook.c
>  create mode 100644 arch/loongarch/kernel/rethook.h
>  rename arch/loongarch/kernel/{kprobes_trampoline.S => rethook_trampoline.S} (93%)
>
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index d38b066fc931..5800aa52c82f 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -120,6 +120,7 @@ config LOONGARCH
>         select HAVE_PERF_REGS
>         select HAVE_PERF_USER_STACK_DUMP
>         select HAVE_REGS_AND_STACK_ACCESS_API
> +       select HAVE_RETHOOK
>         select HAVE_RSEQ
>         select HAVE_SAMPLE_FTRACE_DIRECT
>         select HAVE_SAMPLE_FTRACE_DIRECT_MULTI
> diff --git a/arch/loongarch/include/asm/kprobes.h b/arch/loongarch/include/asm/kprobes.h
> index 798020ae02c6..7b9fc3ed71c3 100644
> --- a/arch/loongarch/include/asm/kprobes.h
> +++ b/arch/loongarch/include/asm/kprobes.h
> @@ -49,9 +49,6 @@ bool kprobe_fault_handler(struct pt_regs *regs, int trapnr);
>  bool kprobe_breakpoint_handler(struct pt_regs *regs);
>  bool kprobe_singlestep_handler(struct pt_regs *regs);
>
> -void __kretprobe_trampoline(void);
> -void *trampoline_probe_handler(struct pt_regs *regs);
> -
>  #else /* !CONFIG_KPROBES */
>
>  static inline bool kprobe_breakpoint_handler(struct pt_regs *regs) { return false; }
> diff --git a/arch/loongarch/kernel/Makefile b/arch/loongarch/kernel/Makefile
> index 9a72d91cd104..7106ca996e55 100644
> --- a/arch/loongarch/kernel/Makefile
> +++ b/arch/loongarch/kernel/Makefile
> @@ -52,6 +52,11 @@ obj-$(CONFIG_UNWINDER_PROLOGUE) += unwind_prologue.o
>  obj-$(CONFIG_PERF_EVENTS)      += perf_event.o perf_regs.o
>  obj-$(CONFIG_HAVE_HW_BREAKPOINT)       += hw_breakpoint.o
>
> -obj-$(CONFIG_KPROBES)          += kprobes.o kprobes_trampoline.o
> +obj-$(CONFIG_KPROBES)          += kprobes.o
> +obj-$(CONFIG_RETHOOK)          += rethook.o rethook_trampoline.o
> +
> +CFLAGS_REMOVE_rethook.o = $(CC_FLAGS_FTRACE)
> +CFLAGS_REMOVE_rethook_trampoline.o = $(CC_FLAGS_FTRACE)
Move them to below CFLAGS_REMOVE_perf_event.o = $(CC_FLAGS_FTRACE), thanks.

Huacai
> +
>
>  CPPFLAGS_vmlinux.lds           := $(KBUILD_CFLAGS)
> diff --git a/arch/loongarch/kernel/kprobes.c b/arch/loongarch/kernel/kprobes.c
> index 56c8c4b09a42..dbce23ba9970 100644
> --- a/arch/loongarch/kernel/kprobes.c
> +++ b/arch/loongarch/kernel/kprobes.c
> @@ -378,26 +378,6 @@ int __init arch_init_kprobes(void)
>         return 0;
>  }
>
> -/* ASM function that handles the kretprobes must not be probed */
> -NOKPROBE_SYMBOL(__kretprobe_trampoline);
> -
> -/* Called from __kretprobe_trampoline */
> -void __used *trampoline_probe_handler(struct pt_regs *regs)
> -{
> -       return (void *)kretprobe_trampoline_handler(regs, NULL);
> -}
> -NOKPROBE_SYMBOL(trampoline_probe_handler);
> -
> -void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> -                           struct pt_regs *regs)
> -{
> -       ri->ret_addr = (kprobe_opcode_t *)regs->regs[1];
> -       ri->fp = NULL;
> -
> -       /* Replace the return addr with trampoline addr */
> -       regs->regs[1] = (unsigned long)&__kretprobe_trampoline;
> -}
> -NOKPROBE_SYMBOL(arch_prepare_kretprobe);
>
>  int arch_trampoline_kprobe(struct kprobe *p)
>  {
> diff --git a/arch/loongarch/kernel/rethook.c b/arch/loongarch/kernel/rethook.c
> new file mode 100644
> index 000000000000..b5e7f62f7dea
> --- /dev/null
> +++ b/arch/loongarch/kernel/rethook.c
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Generic return hook for LoongArch.
> + */
> +
> +#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, 0);
> +}
> +
> +NOKPROBE_SYMBOL(arch_rethook_trampoline_callback);
> +
> +void arch_rethook_prepare(struct rethook_node *rhn, struct pt_regs *regs, bool mcount)
> +{
> +       rhn->ret_addr = regs->regs[1];
> +       rhn->frame = 0;
> +
> +       /* replace return addr with trampoline */
> +       regs->regs[1] = (unsigned long)arch_rethook_trampoline;
> +}
> +
> +NOKPROBE_SYMBOL(arch_rethook_prepare);
> diff --git a/arch/loongarch/kernel/rethook.h b/arch/loongarch/kernel/rethook.h
> new file mode 100644
> index 000000000000..3f1c1edf0d0b
> --- /dev/null
> +++ b/arch/loongarch/kernel/rethook.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __LOONGARCH_RETHOOK_H
> +#define __LOONGARCH_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/loongarch/kernel/kprobes_trampoline.S b/arch/loongarch/kernel/rethook_trampoline.S
> similarity index 93%
> rename from arch/loongarch/kernel/kprobes_trampoline.S
> rename to arch/loongarch/kernel/rethook_trampoline.S
> index af94b0d213fa..bd5772c96338 100644
> --- a/arch/loongarch/kernel/kprobes_trampoline.S
> +++ b/arch/loongarch/kernel/rethook_trampoline.S
> @@ -75,7 +75,7 @@
>         csrxchg t0, t1, LOONGARCH_CSR_CRMD
>         .endm
>
> -SYM_CODE_START(__kretprobe_trampoline)
> +SYM_CODE_START(arch_rethook_trampoline)
>         addi.d  sp, sp, -PT_SIZE
>         save_all_base_regs
>
> @@ -84,7 +84,7 @@ SYM_CODE_START(__kretprobe_trampoline)
>
>         move a0, sp /* pt_regs */
>
> -       bl trampoline_probe_handler
> +       bl arch_rethook_trampoline_callback
>
>         /* use the result as the return-address */
>         move ra, a0
> @@ -93,4 +93,4 @@ SYM_CODE_START(__kretprobe_trampoline)
>         addi.d  sp, sp, PT_SIZE
>
>         jr ra
> -SYM_CODE_END(__kretprobe_trampoline)
> +SYM_CODE_END(arch_rethook_trampoline)
> --
> 2.27.0
>
Re: [PATCH v3] LoongArch/rethook: Replace kretprobe with rethook on LoongArch
Posted by Tiezhu Yang 2 years, 8 months ago

On 06/13/2023 02:03 PM, Haoran Jiang wrote:
> That's an adaptation of commit f3a112c0c40d ("x86,rethook,kprobes:
> Replace kretprobe with rethook on x86")and commit b57c2f124098
> ("riscv: add riscv rethook implementation") to LoongArch.
> Mainly refer to this commit
> b57c2f124098 ("riscv: add riscv rethook implementation").
>
> Replaces the kretprobe code with rethook on LoongArch. With this patch,
> kretprobe on LoongArch uses the rethook instead of kretprobe specific
> trampoline code.

Maybe Huacai can refine the commit message.

>
> Signed-off-by: Haoran Jiang <jianghaoran@kylinos.cn>

...

> diff --git a/arch/loongarch/kernel/kprobes.c b/arch/loongarch/kernel/kprobes.c
> index 56c8c4b09a42..dbce23ba9970 100644
> --- a/arch/loongarch/kernel/kprobes.c
> +++ b/arch/loongarch/kernel/kprobes.c
> @@ -378,26 +378,6 @@ int __init arch_init_kprobes(void)
>  	return 0;
>  }
>
> -/* ASM function that handles the kretprobes must not be probed */
> -NOKPROBE_SYMBOL(__kretprobe_trampoline);
> -
> -/* Called from __kretprobe_trampoline */
> -void __used *trampoline_probe_handler(struct pt_regs *regs)
> -{
> -	return (void *)kretprobe_trampoline_handler(regs, NULL);
> -}
> -NOKPROBE_SYMBOL(trampoline_probe_handler);
> -
> -void arch_prepare_kretprobe(struct kretprobe_instance *ri,
> -			    struct pt_regs *regs)
> -{
> -	ri->ret_addr = (kprobe_opcode_t *)regs->regs[1];
> -	ri->fp = NULL;
> -
> -	/* Replace the return addr with trampoline addr */
> -	regs->regs[1] = (unsigned long)&__kretprobe_trampoline;
> -}
> -NOKPROBE_SYMBOL(arch_prepare_kretprobe);
>

Please remove the blank line.

>  int arch_trampoline_kprobe(struct kprobe *p)
>  {
> diff --git a/arch/loongarch/kernel/rethook.c b/arch/loongarch/kernel/rethook.c
> new file mode 100644
> index 000000000000..b5e7f62f7dea
> --- /dev/null
> +++ b/arch/loongarch/kernel/rethook.c
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Generic return hook for LoongArch.
> + */
> +
> +#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, 0);
> +}
> +

Ditto

> +NOKPROBE_SYMBOL(arch_rethook_trampoline_callback);
> +
> +void arch_rethook_prepare(struct rethook_node *rhn, struct pt_regs *regs, bool mcount)
> +{
> +	rhn->ret_addr = regs->regs[1];
> +	rhn->frame = 0;
> +
> +	/* replace return addr with trampoline */
> +	regs->regs[1] = (unsigned long)arch_rethook_trampoline;
> +}
> +

Ditto

> +NOKPROBE_SYMBOL(arch_rethook_prepare);

Like s390 and x86, please add:

/* assembler function that handles the rethook must not be probed itself */
NOKPROBE_SYMBOL(arch_rethook_trampoline);

> diff --git a/arch/loongarch/kernel/rethook.h b/arch/loongarch/kernel/rethook.h

...

> -SYM_CODE_END(__kretprobe_trampoline)
> +SYM_CODE_END(arch_rethook_trampoline)

Thanks,
Tiezhu