[PATCH 1/4] rethook: use preempt_{disable, enable}_notrace in rethook_trampoline_handler

[PATCH 1/4] rethook: use preempt_{disable, enable}_notrace in rethook_trampoline_handler

[Ze Gao]

This patch replace preempt_{disable, enable} with its corresponding
notrace version in rethook_trampoline_handler so no worries about stack
recursion or overflow introduced by preempt_count_{add, sub} under
fprobe + rethook context.

Signed-off-by: Ze Gao <zegao@tencent.com>
---
 kernel/trace/rethook.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
index 32c3dfdb4d6a..60f6cb2b486b 100644
--- a/kernel/trace/rethook.c
+++ b/kernel/trace/rethook.c
@@ -288,7 +288,7 @@ unsigned long rethook_trampoline_handler(struct pt_regs *regs,
 	 * These loops must be protected from rethook_free_rcu() because those
 	 * are accessing 'rhn->rethook'.
 	 */
-	preempt_disable();
+	preempt_disable_notrace();
 
 	/*
 	 * Run the handler on the shadow stack. Do not unlink the list here because
@@ -321,7 +321,7 @@ unsigned long rethook_trampoline_handler(struct pt_regs *regs,
 		first = first->next;
 		rethook_recycle(rhn);
 	}
-	preempt_enable();
+	preempt_enable_notrace();
 
 	return correct_ret_addr;
 }
-- 
2.40.1

[PATCH 1/4] rethook: use preempt_{disable, enable}_notrace in rethook_trampoline_handler

[Ze Gao]

This patch replace preempt_{disable, enable} with its corresponding
notrace version in rethook_trampoline_handler so no worries about stack
recursion or overflow introduced by preempt_count_{add, sub} under
fprobe + rethook context.

Signed-off-by: Ze Gao <zegao@tencent.com>
---
 kernel/trace/rethook.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
index 32c3dfdb4d6a..60f6cb2b486b 100644
--- a/kernel/trace/rethook.c
+++ b/kernel/trace/rethook.c
@@ -288,7 +288,7 @@ unsigned long rethook_trampoline_handler(struct pt_regs *regs,
 	 * These loops must be protected from rethook_free_rcu() because those
 	 * are accessing 'rhn->rethook'.
 	 */
-	preempt_disable();
+	preempt_disable_notrace();
 
 	/*
 	 * Run the handler on the shadow stack. Do not unlink the list here because
@@ -321,7 +321,7 @@ unsigned long rethook_trampoline_handler(struct pt_regs *regs,
 		first = first->next;
 		rethook_recycle(rhn);
 	}
-	preempt_enable();
+	preempt_enable_notrace();
 
 	return correct_ret_addr;
 }
-- 
2.40.1

[PATCH 2/4] fprobe: make fprobe_kprobe_handler recursion free

[Ze Gao]

Current implementation calls kprobe related functions before doing
ftrace recursion check in fprobe_kprobe_handler, which opens door
to kernel crash due to stack recursion if preempt_count_{add, sub}
is traceable.

Refactor the common part out of fprobe_kprobe_handler and fprobe_
handler and call ftrace recursion detection at the very beginning,
and also mark these functions notrace so that the whole fprobe_k-
probe_handler is free from recusion. And

Signed-off-by: Ze Gao <zegao@tencent.com>
---
 kernel/trace/fprobe.c | 61 +++++++++++++++++++++++++++++++------------
 1 file changed, 45 insertions(+), 16 deletions(-)

diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 9abb3905bc8e..ad9a36c87ad9 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -20,30 +20,22 @@ struct fprobe_rethook_node {
 	char data[];
 };
 
-static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
-			   struct ftrace_ops *ops, struct ftrace_regs *fregs)
+static inline notrace void __fprobe_handler(unsigned long ip, unsigned long
+		parent_ip, struct ftrace_ops *ops, struct ftrace_regs *fregs)
 {
 	struct fprobe_rethook_node *fpr;
 	struct rethook_node *rh = NULL;
 	struct fprobe *fp;
 	void *entry_data = NULL;
-	int bit, ret;
+	int ret;
 
 	fp = container_of(ops, struct fprobe, ops);
-	if (fprobe_disabled(fp))
-		return;
-
-	bit = ftrace_test_recursion_trylock(ip, parent_ip);
-	if (bit < 0) {
-		fp->nmissed++;
-		return;
-	}
 
 	if (fp->exit_handler) {
 		rh = rethook_try_get(fp->rethook);
 		if (!rh) {
 			fp->nmissed++;
-			goto out;
+			return;
 		}
 		fpr = container_of(rh, struct fprobe_rethook_node, node);
 		fpr->entry_ip = ip;
@@ -61,23 +53,60 @@ static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
 		else
 			rethook_hook(rh, ftrace_get_regs(fregs), true);
 	}
-out:
+}
+
+static void notrace fprobe_handler(unsigned long ip, unsigned long parent_ip,
+			   struct ftrace_ops *ops, struct ftrace_regs *fregs)
+{
+	struct fprobe *fp;
+	int bit;
+
+	fp = container_of(ops, struct fprobe, ops);
+	if (fprobe_disabled(fp))
+		return;
+
+	/* recursion detection has to go before any traceable function and
+	 * all functions before this point should be marked as notrace
+	 */
+	bit = ftrace_test_recursion_trylock(ip, parent_ip);
+	if (bit < 0) {
+		fp->nmissed++;
+		return;
+	}
+	__fprobe_handler(ip, parent_ip, ops, fregs);
 	ftrace_test_recursion_unlock(bit);
+
 }
 NOKPROBE_SYMBOL(fprobe_handler);
 
