From 2a9237e80dac9df26fecfeda8a4fff62d85b0070 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Tue, 12 Nov 2024 17:54:49 +0000 Subject: [PATCH] Revert "perf,x86: avoid missing caller address in stack traces captured in uprobe" This reverts commit bd0449700389942ee7c4a929ac0f7939e221a9fe which is commit cfa7f3d2c526c224a6271cc78a4a27a0de06f4f0 upstream. It breaks the Android kernel abi and can be brought back in the future in an abi-safe way if it is really needed. Bug: 161946584 Change-Id: Ic71d998534456fcb573ec47c0897698e96bc675d Signed-off-by: Greg Kroah-Hartman --- arch/x86/events/core.c | 63 ----------------------------------------- include/linux/uprobes.h | 2 -- kernel/events/uprobes.c | 2 -- 3 files changed, 67 deletions(-) diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index 39f86188be54..2b2c9fd74ef9 100644 --- a/arch/x86/events/core.c +++ b/arch/x86/events/core.c @@ -41,8 +41,6 @@ #include #include #include -#include -#include #include "perf_event.h" @@ -2843,46 +2841,6 @@ static unsigned long get_segment_base(unsigned int segment) return get_desc_base(desc); } -#ifdef CONFIG_UPROBES -/* - * Heuristic-based check if uprobe is installed at the function entry. - * - * Under assumption of user code being compiled with frame pointers, - * `push %rbp/%ebp` is a good indicator that we indeed are. - * - * Similarly, `endbr64` (assuming 64-bit mode) is also a common pattern. - * If we get this wrong, captured stack trace might have one extra bogus - * entry, but the rest of stack trace will still be meaningful. - */ -static bool is_uprobe_at_func_entry(struct pt_regs *regs) -{ - struct arch_uprobe *auprobe; - - if (!current->utask) - return false; - - auprobe = current->utask->auprobe; - if (!auprobe) - return false; - - /* push %rbp/%ebp */ - if (auprobe->insn[0] == 0x55) - return true; - - /* endbr64 (64-bit only) */ - if (user_64bit_mode(regs) && is_endbr(*(u32 *)auprobe->insn)) - return true; - - return false; -} - -#else -static bool is_uprobe_at_func_entry(struct pt_regs *regs) -{ - return false; -} -#endif /* CONFIG_UPROBES */ - #ifdef CONFIG_IA32_EMULATION #include @@ -2894,7 +2852,6 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent unsigned long ss_base, cs_base; struct stack_frame_ia32 frame; const struct stack_frame_ia32 __user *fp; - u32 ret_addr; if (user_64bit_mode(regs)) return 0; @@ -2904,12 +2861,6 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent fp = compat_ptr(ss_base + regs->bp); pagefault_disable(); - - /* see perf_callchain_user() below for why we do this */ - if (is_uprobe_at_func_entry(regs) && - !get_user(ret_addr, (const u32 __user *)regs->sp)) - perf_callchain_store(entry, ret_addr); - while (entry->nr < entry->max_stack) { if (!valid_user_frame(fp, sizeof(frame))) break; @@ -2938,7 +2889,6 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs { struct stack_frame frame; const struct stack_frame __user *fp; - unsigned long ret_addr; if (perf_guest_state()) { /* TODO: We don't support guest os callchain now */ @@ -2962,19 +2912,6 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs return; pagefault_disable(); - - /* - * If we are called from uprobe handler, and we are indeed at the very - * entry to user function (which is normally a `push %rbp` instruction, - * under assumption of application being compiled with frame pointers), - * we should read return address from *regs->sp before proceeding - * to follow frame pointers, otherwise we'll skip immediate caller - * as %rbp is not yet setup. - */ - if (is_uprobe_at_func_entry(regs) && - !get_user(ret_addr, (const unsigned long __user *)regs->sp)) - perf_callchain_store(entry, ret_addr); - while (entry->nr < entry->max_stack) { if (!valid_user_frame(fp, sizeof(frame))) break; diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h index d91e32aff5a1..f46e0ca0169c 100644 --- a/include/linux/uprobes.h +++ b/include/linux/uprobes.h @@ -76,8 +76,6 @@ struct uprobe_task { struct uprobe *active_uprobe; unsigned long xol_vaddr; - struct arch_uprobe *auprobe; - struct return_instance *return_instances; unsigned int depth; }; diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 33776b61e494..97aa8df24bf2 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -2079,7 +2079,6 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs) bool need_prep = false; /* prepare return uprobe, when needed */ down_read(&uprobe->register_rwsem); - current->utask->auprobe = &uprobe->arch; for (uc = uprobe->consumers; uc; uc = uc->next) { int rc = 0; @@ -2094,7 +2093,6 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs) remove &= rc; } - current->utask->auprobe = NULL; if (need_prep && !remove) prepare_uretprobe(uprobe, regs); /* put bp at return */