On Mon, 15 May 2023 11:26:41 +0800
Ze Gao <zegao2021@xxxxxxxxx> wrote:
> These functions are already marked as NOKPROBE to prevent recusion and
> we have the same reason to blacklist them if rethook is used with fprobe,
> since they are beyond the recursion-free region ftrace can guard.
>
> Signed-off-by: Ze Gao <zegao@xxxxxxxxxxx>
> ---
> arch/riscv/kernel/probes/rethook.c | 4 ++--
> arch/s390/kernel/rethook.c | 6 +++---
> arch/x86/kernel/rethook.c | 8 +++++---
> kernel/trace/rethook.c | 8 ++++----
Except for the kernel/trace/rethook.c, those looks good to me.
Could you drop notrace from kernel/trace/rethook.c? As Steve mentioned
all functions in that file is automatically notraced.
Thank you,
> 4 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/arch/riscv/kernel/probes/rethook.c b/arch/riscv/kernel/probes/rethook.c
> index 5c27c1f50989..803c412a1bea 100644
> --- a/arch/riscv/kernel/probes/rethook.c
> +++ b/arch/riscv/kernel/probes/rethook.c
> @@ -8,14 +8,14 @@
> #include "rethook.h"
>
> /* This is called from arch_rethook_trampoline() */
> -unsigned long __used arch_rethook_trampoline_callback(struct pt_regs *regs)
> +unsigned long __used notrace 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)
> +void notrace arch_rethook_prepare(struct rethook_node *rhn, struct pt_regs *regs, bool mcount)
> {
> rhn->ret_addr = regs->ra;
> rhn->frame = regs->s0;
> diff --git a/arch/s390/kernel/rethook.c b/arch/s390/kernel/rethook.c
> index af10e6bdd34e..ad52119826c1 100644
> --- a/arch/s390/kernel/rethook.c
> +++ b/arch/s390/kernel/rethook.c
> @@ -3,7 +3,7 @@
> #include <linux/kprobes.h>
> #include "rethook.h"
>
> -void arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mcount)
> +void notrace arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mcount)
> {
> rh->ret_addr = regs->gprs[14];
> rh->frame = regs->gprs[15];
> @@ -13,7 +13,7 @@ void arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mc
> }
> NOKPROBE_SYMBOL(arch_rethook_prepare);
>
> -void arch_rethook_fixup_return(struct pt_regs *regs,
> +void notrace arch_rethook_fixup_return(struct pt_regs *regs,
> unsigned long correct_ret_addr)
> {
> /* Replace fake return address with real one. */
> @@ -24,7 +24,7 @@ NOKPROBE_SYMBOL(arch_rethook_fixup_return);
> /*
> * Called from arch_rethook_trampoline
> */
> -unsigned long arch_rethook_trampoline_callback(struct pt_regs *regs)
> +unsigned long notrace arch_rethook_trampoline_callback(struct pt_regs *regs)
> {
> return rethook_trampoline_handler(regs, regs->gprs[15]);
> }
> diff --git a/arch/x86/kernel/rethook.c b/arch/x86/kernel/rethook.c
> index 8a1c0111ae79..1f7cef86f73d 100644
> --- a/arch/x86/kernel/rethook.c
> +++ b/arch/x86/kernel/rethook.c
> @@ -64,7 +64,8 @@ NOKPROBE_SYMBOL(arch_rethook_trampoline);
> /*
> * Called from arch_rethook_trampoline
> */
> -__used __visible void arch_rethook_trampoline_callback(struct pt_regs *regs)
> +__used __visible void notrace arch_rethook_trampoline_callback(struct pt_regs
> + *regs)
> {
> unsigned long *frame_pointer;
>
> @@ -104,7 +105,7 @@ NOKPROBE_SYMBOL(arch_rethook_trampoline_callback);
> STACK_FRAME_NON_STANDARD_FP(arch_rethook_trampoline);
>
> /* This is called from rethook_trampoline_handler(). */
> -void arch_rethook_fixup_return(struct pt_regs *regs,
> +void notrace arch_rethook_fixup_return(struct pt_regs *regs,
> unsigned long correct_ret_addr)
> {
> unsigned long *frame_pointer = (void *)(regs + 1);
> @@ -114,7 +115,8 @@ void arch_rethook_fixup_return(struct pt_regs *regs,
> }
> NOKPROBE_SYMBOL(arch_rethook_fixup_return);
>
> -void arch_rethook_prepare(struct rethook_node *rh, struct pt_regs *regs, bool mcount)
> +void notrace arch_rethook_prepare(struct rethook_node *rh, struct pt_regs
> + *regs, bool mcount)
> {
> unsigned long *stack = (unsigned long *)regs->sp;
>
> diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
> index 60f6cb2b486b..e551e86d3927 100644
> --- a/kernel/trace/rethook.c
> +++ b/kernel/trace/rethook.c
> @@ -127,7 +127,7 @@ static void free_rethook_node_rcu(struct rcu_head *head)
> * Return back the @node to @node::rethook. If the @node::rethook is already
> * marked as freed, this will free the @node.
> */
> -void rethook_recycle(struct rethook_node *node)
> +void notrace rethook_recycle(struct rethook_node *node)
> {
> lockdep_assert_preemption_disabled();
>
> @@ -194,7 +194,7 @@ void rethook_hook(struct rethook_node *node, struct pt_regs *regs, bool mcount)
> NOKPROBE_SYMBOL(rethook_hook);
>
> /* This assumes the 'tsk' is the current task or is not running. */
> -static unsigned long __rethook_find_ret_addr(struct task_struct *tsk,
> +static unsigned long notrace __rethook_find_ret_addr(struct task_struct *tsk,
> struct llist_node **cur)
> {
> struct rethook_node *rh = NULL;
> @@ -256,7 +256,7 @@ unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long frame
> }
> NOKPROBE_SYMBOL(rethook_find_ret_addr);
>
> -void __weak arch_rethook_fixup_return(struct pt_regs *regs,
> +void __weak notrace arch_rethook_fixup_return(struct pt_regs *regs,
> unsigned long correct_ret_addr)
> {
> /*
> @@ -268,7 +268,7 @@ void __weak arch_rethook_fixup_return(struct pt_regs *regs,
> }
>
> /* This function will be called from each arch-defined trampoline. */
> -unsigned long rethook_trampoline_handler(struct pt_regs *regs,
> +unsigned long notrace rethook_trampoline_handler(struct pt_regs *regs,
> unsigned long frame)
> {
> struct llist_node *first, *node = NULL;
> --
> 2.40.1
>
--
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