-static void fprobe_kprobe_handler(unsigned long ip, unsigned long parent_ip,
+static void notrace fprobe_kprobe_handler(unsigned long ip, unsigned long parent_ip,
 				  struct ftrace_ops *ops, struct ftrace_regs *fregs)
 {
-	struct fprobe *fp = container_of(ops, struct fprobe, ops);
+	struct fprobe *fp;
+	int bit;
+
+	fp = container_of(ops, struct fprobe, ops);
+	if (fprobe_disabled(fp))
+		return;
+
+	/* recursion detection has to go before any traceable function and
+	 * all functions called before this point should be marked as notrace
+	 */
+	bit = ftrace_test_recursion_trylock(ip, parent_ip);
+	if (bit < 0) {
+		fp->nmissed++;
+		return;
+	}
 
 	if (unlikely(kprobe_running())) {
 		fp->nmissed++;
 		return;
 	}
+
 	kprobe_busy_begin();
-	fprobe_handler(ip, parent_ip, ops, fregs);
+	__fprobe_handler(ip, parent_ip, ops, fregs);
 	kprobe_busy_end();
+	ftrace_test_recursion_unlock(bit);
 }
 
 static void fprobe_exit_handler(struct rethook_node *rh, void *data,
-- 
2.40.1

[PATCH 3/4] fprobe: add recursion detection in fprobe_exit_handler

[Ze Gao]

fprobe_hander and fprobe_kprobe_handler has guarded ftrace recusion
detection but fprobe_exit_handler has not, which possibly introduce
recurive calls if the fprobe exit callback calls any traceable
functions. Checking in fprobe_hander or fprobe_kprobe_handler
is not enough and misses this case.

So add recusion free guard the same way as fprobe_hander and also
mark fprobe_exit_handler notrace. Since ftrace recursion check does
not employ ips, so here use entry_ip and entry_parent_ip the same as
fprobe_handler.

Signed-off-by: Ze Gao <zegao@tencent.com>
---
 kernel/trace/fprobe.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index ad9a36c87ad9..cf982d4ab142 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -17,6 +17,7 @@
 struct fprobe_rethook_node {
 	struct rethook_node node;
 	unsigned long entry_ip;
+	unsigned long entry_parent_ip;
 	char data[];
 };
 
@@ -39,6 +40,7 @@ static inline notrace void __fprobe_handler(unsigned long ip, unsigned long
 		}
 		fpr = container_of(rh, struct fprobe_rethook_node, node);
 		fpr->entry_ip = ip;
+		fpr->entry_parent_ip = parent_ip;
 		if (fp->entry_data_size)
 			entry_data = fpr->data;
 	}
@@ -109,19 +111,30 @@ static void notrace fprobe_kprobe_handler(unsigned long ip, unsigned long parent
 	ftrace_test_recursion_unlock(bit);
 }
 
-static void fprobe_exit_handler(struct rethook_node *rh, void *data,
+static void notrace fprobe_exit_handler(struct rethook_node *rh, void *data,
 				struct pt_regs *regs)
 {
 	struct fprobe *fp = (struct fprobe *)data;
 	struct fprobe_rethook_node *fpr;
+	int bit;
 
 	if (!fp || fprobe_disabled(fp))
 		return;
 
 	fpr = container_of(rh, struct fprobe_rethook_node, node);
 
+	/* we need to assure no calls to traceable functions in-between the
+	 * end of fprobe_handler and the beginning of fprobe_exit_handler.
+	 */
+	bit = ftrace_test_recursion_trylock(fpr->entry_ip, fpr->entry_parent_ip);
+	if (bit < 0) {
+		fp->nmissed++;
+		return;
+	}
+
 	fp->exit_handler(fp, fpr->entry_ip, regs,
 			 fp->entry_data_size ? (void *)fpr->data : NULL);
+	ftrace_test_recursion_unlock(bit);
 }
 NOKPROBE_SYMBOL(fprobe_exit_handler);
 
-- 
2.40.1

[PATCH 4/4] rehook, fprobe: mark rethook related functions notrace

[Ze Gao]

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@tencent.com>
---
 arch/riscv/kernel/probes/rethook.c | 4 ++--
 arch/s390/kernel/rethook.c         | 6 +++---
 arch/x86/kernel/rethook.c          | 8 +++++---
 kernel/trace/rethook.c             | 8 ++++----
 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

[PATCH 0/4] Make fpobe + rethook immune to recursion

[Ze Gao]

Current fprobe and rethook has some pitfalls and may introduce kernel stack recusion, especially in
massive tracing scenario.

For example, if (DEBUG_PREEMPT | TRACE_PREEMPT_TOGGLE) , preempt_count_{add, sub} can be traced via
ftrace, if we happens to use fprobe + rethook based on ftrace to hook on those functions,
recursion is introduced in functions like rethook_trampoline_handler and leads to kernel crash
because of stack overflow.

Snippets of such bug are like this:
[   56.038709] BUG: #DF stack guard page was hit at 000000000b5b7199 (stack is 00000000f4b5a9b2..00000000af4160ce)
[   56.038713] stack guard page: 0000 [#1] PREEMPT SMP NOPTI
[   56.038715] CPU: 5 PID: 1836 Comm: retsnoop Kdump: loaded Not tainted 6.1.18 #2
[   56.038717] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.1 05/05/2021
[   56.038717] BUG: #DF stack guard page was hit at 0000000069dc65a2 (stack is 000000006b9345c5..00000000a221349b)
[   56.038718] RIP: 0010:ftrace_ops_test+0x1a/0x70
[   56.038721] Code: 89 df e8 79 e2 ff ff e9 6e ff ff ff 0f 1f 40 00 48 81 ec b0 00 00 00 49 89 f1 49 89 f8 31 c0 48 89 e6 b9 16 00 00 00 48 89 f7 <f3> 48 ab 48 85 d2 74 35 49 8b 80 d8 00 00 00 48 8b 40 08 48 89 44
[   56.038722] RSP: 0018:fffffe5a8bba5fa0 EFLAGS: 00010046
[   56.038724] RAX: 0000000000000000 RBX: fffffe5a8bba6090 RCX: 0000000000000016
[   56.038725] RDX: fffffe5a8bba6090 RSI: fffffe5a8bba5fa0 RDI: fffffe5a8bba5fa0
[   56.038726] RBP: ffffffffb7137910 R08: ffff8b967f827c70 R09: ffffffffb7137910
[   56.038727] R10: 0000000000000000 R11: 0000000000000000 R12: fffffe5a8bba6090
[   56.038727] R13: ffffffffb729e2bf R14: ffffffffffffffdf R15: ffff8b967f827c70
[   56.038728] FS:  00007f6592d6ed00(0000) GS:ffff8b977da00000(0000) knlGS:0000000000000000
[   56.038730] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   56.038730] CR2: fffffe5a8bba5f98 CR3: 000000010ed94002 CR4: 00000000003726e0
[   56.038733] Call Trace:
[   56.038735]  <#DF>
[   56.038740]  ? exc_int3+0xa/0xc0
[   56.038743]  arch_ftrace_ops_list_func+0xc2/0x190
[   56.038745]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038748]  ftrace_regs_call+0x5/0x52
[   56.038751]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038754]  ? osnoise_arch_unregister+0x210/0x210
[   56.038757]  ? preempt_count_add+0x5/0xa0
[   56.038760]  preempt_count_add+0x5/0xa0
[   56.038762]  rethook_trampoline_handler+0x5f/0x140
[   56.038764]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038766]  arch_rethook_trampoline_callback+0x3b/0x50
[   56.038768]  arch_rethook_trampoline+0x2c/0x60
[   56.038770]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038775]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038778]  osnoise_arch_unregister+0x210/0x210
[   56.038780]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038781]  arch_rethook_trampoline_callback+0x3b/0x50
[   56.038783]  arch_rethook_trampoline+0x2c/0x60
[   56.038785]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038790]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038792]  osnoise_arch_unregister+0x210/0x210
[   56.038794]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038795]  arch_rethook_trampoline_callback+0x3b/0x50
[   56.038797]  arch_rethook_trampoline+0x2c/0x60
[   56.038799]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038804]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038806]  osnoise_arch_unregister+0x210/0x210
[   56.038808]  ? rethook_trampoline_handler+0x5f/0x140
[   56.038810]  arch_rethook_trampoline_callback+0x3b/0x50
[   56.038811]  arch_rethook_trampoline+0x2c/0x60
...
[   56.039133]  ? rethook_trampoline_handler+0x5f/0x140
[   56.039137]  ? rethook_trampoline_handler+0x5f/0x140
[   56.039139]  osnoise_arch_unregister+0x210/0x210
[   56.039141]  ? rethook_trampoline_handler+0x5f/0x140
[   56.039143]  arch_rethook_trampoline_callback+0x3b/0x50
[   56.039144]  arch_rethook_trampoline+0x2c/0x60
[   56.039147]  ? rethook_trampoline_handler+0x5f/0x140
[   56.039151]  ? rethook_trampoline_handler+0x5f/0x140
[   56.039156]  ? vsnprintf+0x2a3/0x550
[   56.039161]  ? sprintf+0x4e/0x60
[   56.039163]  ? kallsyms_lookup_buildid+0x5f/0x130
[   56.039167]  ? __sprint_symbol.constprop.0+0xec/0x110
[   56.039171]  ? symbol_string+0xc5/0x150
[   56.039197]  ? vsnprintf+0x33a/0x550
[   56.039201]  ? exc_int3+0xa/0xc0
[   56.039204]  ? exc_int3+0xa/0xc0
[   56.039205]  ? ftrace_regs_call+0x5/0x52
[   56.039208]  ? ftrace_regs_call+0x5/0x52
[   56.039211]  ? lock_acquire+0x25d/0x2e0
[   56.039214]  ? lock_release+0x208/0x460
[   56.039218]  ? is_bpf_text_address+0x67/0xf0
[   56.039220]  ? kernel_text_address+0x111/0x120
[   56.039223]  ? __kernel_text_address+0xe/0x40
[   56.039225]  ? show_trace_log_lvl+0x1d7/0x336
[   56.039227]  ? show_trace_log_lvl+0x1d7/0x336
[   56.039236]  ? __die_body.cold+0x1a/0x1f
[   56.039239]  ? die+0x2a/0x50
[   56.039242]  ? handle_stack_overflow+0x49/0x60
[   56.039245]  ? exc_double_fault+0x148/0x180
[   56.039248]  ? asm_exc_double_fault+0x1f/0x30
[   56.039251]  ? rethook_trampoline_handler+0x5f/0x140
[   56.039252]  ? cpu_cgroup_css_free+0x30/0x30
[   56.039254]  ? cpu_cgroup_css_free+0x30/0x30
[   56.039258]  ? ftrace_ops_test+0x1a/0x70
[   56.039260]  </#DF>

