Skip to content

Commit cfa7f3d

Browse files
anakryikoPeter Zijlstra
authored and
Peter Zijlstra
committed
perf,x86: avoid missing caller address in stack traces captured in uprobe
When tracing user functions with uprobe functionality, it's common to install the probe (e.g., a BPF program) at the first instruction of the function. This is often going to be `push %rbp` instruction in function preamble, which means that within that function frame pointer hasn't been established yet. This leads to consistently missing an actual caller of the traced function, because perf_callchain_user() only records current IP (capturing traced function) and then following frame pointer chain (which would be caller's frame, containing the address of caller's caller). So when we have target_1 -> target_2 -> target_3 call chain and we are tracing an entry to target_3, captured stack trace will report target_1 -> target_3 call chain, which is wrong and confusing. This patch proposes a x86-64-specific heuristic to detect `push %rbp` (`push %ebp` on 32-bit architecture) instruction being traced. Given entire kernel implementation of user space stack trace capturing works under assumption that user space code was compiled with frame pointer register (%rbp/%ebp) preservation, it seems pretty reasonable to use this instruction as a strong indicator that this is the entry to the function. In that case, return address is still pointed to by %rsp/%esp, so we fetch it and add to stack trace before proceeding to unwind the rest using frame pointer-based logic. We also check for `endbr64` (for 64-bit modes) as another common pattern for function entry, as suggested by Josh Poimboeuf. Even if we get this wrong sometimes for uprobes attached not at the function entry, it's OK because stack trace will still be overall meaningful, just with one extra bogus entry. If we don't detect this, we end up with guaranteed to be missing caller function entry in the stack trace, which is worse overall. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20240729175223.23914-1-andrii@kernel.org
1 parent 7e8b255 commit cfa7f3d

File tree

3 files changed

+67
-0
lines changed

3 files changed

+67
-0
lines changed

arch/x86/events/core.c

+63
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
#include <asm/desc.h>
4242
#include <asm/ldt.h>
4343
#include <asm/unwind.h>
44+
#include <asm/uprobes.h>
45+
#include <asm/ibt.h>
4446

4547
#include "perf_event.h"
4648

@@ -2814,6 +2816,46 @@ static unsigned long get_segment_base(unsigned int segment)
28142816
return get_desc_base(desc);
28152817
}
28162818

2819+
#ifdef CONFIG_UPROBES
2820+
/*
2821+
* Heuristic-based check if uprobe is installed at the function entry.
2822+
*
2823+
* Under assumption of user code being compiled with frame pointers,
2824+
* `push %rbp/%ebp` is a good indicator that we indeed are.
2825+
*
2826+
* Similarly, `endbr64` (assuming 64-bit mode) is also a common pattern.
2827+
* If we get this wrong, captured stack trace might have one extra bogus
2828+
* entry, but the rest of stack trace will still be meaningful.
2829+
*/
2830+
static bool is_uprobe_at_func_entry(struct pt_regs *regs)
2831+
{
2832+
struct arch_uprobe *auprobe;
2833+
2834+
if (!current->utask)
2835+
return false;
2836+
2837+
auprobe = current->utask->auprobe;
2838+
if (!auprobe)
2839+
return false;
2840+
2841+
/* push %rbp/%ebp */
2842+
if (auprobe->insn[0] == 0x55)
2843+
return true;
2844+
2845+
/* endbr64 (64-bit only) */
2846+
if (user_64bit_mode(regs) && is_endbr(*(u32 *)auprobe->insn))
2847+
return true;
2848+
2849+
return false;
2850+
}
2851+
2852+
#else
2853+
static bool is_uprobe_at_func_entry(struct pt_regs *regs)
2854+
{
2855+
return false;
2856+
}
2857+
#endif /* CONFIG_UPROBES */
2858+
28172859
#ifdef CONFIG_IA32_EMULATION
28182860

28192861
#include <linux/compat.h>
@@ -2825,6 +2867,7 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent
28252867
unsigned long ss_base, cs_base;
28262868
struct stack_frame_ia32 frame;
28272869
const struct stack_frame_ia32 __user *fp;
2870+
u32 ret_addr;
28282871

28292872
if (user_64bit_mode(regs))
28302873
return 0;
@@ -2834,6 +2877,12 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent
28342877

28352878
fp = compat_ptr(ss_base + regs->bp);
28362879
pagefault_disable();
2880+
2881+
/* see perf_callchain_user() below for why we do this */
2882+
if (is_uprobe_at_func_entry(regs) &&
2883+
!get_user(ret_addr, (const u32 __user *)regs->sp))
2884+
perf_callchain_store(entry, ret_addr);
2885+
28372886
while (entry->nr < entry->max_stack) {
28382887
if (!valid_user_frame(fp, sizeof(frame)))
28392888
break;
@@ -2862,6 +2911,7 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs
28622911
{
28632912
struct stack_frame frame;
28642913
const struct stack_frame __user *fp;
2914+
unsigned long ret_addr;
28652915

28662916
if (perf_guest_state()) {
28672917
/* TODO: We don't support guest os callchain now */
@@ -2885,6 +2935,19 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs
28852935
return;
28862936

28872937
pagefault_disable();
2938+
2939+
/*
2940+
* If we are called from uprobe handler, and we are indeed at the very
2941+
* entry to user function (which is normally a `push %rbp` instruction,
2942+
* under assumption of application being compiled with frame pointers),
2943+
* we should read return address from *regs->sp before proceeding
2944+
* to follow frame pointers, otherwise we'll skip immediate caller
2945+
* as %rbp is not yet setup.
2946+
*/
2947+
if (is_uprobe_at_func_entry(regs) &&
2948+
!get_user(ret_addr, (const unsigned long __user *)regs->sp))
2949+
perf_callchain_store(entry, ret_addr);
2950+
28882951
while (entry->nr < entry->max_stack) {
28892952
if (!valid_user_frame(fp, sizeof(frame)))
28902953
break;

include/linux/uprobes.h

+2
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ struct uprobe_task {
7676
struct uprobe *active_uprobe;
7777
unsigned long xol_vaddr;
7878

79+
struct arch_uprobe *auprobe;
80+
7981
struct return_instance *return_instances;
8082
unsigned int depth;
8183
};

kernel/events/uprobes.c

+2
Original file line numberDiff line numberDiff line change
@@ -2082,6 +2082,7 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs)
20822082
bool need_prep = false; /* prepare return uprobe, when needed */
20832083

20842084
down_read(&uprobe->register_rwsem);
2085+
current->utask->auprobe = &uprobe->arch;
20852086
for (uc = uprobe->consumers; uc; uc = uc->next) {
20862087
int rc = 0;
20872088

@@ -2096,6 +2097,7 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs)
20962097

20972098
remove &= rc;
20982099
}
2100+
current->utask->auprobe = NULL;
20992101

21002102
if (need_prep && !remove)
21012103
prepare_uretprobe(uprobe, regs); /* put bp at return */

0 commit comments

Comments
 (0)