kprobes: Fix stacktrace with kretprobes on x86
Subsystems (18)
Masami Hiramatsu
maintainerv3 cover kprobes: Fix stacktrace with kretprobes on x86
Hello,
Here is the 3rd version of the series to fix the stacktrace with kretprobe on x86.
The previous version is;
https://lore.kernel.org/bpf/161553130371.1038734.7661319550287837734.stgit@devnote2/
Instead of solving generic stacktrace, this version is focusing on x86 stack unwinders. Anyway this introduces the generic APIs for the solution, other architectures can fix their unwinder with that.
In this version, generic stacktrace fixups in [4/11] is dropped. Instead, [4/11] is just providing kretprobe_find_ret_addr(). And previous ORC unwinder fix patch is splitted into [9/11] and [10/11]. [9/11] fixes x86 kretprobe trampoline so that it pushes an additional kretprobe_trampoline at the bottom of stackframe as a fake return address according to Josh's suggestion. And [10/11] fixes all unwinders in x86 (guess, frame, and ORC).
Finally, [11/11] is renewed to check the kretprobe_trampoline address correctly instead of removing all indicator logic, because except for the x86, it will see the kretprobe_trampoline in the stack trace until it is fixed on each architecture.
With this series, unwinder can unwind stack correctly from ftrace as below;
# cd /sys/kernel/debug/tracing
# echo > trace
# echo r vfs_read >> kprobe_events
# echo r full_proxy_read >> kprobe_events
# echo traceoff:1 > events/kprobes/r_vfs_read_0/trigger
# echo stacktrace:1 > events/kprobes/r_full_proxy_read_0/trigger
# echo 1 > events/kprobes/enable
# echo 1 > options/sym-offset
# cat /sys/kernel/debug/kprobes/list
ffffffff8133b740 r full_proxy_read+0x0 [FTRACE]
ffffffff812560b0 r vfs_read+0x0 [FTRACE]
# echo 0 > events/kprobes/enable
# cat trace
# tracer: nop
#
# entries-in-buffer/entries-written: 3/3 #P:8
#
# _-----=> irqs-off
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / delay
# TASK-PID CPU# |||| TIMESTAMP FUNCTION
# | | | |||| | |
<...>-135 [005] ...1 9.422114: r_full_proxy_read_0: (vfs_read+0xab/0x1a0 <- full_proxy_read)
<...>-135 [005] ...1 9.422158: <stack trace>
=> kretprobe_trace_func+0x209/0x2f0
=> kretprobe_dispatcher+0x4a/0x70
=> __kretprobe_trampoline_handler+0xca/0x150
=> trampoline_handler+0x44/0x70
=> kretprobe_trampoline+0x2a/0x50
=> vfs_read+0xab/0x1a0
=> ksys_read+0x5f/0xe0
=> do_syscall_64+0x33/0x40
=> entry_SYSCALL_64_after_hwframe+0x44/0xae
=> 0This shows the double return probes (vfs_read and full_proxy_read) on the stack correctly unwinded. (vfs_read was called from ksys_read+0x5f and full_proxy_read was called from vfs_read+0xab)
This actually changes the kretprobe behavisor a bit, now the instraction pointer in the pt_regs passed to kretprobe user handler is correctly set the real return address. So user handlers can get it via instruction_pointer() API.
You can also get this series from
git://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git kprobes/kretprobe-stackfix-v3
Thank you,
---
Josh Poimboeuf (1):
x86/kprobes: Add UNWIND_HINT_FUNC on kretprobe_trampoline code
Masami Hiramatsu (10):
ia64: kprobes: Fix to pass correct trampoline address to the handler
kprobes: treewide: Replace arch_deref_entry_point() with dereference_function_descriptor()
kprobes: treewide: Remove trampoline_address from kretprobe_trampoline_handler()
kprobes: Add kretprobe_find_ret_addr() for searching return address
ARC: Add instruction_pointer_set() API
ia64: Add instruction_pointer_set() API
kprobes: Setup instruction pointer in __kretprobe_trampoline_handler x86/kprobes: Push a fake return address at kretprobe_trampoline x86/unwind: Recover kretprobe trampoline entry
tracing: Show kretprobe unknown indicator only for kretprobe_trampoline
arch/arc/include/asm/ptrace.h | 5 ++ arch/arc/kernel/kprobes.c | 2 - arch/arm/probes/kprobes/core.c | 3 - arch/arm64/kernel/probes/kprobes.c | 3 - arch/csky/kernel/probes/kprobes.c | 2 - arch/ia64/include/asm/ptrace.h | 8 ++- arch/ia64/kernel/kprobes.c | 15 ++--- arch/mips/kernel/kprobes.c | 3 - arch/parisc/kernel/kprobes.c | 4 + arch/powerpc/kernel/kprobes.c | 13 ----- arch/riscv/kernel/probes/kprobes.c | 2 - arch/s390/kernel/kprobes.c | 2 - arch/sh/kernel/kprobes.c | 2 - arch/sparc/kernel/kprobes.c | 2 - arch/x86/include/asm/kprobes.h | 1 arch/x86/include/asm/unwind.h | 17 ++++++ arch/x86/include/asm/unwind_hints.h | 5 ++ arch/x86/kernel/kprobes/core.c | 30 ++++++++--- arch/x86/kernel/unwind_frame.c | 4 + arch/x86/kernel/unwind_guess.c | 3 - arch/x86/kernel/unwind_orc.c | 6 +- include/linux/kprobes.h | 41 ++++++++++++-- kernel/kprobes.c | 99 ++++++++++++++++++++++++----------- kernel/trace/trace_output.c | 17 +----- lib/error-inject.c | 3 + 25 files changed, 187 insertions(+), 105 deletions(-)
--
Masami Hiramatsu (Linaro) <[email protected]>
Masami Hiramatsu
maintainerPATCH v3 1/11 ia64: kprobes: Fix to pass correct trampoline address to the handler
Commit e792ff804f49 ("ia64: kprobes: Use generic kretprobe trampoline handler") missed to pass the wrong trampoline address (it passes the descriptor address instead of function entry address).
This fixes it to pass correct trampoline address to __kretprobe_trampoline_handler(). This also changes to use correct symbol dereference function to get the function address from the kretprobe_trampoline.
--- arch/ia64/kernel/kprobes.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
arch/ia64/kernel/kprobes.c +5 -3
@@ -398,7 +398,8 @@ static void kretprobe_trampoline(void)int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { - regs->cr_iip =__kretprobe_trampoline_handler (regs, kretprobe_trampoline,NULL ); + regs->cr_iip =__kretprobe_trampoline_handler (regs, +dereference_function_descriptor (kretprobe_trampoline),NULL );/* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler
@@ -414,7 +415,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, ri->fp =NULL ;/* Replace the return addr with trampoline addr */ - regs->b0 = ((struct fnptr *)kretprobe_trampoline)->ip; + regs->b0 = (unsigned long )dereference_function_descriptor (kretprobe_trampoline); }/* Check the instruction in the slot is break */
@@ -918,14 +919,14 @@ static struct kprobe trampoline_p = {int __init arch_init_kprobes (void ) { trampoline_p.addr - (kprobe_opcode_t *)((struct fnptr *)kretprobe_trampoline)->ip; +dereference_function_description (kretprobe_trampoline);return register_kprobe (&trampoline_p); }int __kprobes arch_trampoline_kprobe (struct kprobe *p) {if (p->addr = - (kprobe_opcode_t *)((struct fnptr *)kretprobe_trampoline)->ip) +dereference_function_descriptor (kretprobe_trampoline))return 1 ;return 0 ;
Masami Hiramatsu
maintainerPATCH v3 2/11 kprobes: treewide: Replace arch_deref_entry_point() with dereference_function_descriptor()
In reply to Masami HiramatsuReplace arch_deref_entry_point() with dereference_function_descriptor() because those are doing same thing.
--- arch/ia64/kernel/kprobes.c | 5 ----- arch/powerpc/kernel/kprobes.c | 11 ----------- include/linux/kprobes.h | 1 - kernel/kprobes.c | 7 +------ lib/error-inject.c | 3 ++- 5 files changed, 3 insertions(+), 24 deletions(-)
arch/ia64/kernel/kprobes.c +0 -5
@@ -907,11 +907,6 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,return ret; } -unsigned long arch_deref_entry_point (void *entry) -{ -return ((struct fnptr *)entry)->ip; -} -static struct kprobe trampoline_p = { .pre_handler = trampoline_probe_handler };
arch/powerpc/kernel/kprobes.c +0 -11
@@ -539,17 +539,6 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr) }NOKPROBE_SYMBOL (kprobe_fault_handler); -unsigned long arch_deref_entry_point (void *entry) -{ -#ifdef PPC64_ELF_ABI_v1 -if (!kernel_text_address ((unsigned long )entry)) -return ppc_global_function_entry (entry); - else -#endif -return (unsigned long )entry; -} -NOKPROBE_SYMBOL (arch_deref_entry_point); -static struct kprobe trampoline_p = { .addr = (kprobe_opcode_t *) &kretprobe_trampoline, .pre_handler = trampoline_probe_handler
include/linux/kprobes.h +0 -1
@@ -390,7 +390,6 @@ int register_kprobe(struct kprobe *p);void unregister_kprobe (struct kprobe *p);int register_kprobes (struct kprobe **kps,int num);void unregister_kprobes (struct kprobe **kps,int num); -unsigned long arch_deref_entry_point (void *);int register_kretprobe (struct kretprobe *rp);void unregister_kretprobe (struct kretprobe *rp);
kernel/kprobes.c +1 -6
@@ -1856,11 +1856,6 @@ static struct notifier_block kprobe_exceptions_nb = { .priority = 0x7fffffff /* we need to be notified first */ }; -unsigned long __weak arch_deref_entry_point (void *entry) -{ -return (unsigned long )entry; -} -#ifdef CONFIG_KRETPROBES unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,
@@ -2324,7 +2319,7 @@ static int __init populate_kprobe_blacklist(unsigned long *start,int ret;for (iter = start; iter < end; iter++) { - entry =arch_deref_entry_point ((void *)*iter); + entry = (unsigned long )dereference_function_descriptor ((void *)*iter); ret =kprobe_add_ksym_blacklist (entry);if (ret = -EINVAL )continue ;
lib/error-inject.c +2 -1
@@ -8,6 +8,7 @@#include <linux/mutex.h> #include <linux/list.h> #include <linux/slab.h> +#include <asm/sections.h> /* Whitelist of symbols that can be overridden for error injection. */ static LIST_HEAD (error_injection_list);
@@ -64,7 +65,7 @@ static void populate_error_injection_list(struct error_injection_entry *start,mutex_lock (&ei_mutex);for (iter = start; iter < end; iter++) { - entry =arch_deref_entry_point ((void *)iter->addr); + entry = (unsigned long )dereference_function_descriptor ((void *)iter->addr);if (!kernel_text_address (entry) || !kallsyms_lookup_size_offset (entry, &size, &offset)) {
Masami Hiramatsu
maintainerPATCH v3 3/11 kprobes: treewide: Remove trampoline_address from kretprobe_trampoline_handler()
In reply to Masami HiramatsuRemove trampoline_address from kretprobe_trampoline_handler(). Instead of passing the address, kretprobe_trampoline_handler() can use new kretprobe_trampoline_addr().
---
Changes in v3:
- Remove wrong kretprobe_trampoline declaration from
arch/x86/include/asm/kprobes.h. Changes in v2: - Remove arch_deref_entry_point() from comment. --- arch/arc/kernel/kprobes.c | 2 +- arch/arm/probes/kprobes/core.c | 3 +-- arch/arm64/kernel/probes/kprobes.c | 3 +-- arch/csky/kernel/probes/kprobes.c | 2 +- arch/ia64/kernel/kprobes.c | 5 ++--- arch/mips/kernel/kprobes.c | 3 +-- arch/parisc/kernel/kprobes.c | 4 ++-- arch/powerpc/kernel/kprobes.c | 2 +- arch/riscv/kernel/probes/kprobes.c | 2 +- arch/s390/kernel/kprobes.c | 2 +- arch/sh/kernel/kprobes.c | 2 +- arch/sparc/kernel/kprobes.c | 2 +- arch/x86/include/asm/kprobes.h | 1 - arch/x86/kernel/kprobes/core.c | 2 +- include/linux/kprobes.h | 18 +++++++++++++----- kernel/kprobes.c | 3 +-- 16 files changed, 29 insertions(+), 27 deletions(-)
arch/arc/kernel/kprobes.c +1 -1
@@ -397,7 +397,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,static int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { - regs->ret =__kretprobe_trampoline_handler (regs, &kretprobe_trampoline,NULL ); + regs->ret =__kretprobe_trampoline_handler (regs,NULL );/* By returning a non zero value, we are telling the kprobe handler * that we don't want the post_handler to run
arch/arm/probes/kprobes/core.c +1 -2
@@ -413,8 +413,7 @@ void __naked __kprobes kretprobe_trampoline(void)/* Called from kretprobe_trampoline */ static __used __kprobes void *trampoline_handler (struct pt_regs *regs) { -return (void *)kretprobe_trampoline_handler (regs, &kretprobe_trampoline, - (void *)regs->ARM_fp); +return (void *)kretprobe_trampoline_handler (regs, (void *)regs->ARM_fp); } void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
arch/arm64/kernel/probes/kprobes.c +1 -2
@@ -412,8 +412,7 @@ int __init arch_populate_kprobe_blacklist(void)void __kprobes __used *trampoline_probe_handler (struct pt_regs *regs) { -return (void *)kretprobe_trampoline_handler (regs, &kretprobe_trampoline, - (void *)kernel_stack_pointer (regs)); +return (void *)kretprobe_trampoline_handler (regs, (void *)kernel_stack_pointer (regs)); } void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
arch/csky/kernel/probes/kprobes.c +1 -1
@@ -404,7 +404,7 @@ int __init arch_populate_kprobe_blacklist(void)void __kprobes __used *trampoline_probe_handler (struct pt_regs *regs) { -return (void *)kretprobe_trampoline_handler (regs, &kretprobe_trampoline,NULL ); +return (void *)kretprobe_trampoline_handler (regs,NULL ); } void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
arch/ia64/kernel/kprobes.c +2 -3
@@ -392,14 +392,13 @@ static void __kprobes set_current_kprobe(struct kprobe *p,__this_cpu_write (current_kprobe, p); } -static void kretprobe_trampoline (void ) +void kretprobe_trampoline (void ) { }int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { - regs->cr_iip =__kretprobe_trampoline_handler (regs, -dereference_function_descriptor (kretprobe_trampoline),NULL ); + regs->cr_iip =__kretprobe_trampoline_handler (regs,NULL );/* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler
arch/mips/kernel/kprobes.c +1 -2
@@ -489,8 +489,7 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,static int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { -instruction_pointer (regs) =__kretprobe_trampoline_handler (regs, - kretprobe_trampoline,NULL ); +instruction_pointer (regs) =__kretprobe_trampoline_handler (regs,NULL );/* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler
arch/parisc/kernel/kprobes.c +2 -2
@@ -175,7 +175,7 @@ int __kprobes parisc_kprobe_ss_handler(struct pt_regs *regs)return 1 ; } -static inline void kretprobe_trampoline (void ) +void kretprobe_trampoline (void ) { asmvolatile ("nop" ); asmvolatile ("nop" );
@@ -193,7 +193,7 @@ static int __kprobes trampoline_probe_handler(struct kprobe *p, {unsigned long orig_ret_address; - orig_ret_address =__kretprobe_trampoline_handler (regs, trampoline_p.addr,NULL ); + orig_ret_address =__kretprobe_trampoline_handler (regs,NULL );instruction_pointer_set (regs, orig_ret_address);return 1 ;
arch/powerpc/kernel/kprobes.c +1 -1
@@ -399,7 +399,7 @@ static int trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) {unsigned long orig_ret_address; - orig_ret_address =__kretprobe_trampoline_handler (regs, &kretprobe_trampoline,NULL ); + orig_ret_address =__kretprobe_trampoline_handler (regs,NULL );/* * We get here through one of two paths: * 1. by taking a trap -> kprobe_handler() -> here
arch/riscv/kernel/probes/kprobes.c +1 -1
@@ -376,7 +376,7 @@ int __init arch_populate_kprobe_blacklist(void)void __kprobes __used *trampoline_probe_handler (struct pt_regs *regs) { -return (void *)kretprobe_trampoline_handler (regs, &kretprobe_trampoline,NULL ); +return (void *)kretprobe_trampoline_handler (regs,NULL ); } void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
arch/s390/kernel/kprobes.c +1 -1
@@ -351,7 +351,7 @@ static void __used kretprobe_trampoline_holder(void)*/ static int trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { - regs->psw.addr =__kretprobe_trampoline_handler (regs, &kretprobe_trampoline,NULL ); + regs->psw.addr =__kretprobe_trampoline_handler (regs,NULL );/* * By returning a non-zero value, we are telling * kprobe_handler() that we don't want the post_handler
arch/sh/kernel/kprobes.c +1 -1
@@ -303,7 +303,7 @@ static void __used kretprobe_trampoline_holder(void)*/ int __kprobes trampoline_probe_handler (struct kprobe *p,struct pt_regs *regs) { - regs->pc =__kretprobe_trampoline_handler (regs, &kretprobe_trampoline,NULL ); + regs->pc =__kretprobe_trampoline_handler (regs,NULL );return 1 ; }
arch/sparc/kernel/kprobes.c +1 -1
@@ -468,7 +468,7 @@ static int __kprobes trampoline_probe_handler(struct kprobe *p, {unsigned long orig_ret_address =0 ; - orig_ret_address =__kretprobe_trampoline_handler (regs, &kretprobe_trampoline,NULL ); + orig_ret_address =__kretprobe_trampoline_handler (regs,NULL ); regs->tpc = orig_ret_address; regs->tnpc = orig_ret_address +4 ;
arch/x86/include/asm/kprobes.h +0 -1
@@ -49,7 +49,6 @@ extern __visible kprobe_opcode_t optprobe_template_end[];extern const int kretprobe_blacklist_size;void arch_remove_kprobe (struct kprobe *p); -asmlinkage void kretprobe_trampoline(void);extern void arch_kprobe_override_function (struct pt_regs *regs);
arch/x86/kernel/kprobes/core.c +1 -1
@@ -811,7 +811,7 @@ __used __visible void *trampoline_handler(struct pt_regs *regs) regs->ip = (unsigned long )&kretprobe_trampoline; regs->orig_ax = ~0UL ; -return (void *)kretprobe_trampoline_handler (regs, &kretprobe_trampoline, ®s->sp); +return (void *)kretprobe_trampoline_handler (regs, ®s->sp); }NOKPROBE_SYMBOL (trampoline_handler);
include/linux/kprobes.h +13 -5
@@ -205,15 +205,23 @@ extern void arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs);extern int arch_trampoline_kprobe (struct kprobe *p); +void kretprobe_trampoline (void ); +/* +* Since some architecture uses structured function pointer, +* use dereference_function_descriptor() to get real function address. +*/ +static nokprobe_inline void *kretprobe_trampoline_addr(void) +{ +return dereference_function_descriptor (kretprobe_trampoline); +} +/* If the trampoline handler called from a kprobe, use this version */ unsigned long __kretprobe_trampoline_handler (struct pt_regs *regs, -void *trampoline_address, -void *frame_pointer); +void *frame_pointer);static nokprobe_inline unsigned long kretprobe_trampoline_handler (struct pt_regs *regs, -void *trampoline_address, -void *frame_pointer) +void *frame_pointer) {unsigned long ret;/*
@@ -222,7 +230,7 @@ unsigned long kretprobe_trampoline_handler(struct pt_regs *regs,* be running at this point. */ kprobe_busy_begin (); - ret =__kretprobe_trampoline_handler (regs, trampoline_address, frame_pointer); + ret =__kretprobe_trampoline_handler (regs, frame_pointer);kprobe_busy_end ();return ret;
kernel/kprobes.c +1 -2
@@ -1859,7 +1859,6 @@ static struct notifier_block kprobe_exceptions_nb = {#ifdef CONFIG_KRETPROBES unsigned long __kretprobe_trampoline_handler (struct pt_regs *regs, -void *trampoline_address,void *frame_pointer) {kprobe_opcode_t *correct_ret_addr =NULL ;
@@ -1874,7 +1873,7 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,BUG_ON (ri->fp != frame_pointer); -if (ri->ret_addr != trampoline_address) { +if (ri->ret_addr !=kretprobe_trampoline_addr ()) { correct_ret_addr = ri->ret_addr;/* * This is the real return address. Any other
Masami Hiramatsu
maintainerPATCH v3 4/11 kprobes: Add kretprobe_find_ret_addr() for searching return address
In reply to Masami HiramatsuAdd kretprobe_find_ret_addr() for searching correct return address from kretprobe instance list.
---
Changes in v3:
- Remove generic stacktrace fixup. Instead, it should be solved in each unwinder. This just provide the generic interface.
Changes in v2:
- Add is_kretprobe_trampoline() for checking address outside of kretprobe_find_ret_addr()
- Remove unneeded addr from kretprobe_find_ret_addr()
- Rename fixup_kretprobe_tramp_addr() to fixup_kretprobe_trampoline()
--- include/linux/kprobes.h | 22 +++++++++++ kernel/kprobes.c | 90 +++++++++++++++++++++++++++++++++-------------- 2 files changed, 86 insertions(+), 26 deletions(-)
include/linux/kprobes.h +22 -0
@@ -215,6 +215,14 @@ static nokprobe_inline void *kretprobe_trampoline_addr(void)return dereference_function_descriptor (kretprobe_trampoline); } +static nokprobe_inline bool is_kretprobe_trampoline(unsigned long addr) +{ +return (void *)addr =kretprobe_trampoline_addr (); +} + +unsigned long kretprobe_find_ret_addr (struct task_struct *tsk,void *fp, +struct llist_node **cur); +/* If the trampoline handler called from a kprobe, use this version */ unsigned long __kretprobe_trampoline_handler (struct pt_regs *regs,void *frame_pointer);
@@ -514,6 +522,20 @@ static inline bool is_kprobe_optinsn_slot(unsigned long addr) }#endif +#if !defined(CONFIG_KRETPROBES ) +static nokprobe_inline bool is_kretprobe_trampoline(unsigned long addr) +{ +return false; +} + +static nokprobe_inline +unsigned long kretprobe_find_ret_addr (struct task_struct *tsk,void *fp, +struct llist_node **cur) +{ +return 0 ; +} +#endif +/* Returns true if kprobes handled the fault */ static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs,unsigned int trap)
kernel/kprobes.c +64 -26
115 lines not on this page · raw message
Masami Hiramatsu
maintainerPATCH v3 5/11 x86/kprobes: Add UNWIND_HINT_FUNC on kretprobe_trampoline code
In reply to Masami HiramatsuFrom: Josh Poimboeuf <[email protected]>
Add UNWIND_HINT_FUNC on kretporbe_trampoline code so that ORC information is generated on the kretprobe_trampoline correctly.
---
[MH] Add patch description.
--- arch/x86/include/asm/unwind_hints.h | 5 +++++ arch/x86/kernel/kprobes/core.c | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-)
arch/x86/include/asm/unwind_hints.h +5 -0
@@ -52,6 +52,11 @@ UNWIND_HINT sp_reg=ORC_REG_SP sp_offset=8 type=UNWIND_HINT_TYPE_FUNC .endm +#else + +#define UNWIND_HINT_FUNC \ + UNWIND_HINT(ORC_REG_SP, 8, UNWIND_HINT_TYPE_FUNC, 0) +#endif /* __ASSEMBLY__ */ #endif /* _ASM_X86_UNWIND_HINTS_H */
arch/x86/kernel/kprobes/core.c +2 -1
@@ -772,6 +772,7 @@ asm(/* We don't bother saving the ss register */ #ifdef CONFIG_X86_64 " pushq %rsp\n" +UNWIND_HINT_FUNC " pushfq\n" SAVE_REGS_STRING " movq %rsp, %rdi\n"
@@ -782,6 +783,7 @@ asm(" popfq\n" #else" pushl %esp\n" +UNWIND_HINT_FUNC " pushfl\n" SAVE_REGS_STRING " movl %esp, %eax\n"
@@ -795,7 +797,6 @@ asm(".size kretprobe_trampoline, .-kretprobe_trampoline\n" );NOKPROBE_SYMBOL (kretprobe_trampoline); -STACK_FRAME_NON_STANDARD (kretprobe_trampoline);/*
Masami Hiramatsu
maintainerRe: [PATCH -tip v3 05/11] x86/kprobes: Add UNWIND_HINT_FUNC on kretprobe_trampoline code
Quoted 5 lines · On Fri, 19 Mar 2021 21:22:39 +0900 Masami Hiramatsu <[email protected]> wrote:
From: Josh Poimboeuf <[email protected]>
Add
UNWIND_HINT_FUNConkretporbe_trampolinecode so that ORC information is generated on thekretprobe_trampolinecorrectly.
Test bot also found a new warning for this.
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/x86/kernel/kprobes/core.o: warning: objtool: kretprobe_trampoline()+0x25: call without frame pointer save/setup
With CONFIG_FRAME_POINTER=y.
Of course this can be fixed with additional "push %bp; mov %sp, %bp" before calling trampoline_handler. But actually we know that this function has a bit special stack frame too.
Can I recover STACK_FRAME_NON_STANDARD(kretprobe_trampoline) when CONFIG_FRAME_POINTER=y ?
Thank you,
Quoted 53 lines
Signed-off-by Josh Poimboeuf <[email protected]>---
[MH] Add patch description.--- arch/x86/include/asm/unwind_hints.h | 5 +++++ arch/x86/kernel/kprobes/core.c | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-)arch/x86/include/asm/unwind_hints.h +5
-0@@ -52,6 +52,11 @@ UNWIND_HINT sp_reg=ORC_REG_SP sp_offset=8 type=UNWIND_HINT_TYPE_FUNC .endm +#else + +#define UNWIND_HINT_FUNC \ + UNWIND_HINT(ORC_REG_SP, 8, UNWIND_HINT_TYPE_FUNC, 0) +#endif /* __ASSEMBLY__ */ #endif /* _ASM_X86_UNWIND_HINTS_H */ arch/x86/kernel/kprobes/core.c +2
-1@@ -772,6 +772,7 @@ asm(/* We don't bother saving the ss register */ #ifdef CONFIG_X86_64 " pushq %rsp\n" +UNWIND_HINT_FUNC " pushfq\n" SAVE_REGS_STRING " movq %rsp, %rdi\n" @@ -782,6 +783,7 @@ asm(" popfq\n" #else" pushl %esp\n" +UNWIND_HINT_FUNC " pushfl\n" SAVE_REGS_STRING " movl %esp, %eax\n" @@ -795,7 +797,6 @@ asm(".size kretprobe_trampoline, .-kretprobe_trampoline\n" );NOKPROBE_SYMBOL (kretprobe_trampoline); -STACK_FRAME_NON_STANDARD (kretprobe_trampoline);/*
Signature · 1 line
Masami Hiramatsu <[email protected]>
Masami Hiramatsu
maintainerRe: [PATCH -tip v3 05/11] x86/kprobes: Add UNWIND_HINT_FUNC on kretprobe_trampoline code
Quoted 13 lines · On Sat, 20 Mar 2021 21:16:16 +0900 Masami Hiramatsu <[email protected]> wrote:
Quoted 5 lines · On Fri, 19 Mar 2021 21:22:39 +0900 Masami Hiramatsu <[email protected]> wrote:
From: Josh Poimboeuf <[email protected]>
Add
UNWIND_HINT_FUNConkretporbe_trampolinecode so that ORC information is generated on thekretprobe_trampolinecorrectly.Test bot also found a new warning for this.
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/x86/kernel/kprobes/core.o: warning: objtool: kretprobe_trampoline()+0x25: call without frame pointer save/setup
Quoted 7 lines
With
CONFIG_FRAME_POINTER=y.Of course this can be fixed with additional "push %bp; mov %sp, %bp" before calling
trampoline_handler. But actually we know that this function has a bit special stack frame too.Can I recover
STACK_FRAME_NON_STANDARD(kretprobe_trampoline)whenCONFIG_FRAME_POINTER=y?
So something like this. Does it work?
arch/x86/kernel/kprobes/core.c +12 -1
@@ -760,6 +760,10 @@ int kprobe_int3_handler(struct pt_regs *regs) }NOKPROBE_SYMBOL (kprobe_int3_handler); +#ifdef CONFIG_FRAME_POINTER +#undef UNWIND_HINT_FUNC +#define UNWIND_HINT_FUNC +#endif /* * When a retprobed function returns, this code saves registers and * calls trampoline_handler() runs, which calls the kretprobe's handler.
@@ -797,7 +801,14 @@ asm(".size kretprobe_trampoline, .-kretprobe_trampoline\n" );NOKPROBE_SYMBOL (kretprobe_trampoline); - +#ifdef CONFIG_FRAME_POINTER +/* +* kretprobe_trampoline skips updating frame pointer. The frame pointer +* saved in trampoline_handler points to the real caller function's +* frame pointer. +*/ +STACK_FRAME_NON_STANDARD (kretprobe_trampoline); +#endif /* * Called from kretprobe_trampoline
Signature · 1 line
Masami Hiramatsu <[email protected]>
Re: [PATCH -tip v3 05/11] x86/kprobes: Add UNWIND_HINT_FUNC on kretprobe_trampoline code
Quoted 31 lines · Masami Hiramatsu
Quoted 13 lines · On Sat, 20 Mar 2021 21:16:16 +0900 Masami Hiramatsu <[email protected]> wrote:
Quoted 5 lines · On Fri, 19 Mar 2021 21:22:39 +0900 Masami Hiramatsu <[email protected]> wrote:
From: Josh Poimboeuf <[email protected]>
Add
UNWIND_HINT_FUNConkretporbe_trampolinecode so that ORC information is generated on thekretprobe_trampolinecorrectly.Test bot also found a new warning for this.
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/x86/kernel/kprobes/core.o: warning: objtool: kretprobe_trampoline()+0x25: call without frame pointer save/setupQuoted 7 lines
With
CONFIG_FRAME_POINTER=y.Of course this can be fixed with additional "push %bp; mov %sp, %bp" before calling
trampoline_handler. But actually we know that this function has a bit special stack frame too.Can I recover
STACK_FRAME_NON_STANDARD(kretprobe_trampoline)whenCONFIG_FRAME_POINTER=y?So something like this. Does it work?
arch/x86/kernel/kprobes/core.c +0
-0
arch/x86/kernel/kprobes/core.c int kprobe_int3_handler(struct pt_regs *regs) · line 766
Quoted 7 lines
arch/x86/kernel/kprobes/core.c +4
-0@@ -760,6 +760,10 @@ int kprobe_int3_handler(struct pt_regs *regs) }NOKPROBE_SYMBOL (kprobe_int3_handler); +#ifdef CONFIG_FRAME_POINTER +#undef UNWIND_HINT_FUNC +#define UNWIND_HINT_FUNC +#endif
This hunk isn't necessary. The unwind hints don't actually have an effect with frame pointers.
Quoted 9 lines
arch/x86/kernel/kprobes/core.c +1
-1/* * When a retprobed function returns, this code saves registers and * calls trampoline_handler() runs, which calls the kretprobe's handler. @@ -797,7 +801,14 @@ asm(".size kretprobe_trampoline, .-kretprobe_trampoline\n" );NOKPROBE_SYMBOL (kretprobe_trampoline); - +#ifdef CONFIG_FRAME_POINTER
arch/x86/kernel/kprobes/core.c asm( · line 814
Quoted 10 lines
arch/x86/kernel/kprobes/core.c +7
-0+/* +* kretprobe_trampoline skips updating frame pointer. The frame pointer +* saved in trampoline_handler points to the real caller function's +* frame pointer. +*/ +STACK_FRAME_NON_STANDARD (kretprobe_trampoline); +#endif /* * Called from kretprobe_trampoline
Ack.
Signature · 1 line
Josh
Masami Hiramatsu
maintainerRe: [PATCH -tip v3 05/11] x86/kprobes: Add UNWIND_HINT_FUNC on kretprobe_trampoline code
Quoted 35 lines · On Sun, 21 Mar 2021 12:52:03 -0500 Josh Poimboeuf <[email protected]> wrote:
Quoted 34 lines · Masami Hiramatsu
Quoted 13 lines · On Sat, 20 Mar 2021 21:16:16 +0900 Masami Hiramatsu <[email protected]> wrote:
Quoted 5 lines · On Fri, 19 Mar 2021 21:22:39 +0900 Masami Hiramatsu <[email protected]> wrote:
From: Josh Poimboeuf <[email protected]>
Add
UNWIND_HINT_FUNConkretporbe_trampolinecode so that ORC information is generated on thekretprobe_trampolinecorrectly.Test bot also found a new warning for this.
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/x86/kernel/kprobes/core.o: warning: objtool: kretprobe_trampoline()+0x25: call without frame pointer save/setupQuoted 7 lines
With
CONFIG_FRAME_POINTER=y.Of course this can be fixed with additional "push %bp; mov %sp, %bp" before calling
trampoline_handler. But actually we know that this function has a bit special stack frame too.Can I recover
STACK_FRAME_NON_STANDARD(kretprobe_trampoline)whenCONFIG_FRAME_POINTER=y?So something like this. Does it work?
arch/x86/kernel/kprobes/core.c +0
-0@@ -760,6 +760,10 @@ int kprobe_int3_handler(struct pt_regs *regs) }NOKPROBE_SYMBOL (kprobe_int3_handler);
Quoted 7 lines
arch/x86/kernel/kprobes/core.c int kprobe_int3_handler(struct pt_regs *regs) · line 766
Quoted 4 lines
arch/x86/kernel/kprobes/core.c +4
-0+#ifdef CONFIG_FRAME_POINTER +#undef UNWIND_HINT_FUNC +#define UNWIND_HINT_FUNC +#endif This hunk isn't necessary. The unwind hints don't actually have an effect with frame pointers.
Without this, objtool shows the following warning with CONFIG_FRAME_POINTER=y.
arch/x86/kernel/kprobes/core.o: warning: objtool: kretprobe_trampoline()+0x1: BUG: why am I validating an ignored function?
It seems to complain about putting UNWIND_HINT on STACK_FRAME_NON_STANDARD function.
Quoted 18 lines
Quoted 18 lines
arch/x86/kernel/kprobes/core.c +8
-1/* * When a retprobed function returns, this code saves registers and * calls trampoline_handler() runs, which calls the kretprobe's handler. @@ -797,7 +801,14 @@ asm(".size kretprobe_trampoline, .-kretprobe_trampoline\n" );NOKPROBE_SYMBOL (kretprobe_trampoline); - +#ifdef CONFIG_FRAME_POINTER +/* +* kretprobe_trampoline skips updating frame pointer. The frame pointer +* saved in trampoline_handler points to the real caller function's +* frame pointer. +*/ +STACK_FRAME_NON_STANDARD (kretprobe_trampoline); +#endif
Quoted 4 lines
arch/x86/kernel/kprobes/core.c asm( · line 814
Quoted 2 lines
arch/x86/kernel/kprobes/core.c +0
-0/* * Called from kretprobe_trampoline Ack.
Thank you!
Quoted 4 lines
Signature · 2 lines
-- Josh
Signature · 1 line
Masami Hiramatsu <[email protected]>
Re: [PATCH -tip v3 05/11] x86/kprobes: Add UNWIND_HINT_FUNC on kretprobe_trampoline code
In reply to Masami HiramatsuQuoted 13 lines · Masami Hiramatsu
Quoted 5 lines · On Fri, 19 Mar 2021 21:22:39 +0900 Masami Hiramatsu <[email protected]> wrote:
From: Josh Poimboeuf <[email protected]>
Add
UNWIND_HINT_FUNConkretporbe_trampolinecode so that ORC information is generated on thekretprobe_trampolinecorrectly.Test bot also found a new warning for this.
Quoted 1 line
Quoted 1 line
Quoted 1 line
arch/x86/kernel/kprobes/core.o: warning: objtool: kretprobe_trampoline()+0x25: call without frame pointer save/setup
Quoted 7 lines
With
CONFIG_FRAME_POINTER=y.Of course this can be fixed with additional "push %bp; mov %sp, %bp" before calling
trampoline_handler. But actually we know that this function has a bit special stack frame too.Can I recover
STACK_FRAME_NON_STANDARD(kretprobe_trampoline)whenCONFIG_FRAME_POINTER=y?
Yes, that's what I'd recommend.
Signature · 1 line
Josh
Masami Hiramatsu
maintainerPATCH v3 6/11 ARC: Add instruction_pointer_set() API
In reply to Masami HiramatsuAdd instruction_pointer_set() API for arc.
--- arch/arc/include/asm/ptrace.h | 5 +++++ 1 file changed, 5 insertions(+)
arch/arc/include/asm/ptrace.h +5 -0
@@ -149,6 +149,11 @@ static inline long regs_return_value(struct pt_regs *regs)return (long )regs->r0; } +static inline void instruction_pointer_set (struct pt_regs *regs, +unsigned long val) +{ +instruction_pointer (regs) = val; +}#endif /* !__ASSEMBLY__ */ #endif /* __ASM_PTRACE_H */
Masami Hiramatsu
maintainerPATCH v3 7/11 ia64: Add instruction_pointer_set() API
In reply to Masami HiramatsuAdd instruction_pointer_set() API for ia64.
--- arch/ia64/include/asm/ptrace.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
arch/ia64/include/asm/ptrace.h +7 -1
@@ -45,6 +45,7 @@#include <asm/current.h> #include <asm/page.h> +# define ia64_psr (regs) ((struct ia64_psr *) &(regs)->cr_ipsr)/* * We use the ia64_psr(regs)->ri to determine which of the three * instructions in bundle (16 bytes) took the sample. Generate
@@ -71,6 +72,12 @@ static inline long regs_return_value(struct pt_regs *regs)return -regs->r8; } +static inline void instruction_pointer_set (struct pt_regs *regs,unsigned long val) +{ +ia64_psr (regs)->ri = (val &0xf ); + regs->cr_iip = (val & ~0xfULL ); +} +/* Conserve space in histogram by encoding slot bits in address * bits 2 and 3 rather than bits 0 and 1. */
@@ -87,7 +94,6 @@ static inline long regs_return_value(struct pt_regs *regs)/* given a pointer to a task_struct, return the user's pt_regs */ # define task_pt_regs (t) (((struct pt_regs *) ((char *) (t) + IA64_STK_OFFSET)) - 1) -# define ia64_psr (regs) ((struct ia64_psr *) &(regs)->cr_ipsr)# define user_mode (regs) (((struct ia64_psr *) &(regs)->cr_ipsr)->cpl != 0)# define user_stack (task,regs) ((long) regs - (long) task = IA64_STK_OFFSET - sizeof(*regs))# define fsys_mode (task,regs) \
Masami Hiramatsu
maintainerRe: [PATCH -tip v3 07/11] ia64: Add instruction_pointer_set() API
Quoted 20 lines · On Fri, 19 Mar 2021 21:23:01 +0900 Masami Hiramatsu <[email protected]> wrote:
Add
instruction_pointer_set()API for ia64.Signed-off-by Masami Hiramatsu <[email protected]>--- arch/ia64/include/asm/ptrace.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)arch/ia64/include/asm/ptrace.h +1
-0@@ -45,6 +45,7 @@#include <asm/current.h> #include <asm/page.h> +# define ia64_psr (regs) ((struct ia64_psr *) &(regs)->cr_ipsr)/* * We use the ia64_psr(regs)->ri to determine which of the three * instructions in bundle (16 bytes) took the sample. Generate
arch/ia64/include/asm/ptrace.h static inline long regs_return_value(struct pt_regs *regs) · line 79
Quoted 8 lines
arch/ia64/include/asm/ptrace.h +5
-0@@ -71,6 +72,12 @@ static inline long regs_return_value(struct pt_regs *regs)return -regs->r8; } +static inline void instruction_pointer_set (struct pt_regs *regs,unsigned long val) +{ +ia64_psr (regs)->ri = (val &0xf ); + regs->cr_iip = (val & ~0xfULL ); +}
Oops, this caused a build error. Thanks for the kernel test bot.
It seems that all code which accessing to the "struct ia64_psr" in asm/ptrace.h has to be a macro, because "struct ia64_psr" is defined in the asm/processor.h which includes asm/ptrace.h (for pt_regs?).
If the code is defined as an inline function, the "struct ia64_psr" is evaluated at that point, and caused build error.
arch/ia64/include/asm/ptrace.h:77:16: error: dereferencing pointer to incomplete type 'struct ia64_psr'
But macro code evaluation is postponed until it is used...
Let me update it.
Thank you,
Signature · 1 line
Masami Hiramatsu <[email protected]>
Masami Hiramatsu
maintainerPATCH v3 8/11 kprobes: Setup instruction pointer in __kretprobe_trampoline_handler
In reply to Masami HiramatsuTo simplify the stacktrace with pt_regs from kretprobe handler, set the correct return address to the instruction pointer in the pt_regs before calling kretprobe handlers.
---
Changes in v3:
- Cast the correct_ret_addr to unsigned long.
--- kernel/kprobes.c | 3 +++ 1 file changed, 3 insertions(+)
kernel/kprobes.c +3 -0
@@ -1914,6 +1914,9 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,BUG_ON (1 ); } +/* Set the instruction pointer to the correct address */ +instruction_pointer_set (regs, (unsigned long )correct_ret_addr); +/* Run them. */ first = current->kretprobe_instances.first;while (first) {
Masami Hiramatsu
maintainerRe: [PATCH -tip v3 08/11] kprobes: Setup instruction pointer in __kretprobe_trampoline_handler
Quoted 4 lines · On Fri, 19 Mar 2021 21:23:12 +0900 Masami Hiramatsu <[email protected]> wrote:
To simplify the stacktrace with
pt_regsfrom kretprobe handler, set the correct return address to the instruction pointer in thept_regsbefore calling kretprobe handlers.
Oops, now I also find this breaks kretprobe for arm. It seems not enough registers stores. I need to fix the arm kprobe code too.
In arch/arm/include/uapi/asm/ptrace.h,
---
#define ARM_pc uregs[15]
---
And in arch/arm/probes/kprobes/core.c,
---
/*
* When a retprobed function returns, trampoline_handler() is called,
* calling the kretprobe's handler. We construct a struct pt_regs to
* give a view of registers r0-r11 to the user return-handler. This is
* not a complete pt_regs structure, but that should be plenty sufficient
* for kretprobe handlers which should normally be interested in r0 only
* anyway.
*/
void __naked __kprobes kretprobe_trampoline(void)
{
__asm__ __volatile__ (
"stmdb sp!, {r0 - r11} \n\t"---
So, changing regs->ARM_pc will break a stack entry. I need to expand it to r15.
Thanks,
Quoted 24 lines
Suggested-by Josh Poimboeuf <[email protected]>Signed-off-by Masami Hiramatsu <[email protected]>---
Changes in v3:
- Cast thecorrect_ret_addrto unsigned long.--- kernel/kprobes.c | 3 +++ 1 file changed, 3 insertions(+)kernel/kprobes.c +3
-0@@ -1914,6 +1914,9 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs,BUG_ON (1 ); } +/* Set the instruction pointer to the correct address */ +instruction_pointer_set (regs, (unsigned long )correct_ret_addr); +/* Run them. */ first = current->kretprobe_instances.first;while (first) {
Signature · 1 line
Masami Hiramatsu <[email protected]>
Masami Hiramatsu
maintainerPATCH v3 9/11 x86/kprobes: Push a fake return address at kretprobe_trampoline
In reply to Masami HiramatsuThis changes x86/kretprobe stack frame on kretprobe_trampoline a bit, which now push the kretprobe_trampoline as a fake return address at the bottom of the stack frame. With this fix, the ORC unwinder will see the kretprobe_trampoline as a return address.
--- arch/x86/kernel/kprobes/core.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-)
arch/x86/kernel/kprobes/core.c +22 -9
@@ -769,28 +769,31 @@ asm( ".global kretprobe_trampoline\n" ".type kretprobe_trampoline, @function\n" "kretprobe_trampoline:\n" -/* We don't bother saving the ss register */ #ifdef CONFIG_X86_64 -" pushq %rsp\n" + /* Push fake return address to tell the unwinder it's a kretprobe */ + " pushq $kretprobe_trampoline\n" UNWIND_HINT_FUNC + /* Save the sp-8, this will be fixed later */ + " pushq %rsp\n" " pushfq\n" SAVE_REGS_STRING " movq %rsp, %rdi\n" " call trampoline_handler\n" -/* Replace saved sp with true return address. */ -" movq %rax, 19*8(%rsp)\n" RESTORE_REGS_STRING + " addq $8, %rsp\n" " popfq\n" #else - " pushl %esp\n" +/* Push fake return address to tell the unwinder it's a kretprobe */ +" pushl $kretprobe_trampoline\n" UNWIND_HINT_FUNC +/* Save the sp-4, this will be fixed later */ +" pushl %esp\n" " pushfl\n" SAVE_REGS_STRING " movl %esp, %eax\n" " call trampoline_handler\n" -/* Replace saved sp with true return address. */ -" movl %eax, 15*4(%esp)\n" RESTORE_REGS_STRING +" addl $4, %esp\n" " popfl\n" #endif " ret\n"
@@ -802,8 +805,10 @@ NOKPROBE_SYMBOL(kretprobe_trampoline);/* * Called from kretprobe_trampoline */ -__used __visible void *trampoline_handler(struct pt_regs *regs) +__used __visible void trampoline_handler(struct pt_regs *regs) { +unsigned long *frame_pointer; +/* fixup registers */ regs->cs = __KERNEL_CS;#ifdef CONFIG_X86_32
@@ -811,8 +816,16 @@ __used __visible void *trampoline_handler(struct pt_regs *regs)#endif regs->ip = (unsigned long )&kretprobe_trampoline; regs->orig_ax = ~0UL ; + regs->sp +=sizeof (long ); + frame_pointer = ((unsigned long *)®s->sp) +1 ; -return (void *)kretprobe_trampoline_handler (regs, ®s->sp); +/* Replace fake return address with real one. */ + *frame_pointer =kretprobe_trampoline_handler (regs, frame_pointer); +/* +* Move flags to sp so that kretprobe_trapmoline can return +* right after popf. +*/ + regs->sp = regs->flags; }NOKPROBE_SYMBOL (trampoline_handler);
Masami Hiramatsu
maintainerPATCH v3 10/11 x86/unwind: Recover kretprobe trampoline entry
In reply to Masami HiramatsuSince the kretprobe replaces the function return address with the kretprobe_trampoline on the stack, x86 unwinders can not continue the stack unwinding at that point, or record kretprobe_trampoline instead of correct return address.
To fix this issue, find the correct return address from task's kretprobe_instances as like as function-graph tracer does.
With this fix, the unwinder can correctly unwind the stack from kretprobe event on x86, as below.
<...>-135 [003] ...1 6.722338: r_full_proxy_read_0: (vfs_read+0xab/0x1a0 <- full_proxy_read)
<...>-135 [003] ...1 6.722377: <stack trace> => kretprobe_trace_func+0x209/0x2f0
=> kretprobe_dispatcher+0x4a/0x70
=> __kretprobe_trampoline_handler+0xca/0x150
=> trampoline_handler+0x44/0x70
=> kretprobe_trampoline+0x2a/0x50
=> vfs_read+0xab/0x1a0
=> ksys_read+0x5f/0xe0
=> do_syscall_64+0x33/0x40
=> entry_SYSCALL_64_after_hwframe+0x44/0xae
---
Changes in v3:
- Split out the kretprobe side patch
- Fix build error when CONFIG_KRETPROBES=n.
Changes in v2:
- Remove kretprobe wrapper functions from unwind_orc.c
- Do not fixup state->ip when unwinding with regs because kretprobe fixup instruction pointer before calling handler.
--- arch/x86/include/asm/unwind.h | 17 +++++++++++++++++ arch/x86/kernel/unwind_frame.c | 4 ++-- arch/x86/kernel/unwind_guess.c | 3 +-- arch/x86/kernel/unwind_orc.c | 6 +++--- 4 files changed, 23 insertions(+), 7 deletions(-)
arch/x86/include/asm/unwind.h +17 -0
@@ -4,6 +4,7 @@#include <linux/sched.h> #include <linux/ftrace.h> +#include <linux/kprobes.h> #include <asm/ptrace.h> #include <asm/stacktrace.h>
@@ -15,6 +16,7 @@ struct unwind_state {unsigned long stack_mask;struct task_struct *task;int graph_idx; +struct llist_node *kr_cur;bool error;#if defined(CONFIG_UNWINDER_ORC )bool signal, full_regs;
@@ -99,6 +101,21 @@ void unwind_module_init(struct module *mod, void *orc_ip, size_t orc_ip_size, void *orc, size_t orc_size) {}#endif +/* Recover the return address modified by instrumentation (e.g. kretprobe) */ +static inline +unsigned long unwind_recover_ret_addr (struct unwind_state *state, +unsigned long addr,unsigned long *addr_p) +{ +unsigned long ret; + + ret =ftrace_graph_ret_addr (state->task, &state->graph_idx, + addr, addr_p); +if (is_kretprobe_trampoline (ret)) + ret =kretprobe_find_ret_addr (state->task, addr_p, + &state->kr_cur); +return ret; +} +/* * This disables KASAN checking when reading a value from another task's stack, * since the other task could be running on another CPU and could have poisoned
arch/x86/kernel/unwind_frame.c +2 -2
@@ -3,6 +3,7 @@#include <linux/sched/task.h> #include <linux/sched/task_stack.h> #include <linux/interrupt.h> +#include <linux/kprobes.h> #include <asm/sections.h> #include <asm/ptrace.h> #include <asm/bitops.h>
@@ -240,8 +241,7 @@ static bool update_stack_state(struct unwind_state *state,else { addr_p =unwind_get_return_address_ptr (state); addr =READ_ONCE_TASK_STACK (state->task, *addr_p); - state->ip =ftrace_graph_ret_addr (state->task, &state->graph_idx, - addr, addr_p); + state->ip =unwind_recover_ret_addr (state, addr, addr_p); }/* Save the original stack pointer for unwind_dump(): */
arch/x86/kernel/unwind_guess.c +1 -2
@@ -15,8 +15,7 @@ unsigned long unwind_get_return_address(struct unwind_state *state) addr =READ_ONCE_NOCHECK (*state->sp); -return ftrace_graph_ret_addr (state->task, &state->graph_idx, - addr, state->sp); +return unwind_recover_ret_addr (state, addr, state->sp); }EXPORT_SYMBOL_GPL (unwind_get_return_address);
arch/x86/kernel/unwind_orc.c +3 -3
@@ -2,6 +2,7 @@#include <linux/objtool.h> #include <linux/module.h> #include <linux/sort.h> +#include <linux/kprobes.h> #include <asm/ptrace.h> #include <asm/stacktrace.h> #include <asm/unwind.h>
@@ -534,9 +535,8 @@ bool unwind_next_frame(struct unwind_state *state)if (!deref_stack_reg (state, ip_p, &state->ip)) goto err; - state->ip =ftrace_graph_ret_addr (state->task, &state->graph_idx, - state->ip, (void *)ip_p); - + state->ip =unwind_recover_ret_addr (state, state->ip, + (unsigned long *)ip_p); state->sp = sp; state->regs =NULL ; state->prev_regs =NULL ;
Masami Hiramatsu
maintainerPATCH v3 11/11 tracing: Show kretprobe unknown indicator only for kretprobe_trampoline
In reply to Masami Hiramatsuftrace shows "[unknown/kretprobe'd]" indicator all addresses in the kretprobe_trampoline, but the modified address by kretprobe should be only kretprobe_trampoline+0.
--- kernel/trace/trace_output.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)
kernel/trace/trace_output.c +4 -13
@@ -8,6 +8,7 @@#include <linux/module.h> #include <linux/mutex.h> #include <linux/ftrace.h> +#include <linux/kprobes.h> #include <linux/sched/clock.h> #include <linux/sched/mm.h>
@@ -346,22 +347,12 @@ int trace_output_call(struct trace_iterator *iter, char *name, char *fmt, ...) }EXPORT_SYMBOL_GPL (trace_output_call); -#ifdef CONFIG_KRETPROBES -static inline const char *kretprobed (const char *name) +static inline const char *kretprobed (const char *name,unsigned long addr) { -static const char tramp_name[] ="kretprobe_trampoline" ; -int size =sizeof (tramp_name); - -if (strncmp (tramp_name, name, size) =0 ) +if (is_kretprobe_trampoline (addr))return "[unknown/kretprobe'd]" ;return name; } -#else -static inline const char *kretprobed (const char *name) -{ -return name; -} -#endif /* CONFIG_KRETPROBES */ void trace_seq_print_sym (struct trace_seq *s,unsigned long address,bool offset)
@@ -374,7 +365,7 @@ trace_seq_print_sym(struct trace_seq *s, unsigned long address, bool offset)sprint_symbol (str, address); elsekallsyms_lookup (address ,NULL ,NULL ,NULL ,str ); - name =kretprobed (str); + name =kretprobed (str, address);if (name &&strlen (name)) {trace_seq_puts (s, name);