This bug is found via tool retsnoop which internally uses bpf based on fprobe + rethook
Discussion of this bug can be found here:
  Link: https://lore.kernel.org/bpf/20230510122045.2259-1-zegao@tencent.com/

This patch series fix this problem by adding more recursion detection in each possible entry
functions, and also mark these specific to fprobe or rethook which are beyond the recusion-free
guarded region notrace.


Ze Gao (4):
  rethook: use preempt_{disable, enable}_notrace in
    rethook_trampoline_handler
  fprobe: make fprobe_kprobe_handler recursion free
  fprobe: add recursion detection in fprobe_exit_handler
  rehook, fprobe: mark rethook related functions notrace

 arch/riscv/kernel/probes/rethook.c |  4 +-
 arch/s390/kernel/rethook.c         |  6 +--
 arch/x86/kernel/rethook.c          |  8 ++--
 kernel/trace/fprobe.c              | 76 +++++++++++++++++++++++-------
 kernel/trace/rethook.c             | 12 ++---
 5 files changed, 75 insertions(+), 31 deletions(-)

-- 
2.40.1

Re: [PATCH 2/4] fprobe: make fprobe_kprobe_handler recursion free

[Steven Rostedt]

On Mon, 15 May 2023 11:26:39 +0800
Ze Gao <zegao2021@gmail.com> wrote:

