[PATCH] x86: kprobes: orc: Fix ORC walks in kretprobes

From: Daniel Xu
Date: Thu Mar 04 2021 - 19:08:36 EST


Getting a stack trace from inside a kretprobe used to work with frame
pointer stack walks. After the default unwinder was switched to ORC,
stack traces broke because ORC did not know how to skip the
`kretprobe_trampoline` "frame".

Frame based stack walks used to work with kretprobes because
`kretprobe_trampoline` does not set up a new call frame. Thus, the frame
pointer based unwinder could walk directly to the kretprobe'd caller.

For example, this stack is walked incorrectly with ORC + kretprobe:

# bpftrace -e 'kretprobe:do_nanosleep { @[kstack] = count() }'
Attaching 1 probe...
^C

@[
kretprobe_trampoline+0
]: 1

After this patch, the stack is walked correctly:

# bpftrace -e 'kretprobe:do_nanosleep { @[kstack] = count() }'
Attaching 1 probe...
^C

@[
kretprobe_trampoline+0
__x64_sys_nanosleep+150
do_syscall_64+51
entry_SYSCALL_64_after_hwframe+68
]: 12

Fixes: fc72ae40e303 ("x86/unwind: Make CONFIG_UNWINDER_ORC=y the default in kconfig for 64-bit")
Signed-off-by: Daniel Xu <dxu@xxxxxxxxx>
---
arch/x86/kernel/unwind_orc.c | 53 +++++++++++++++++++++++++++++++++++-
kernel/kprobes.c | 8 +++---
2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 2a1d47f47eee..1b88d75e2e9e 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -1,7 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/kprobes.h>
#include <linux/objtool.h>
#include <linux/module.h>
#include <linux/sort.h>
+#include <asm/kprobes.h>
#include <asm/ptrace.h>
#include <asm/stacktrace.h>
#include <asm/unwind.h>
@@ -77,9 +79,11 @@ static struct orc_entry *orc_module_find(unsigned long ip)
}
#endif

-#ifdef CONFIG_DYNAMIC_FTRACE
+#if defined(CONFIG_DYNAMIC_FTRACE) || defined(CONFIG_KRETPROBES)
static struct orc_entry *orc_find(unsigned long ip);
+#endif

+#ifdef CONFIG_DYNAMIC_FTRACE
/*
* Ftrace dynamic trampolines do not have orc entries of their own.
* But they are copies of the ftrace entries that are static and
@@ -117,6 +121,43 @@ static struct orc_entry *orc_ftrace_find(unsigned long ip)
}
#endif

+#ifdef CONFIG_KRETPROBES
+static struct orc_entry *orc_kretprobe_find(void)
+{
+ kprobe_opcode_t *correct_ret_addr = NULL;
+ struct kretprobe_instance *ri = NULL;
+ struct llist_node *node;
+
+ node = current->kretprobe_instances.first;
+ while (node) {
+ ri = container_of(node, struct kretprobe_instance, llist);
+
+ if ((void *)ri->ret_addr != &kretprobe_trampoline) {
+ /*
+ * This is the real return address. Any other
+ * instances associated with this task are for
+ * other calls deeper on the call stack
+ */
+ correct_ret_addr = ri->ret_addr;
+ break;
+ }
+
+
+ node = node->next;
+ }
+
+ if (!correct_ret_addr)
+ return NULL;
+
+ return orc_find((unsigned long)correct_ret_addr);
+}
+#else
+static struct orc_entry *orc_kretprobe_find(void)
+{
+ return NULL;
+}
+#endif
+
/*
* If we crash with IP==0, the last successfully executed instruction
* was probably an indirect function call with a NULL function pointer,
@@ -148,6 +189,16 @@ static struct orc_entry *orc_find(unsigned long ip)
if (ip == 0)
return &null_orc_entry;

+ /*
+ * Kretprobe lookup -- must occur before vmlinux addresses as
+ * kretprobe_trampoline is in the symbol table.
+ */
+ if (ip == (unsigned long) &kretprobe_trampoline) {
+ orc = orc_kretprobe_find();
+ if (orc)
+ return orc;
+ }
+
/* For non-init vmlinux addresses, use the fast lookup table: */
if (ip >= LOOKUP_START_IP && ip < LOOKUP_STOP_IP) {
unsigned int idx, start, stop;
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 745f08fdd7a6..334c23d33451 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1895,10 +1895,6 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,
BUG_ON(1);

found:
- /* Unlink all nodes for this frame. */
- current->kretprobe_instances.first = node->next;
- node->next = NULL;
-
/* Run them.. */
while (first) {
ri = container_of(first, struct kretprobe_instance, llist);
@@ -1917,6 +1913,10 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,
recycle_rp_inst(ri);
}

+ /* Unlink all nodes for this frame. */
+ current->kretprobe_instances.first = node->next;
+ node->next = NULL;
+
return (unsigned long)correct_ret_addr;
}
NOKPROBE_SYMBOL(__kretprobe_trampoline_handler)
--
2.30.1