> Current implementation calls kprobe related functions before doing
> ftrace recursion check in fprobe_kprobe_handler, which opens door
> to kernel crash due to stack recursion if preempt_count_{add, sub}
> is traceable.
> 
> Refactor the common part out of fprobe_kprobe_handler and fprobe_
> handler and call ftrace recursion detection at the very beginning,
> and also mark these functions notrace so that the whole fprobe_k-
> probe_handler is free from recusion. And
> 
> Signed-off-by: Ze Gao <zegao@tencent.com>
> ---
>  kernel/trace/fprobe.c | 61 +++++++++++++++++++++++++++++++------------
>  1 file changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 9abb3905bc8e..ad9a36c87ad9 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -20,30 +20,22 @@ struct fprobe_rethook_node {
>  	char data[];
>  };
>  
> -static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
> -			   struct ftrace_ops *ops, struct ftrace_regs *fregs)
> +static inline notrace void __fprobe_handler(unsigned long ip, unsigned long


FYI, if you look in kernel/trace/Makefile you'll see:

ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)

Which removes the flags to add tracing. So there's no reason to add
"notrace" here, as all functions in this directory are by default "notrace".

-- Steve


> +		parent_ip, struct ftrace_ops *ops, struct ftrace_regs *fregs)
>  {
>  	struct fprobe_rethook_node *fpr;
>  	struct rethook_node *rh = NULL;
>  	struct fprobe *fp;
>  	void *entry_data = NULL;
> -	int bit, ret;
> +	int ret;
>  
>  	fp = container_of(ops, struct fprobe, ops);
> -	if (fprobe_disabled(fp))
> -		return;
> -
> -	bit = ftrace_test_recursion_trylock(ip, parent_ip);
> -	if (bit < 0) {
> -		fp->nmissed++;
> -		return;
> -	}
>  
>  	if (fp->exit_handler) {
>  		rh = rethook_try_get(fp->rethook);
>  		if (!rh) {
>  			fp->nmissed++;
> -			goto out;
> +			return;
>  		}
>  		fpr = container_of(rh, struct fprobe_rethook_node, node);
>  		fpr->entry_ip = ip;
> @@ -61,23 +53,60 @@ static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
>  		else
>  			rethook_hook(rh, ftrace_get_regs(fregs), true);
>  	}
>

Re: [PATCH 2/4] fprobe: make fprobe_kprobe_handler recursion free

[Ze Gao]

Hi Steven,

On Tue, May 16, 2023 at 9:25 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon, 15 May 2023 11:26:39 +0800
> Ze Gao <zegao2021@gmail.com> wrote:
>
> > Current implementation calls kprobe related functions before doing
> > ftrace recursion check in fprobe_kprobe_handler, which opens door
> > to kernel crash due to stack recursion if preempt_count_{add, sub}
> > is traceable.
> >
> > Refactor the common part out of fprobe_kprobe_handler and fprobe_
> > handler and call ftrace recursion detection at the very beginning,
> > and also mark these functions notrace so that the whole fprobe_k-
> > probe_handler is free from recusion. And
> >
> > Signed-off-by: Ze Gao <zegao@tencent.com>
> > ---
> >  kernel/trace/fprobe.c | 61 +++++++++++++++++++++++++++++++------------
> >  1 file changed, 45 insertions(+), 16 deletions(-)
> >
> > diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> > index 9abb3905bc8e..ad9a36c87ad9 100644
> > --- a/kernel/trace/fprobe.c
> > +++ b/kernel/trace/fprobe.c
> > @@ -20,30 +20,22 @@ struct fprobe_rethook_node {
> >       char data[];
> >  };
> >
> > -static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
> > -                        struct ftrace_ops *ops, struct ftrace_regs *fregs)
> > +static inline notrace void __fprobe_handler(unsigned long ip, unsigned long
>
>
> FYI, if you look in kernel/trace/Makefile you'll see:
>
> ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
>
> Which removes the flags to add tracing. So there's no reason to add
> "notrace" here, as all functions in this directory are by default "notrace".

Thanks for your valuable info, which I missed before. I'll send v2 to
remove those
unnecessary notrace annotations, and use the same trick for rethook too.

BTW,  I think we can mark rethook routines decls notrace in
include/linux/rethook.h,
which helps to remind developers of other arch(s) this important info.
What do you think of it?

Regards,
Ze

Re: [PATCH 1/4] rethook: use preempt_{disable, enable}_notrace in rethook_trampoline_handler

[Masami Hiramatsu]

Hi Ze Gao,

Thanks for the patch.

On Mon, 15 May 2023 11:26:38 +0800
Ze Gao <zegao2021@gmail.com> wrote:

> This patch replace preempt_{disable, enable} with its corresponding
> notrace version in rethook_trampoline_handler so no worries about stack
> recursion or overflow introduced by preempt_count_{add, sub} under
> fprobe + rethook context.

So, have you ever see that recursion of preempt_count overflow case?

I intended to use the normal preempt_disable() here because it does NOT
prohibit any function-trace call (Note that both kprobes and
fprobe checks recursive call by itself) but it is used for preempt_onoff
tracer.

Thanks,

> 
> Signed-off-by: Ze Gao <zegao@tencent.com>
> ---
>  kernel/trace/rethook.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
> index 32c3dfdb4d6a..60f6cb2b486b 100644
> --- a/kernel/trace/rethook.c
> +++ b/kernel/trace/rethook.c
> @@ -288,7 +288,7 @@ unsigned long rethook_trampoline_handler(struct pt_regs *regs,
>  	 * These loops must be protected from rethook_free_rcu() because those
>  	 * are accessing 'rhn->rethook'.
>  	 */
> -	preempt_disable();
> +	preempt_disable_notrace();
>  
>  	/*
>  	 * Run the handler on the shadow stack. Do not unlink the list here because
> @@ -321,7 +321,7 @@ unsigned long rethook_trampoline_handler(struct pt_regs *regs,
>  		first = first->next;
>  		rethook_recycle(rhn);
>  	}
> -	preempt_enable();
> +	preempt_enable_notrace();
>  
>  	return correct_ret_addr;
>  }
> -- 
> 2.40.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 3/4] fprobe: add recursion detection in fprobe_exit_handler

[Masami Hiramatsu]

On Mon, 15 May 2023 11:26:40 +0800
Ze Gao <zegao2021@gmail.com> wrote:

> fprobe_hander and fprobe_kprobe_handler has guarded ftrace recusion
> detection but fprobe_exit_handler has not, which possibly introduce
> recurive calls if the fprobe exit callback calls any traceable
> functions. Checking in fprobe_hander or fprobe_kprobe_handler
> is not enough and misses this case.

Good catch! Yes, this can fix such recursive call case because if
we put a fprobe to the exit of the "func()", recursive call happens
as below;

func() {
} => rethook 
  => fprobe_exit_handler()
  => fp->exit_handler() {
     func() {
     } => rethook
       => fprobe_exit_handler()
       => fp->exit_handler() {
          func() {
          } => rethook ...

Note that this should not happen with fprobe-based events because
all the code (except for tests) under kernel/trace/ are marked
notrace automatically.

kretprobe avoids this by setting itself to current_kprobe, thus the
other kprobes recursively called from the rethook will be skipped.

> 
> So add recusion free guard the same way as fprobe_hander and also
> mark fprobe_exit_handler notrace. Since ftrace recursion check does
> not employ ips, so here use entry_ip and entry_parent_ip the same as
> fprobe_handler.

Looks good to me.

Fixes: 5b0ab78998e3 ("fprobe: Add exit_handler support")
Cc: stable@vger.kernel.org
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

> 
> Signed-off-by: Ze Gao <zegao@tencent.com>
> ---
>  kernel/trace/fprobe.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index ad9a36c87ad9..cf982d4ab142 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -17,6 +17,7 @@
>  struct fprobe_rethook_node {
>  	struct rethook_node node;
>  	unsigned long entry_ip;
> +	unsigned long entry_parent_ip;
>  	char data[];
>  };
>  
> @@ -39,6 +40,7 @@ static inline notrace void __fprobe_handler(unsigned long ip, unsigned long
>  		}
>  		fpr = container_of(rh, struct fprobe_rethook_node, node);
>  		fpr->entry_ip = ip;
> +		fpr->entry_parent_ip = parent_ip;
>  		if (fp->entry_data_size)
>  			entry_data = fpr->data;
>  	}
> @@ -109,19 +111,30 @@ static void notrace fprobe_kprobe_handler(unsigned long ip, unsigned long parent
>  	ftrace_test_recursion_unlock(bit);
>  }
>  
> -static void fprobe_exit_handler(struct rethook_node *rh, void *data,
> +static void notrace fprobe_exit_handler(struct rethook_node *rh, void *data,
>  				struct pt_regs *regs)
>  {
>  	struct fprobe *fp = (struct fprobe *)data;
>  	struct fprobe_rethook_node *fpr;
> +	int bit;
>  
>  	if (!fp || fprobe_disabled(fp))
>  		return;
>  
>  	fpr = container_of(rh, struct fprobe_rethook_node, node);
>  
> +	/* we need to assure no calls to traceable functions in-between the
> +	 * end of fprobe_handler and the beginning of fprobe_exit_handler.
> +	 */
> +	bit = ftrace_test_recursion_trylock(fpr->entry_ip, fpr->entry_parent_ip);
> +	if (bit < 0) {
> +		fp->nmissed++;
> +		return;
> +	}
> +
>  	fp->exit_handler(fp, fpr->entry_ip, regs,
>  			 fp->entry_data_size ? (void *)fpr->data : NULL);
> +	ftrace_test_recursion_unlock(bit);
>  }
>  NOKPROBE_SYMBOL(fprobe_exit_handler);
>  
> -- 
> 2.40.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 4/4] rehook, fprobe: mark rethook related functions notrace

[Masami Hiramatsu]

On Mon, 15 May 2023 11:26:41 +0800
Ze Gao <zegao2021@gmail.com> 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@tencent.com>
> ---
>  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@kernel.org>

Re: [PATCH 1/4] rethook: use preempt_{disable, enable}_notrace in rethook_trampoline_handler

[Masami Hiramatsu]

On Tue, 16 May 2023 13:25:02 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> Hi Ze Gao,
> 
> Thanks for the patch.
> 
> On Mon, 15 May 2023 11:26:38 +0800
> Ze Gao <zegao2021@gmail.com> wrote:
> 
> > This patch replace preempt_{disable, enable} with its corresponding
> > notrace version in rethook_trampoline_handler so no worries about stack
> > recursion or overflow introduced by preempt_count_{add, sub} under
> > fprobe + rethook context.
> 
> So, have you ever see that recursion of preempt_count overflow case?
> 
> I intended to use the normal preempt_disable() here because it does NOT
> prohibit any function-trace call (Note that both kprobes and
> fprobe checks recursive call by itself) but it is used for preempt_onoff
> tracer.

OK, I got the point.

  rethook_trampoline_handler() {
    preempt_disable() {
      preempt_count_add() { => fprobe and set rethook
      } => rethook_trampoline_handler() {
        preempt_disable() {
          ...   

So the problem is that the preempt_disable() macro calls preempt_count_add()
which can be tracable.

So, let's make it notrace.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

and

Fixes: 54ecbe6f1ed5 ("rethook: Add a generic return hook")
Cc: stable@vger.kernel.org

Thank you,

> 
> Thanks,
> 
> > 
> > Signed-off-by: Ze Gao <zegao@tencent.com>
> > ---
> >  kernel/trace/rethook.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
> > index 32c3dfdb4d6a..60f6cb2b486b 100644
> > --- a/kernel/trace/rethook.c
> > +++ b/kernel/trace/rethook.c
> > @@ -288,7 +288,7 @@ unsigned long rethook_trampoline_handler(struct pt_regs *regs,
> >  	 * These loops must be protected from rethook_free_rcu() because those
> >  	 * are accessing 'rhn->rethook'.
> >  	 */
> > -	preempt_disable();
> > +	preempt_disable_notrace();
> >  
> >  	/*
> >  	 * Run the handler on the shadow stack. Do not unlink the list here because
> > @@ -321,7 +321,7 @@ unsigned long rethook_trampoline_handler(struct pt_regs *regs,
> >  		first = first->next;
> >  		rethook_recycle(rhn);
> >  	}
> > -	preempt_enable();
> > +	preempt_enable_notrace();
> >  
> >  	return correct_ret_addr;
> >  }
> > -- 
> > 2.40.1
> > 
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH 4/4] rehook, fprobe: mark rethook related functions notrace

[Ze Gao]

Hi Masami,

Thanks for your review. I've applied the makefile trick to arch files
specific to rethook just as
mentioned by Steven. And here is the link:

https://lore.kernel.org/linux-trace-kernel/20230516071830.8190-2-zegao@tencent.com/T/#m503e513071e82d5234d80a1b9e15eb126e334608

Unnecessary notrace annotations have been dropped in this new series.

Thank you,
Ze

On Tue, May 16, 2023 at 12:28 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Mon, 15 May 2023 11:26:41 +0800
> Ze Gao <zegao2021@gmail.com> 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@tencent.com>
> > ---
> >  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@kernel.org>