Message ID | 163163030719.489837.2236069935502195491.stgit@devnote2 (mailing list archive) |
---|---|
Headers | show |
Series | kprobes: Fix stacktrace with kretprobes on x86 | expand |
On Tue, Sep 14, 2021 at 7:38 AM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > Hello, > > This is the 11th version of the series to fix the stacktrace with kretprobe on x86. > > The previous version is here; > > https://lore.kernel.org/all/162756755600.301564.4957591913842010341.stgit@devnote2/ > > This version is rebased on the latest tip/master branch and includes the kprobe cleanup > series[1][2]. No code change. > > [1] https://lore.kernel.org/bpf/162748615977.59465.13262421617578791515.stgit@devnote2/ > [2] https://lore.kernel.org/linux-csky/20210727133426.2919710-1-punitagrawal@gmail.com/ > > > With this series, unwinder can unwind stack correctly from ftrace as below; > > # cd /sys/kernel/debug/tracing > # echo > trace > # echo 1 > options/sym-offset > # 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 > # cat /sys/kernel/debug/kprobes/list > ffffffff813bedf0 r full_proxy_read+0x0 [FTRACE] > ffffffff812c13e0 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 > # ||| / _-=> migrate-disable > # |||| / delay > # TASK-PID CPU# ||||| TIMESTAMP FUNCTION > # | | | ||||| | | > cat-136 [000] ...1. 14.474966: r_full_proxy_read_0: (vfs_read+0x99/0x190 <- full_proxy_read) > cat-136 [000] ...1. 14.474970: <stack trace> > => kretprobe_trace_func+0x209/0x300 > => kretprobe_dispatcher+0x9d/0xb0 > => __kretprobe_trampoline_handler+0xd4/0x1b0 > => trampoline_handler+0x43/0x60 > => __kretprobe_trampoline+0x2a/0x50 > => vfs_read+0x99/0x190 > => ksys_read+0x68/0xe0 > => do_syscall_64+0x3b/0x90 > => entry_SYSCALL_64_after_hwframe+0x44/0xae > cat-136 [000] ...1. 14.474971: r_vfs_read_0: (ksys_read+0x68/0xe0 <- vfs_read) > > This shows the double return probes (vfs_read() and full_proxy_read()) on the stack > correctly unwinded. (vfs_read() returns to 'ksys_read+0x68' and full_proxy_read() > returns to 'vfs_read+0x99') > > This also 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, and can use > stack_trace_save_regs(). > > You can also get this series from > git://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git kprobes/kretprobe-stackfix-v11 > > > Thank you, > > --- > > Josh Poimboeuf (3): > objtool: Add frame-pointer-specific function ignore > objtool: Ignore unwind hints for ignored functions > x86/kprobes: Add UNWIND_HINT_FUNC on kretprobe_trampoline() > > Masami Hiramatsu (19): > kprobes: treewide: Cleanup the error messages for kprobes > kprobes: Fix coding style issues > kprobes: Use IS_ENABLED() instead of kprobes_built_in() > kprobes: Add assertions for required lock > kprobes: treewide: Use 'kprobe_opcode_t *' for the code address in get_optimized_kprobe() > kprobes: Use bool type for functions which returns boolean value > ia64: kprobes: Fix to pass correct trampoline address to the handler > kprobes: treewide: Replace arch_deref_entry_point() with dereference_symbol_descriptor() > kprobes: treewide: Remove trampoline_address from kretprobe_trampoline_handler() > kprobes: treewide: Make it harder to refer kretprobe_trampoline directly > kprobes: Add kretprobe_find_ret_addr() for searching return address > ARC: Add instruction_pointer_set() API > ia64: Add instruction_pointer_set() API > arm: kprobes: Make space for instruction pointer on stack > kprobes: Enable stacktrace from pt_regs in kretprobe 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 > x86/kprobes: Fixup return address in generic trampoline handler > > Punit Agrawal (5): > kprobes: Do not use local variable when creating debugfs file > kprobes: Use helper to parse boolean input from userspace > kprobe: Simplify prepare_kprobe() by dropping redundant version > csky: ftrace: Drop duplicate implementation of arch_check_ftrace_location() > kprobes: Make arch_check_ftrace_location static > Re-tested with latest BPF selftests and retsnoop tool utilizing kretprobe and capturing stack traces (broken without these changes). Looks good, thank you! Tested-by: Andrii Nakryiko <andriin@kernel.org> > > arch/arc/include/asm/kprobes.h | 2 > arch/arc/include/asm/ptrace.h | 5 > arch/arc/kernel/kprobes.c | 13 - > arch/arm/probes/kprobes/core.c | 15 - > arch/arm/probes/kprobes/opt-arm.c | 7 > arch/arm64/include/asm/kprobes.h | 2 > arch/arm64/kernel/probes/kprobes.c | 10 > arch/arm64/kernel/probes/kprobes_trampoline.S | 4 > arch/csky/include/asm/kprobes.h | 2 > arch/csky/kernel/probes/ftrace.c | 7 > arch/csky/kernel/probes/kprobes.c | 14 - > arch/csky/kernel/probes/kprobes_trampoline.S | 4 > arch/ia64/include/asm/ptrace.h | 5 > arch/ia64/kernel/kprobes.c | 15 - > arch/mips/kernel/kprobes.c | 26 + > arch/parisc/kernel/kprobes.c | 6 > arch/powerpc/include/asm/kprobes.h | 2 > arch/powerpc/kernel/kprobes.c | 29 - > arch/powerpc/kernel/optprobes.c | 8 > arch/powerpc/kernel/stacktrace.c | 2 > arch/riscv/include/asm/kprobes.h | 2 > arch/riscv/kernel/probes/kprobes.c | 15 - > arch/riscv/kernel/probes/kprobes_trampoline.S | 4 > arch/s390/include/asm/kprobes.h | 2 > arch/s390/kernel/kprobes.c | 16 - > arch/s390/kernel/stacktrace.c | 2 > arch/sh/include/asm/kprobes.h | 2 > arch/sh/kernel/kprobes.c | 12 - > arch/sparc/include/asm/kprobes.h | 2 > arch/sparc/kernel/kprobes.c | 12 - > arch/x86/include/asm/kprobes.h | 1 > arch/x86/include/asm/unwind.h | 23 + > arch/x86/include/asm/unwind_hints.h | 5 > arch/x86/kernel/kprobes/core.c | 71 +++- > arch/x86/kernel/kprobes/opt.c | 6 > arch/x86/kernel/unwind_frame.c | 3 > arch/x86/kernel/unwind_guess.c | 3 > arch/x86/kernel/unwind_orc.c | 21 + > include/linux/kprobes.h | 113 ++++-- > include/linux/objtool.h | 12 + > kernel/kprobes.c | 502 ++++++++++++++----------- > kernel/trace/trace_kprobe.c | 2 > kernel/trace/trace_output.c | 17 - > lib/error-inject.c | 3 > tools/include/linux/objtool.h | 12 + > tools/objtool/check.c | 2 > 46 files changed, 607 insertions(+), 436 deletions(-) > > -- > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
Hi Ingo, Can you merge this series to -tip tree since if I understand correctly, all kprobes patches still should be merged via -tip tree. If you don't think so anymore, I would like to handle the kprobe related patches on my tree. Since many kprobes fixes/cleanups have not been merged these months, it seems unhealthy now. Thank you, On Tue, 14 Sep 2021 23:38:27 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > Hello, > > This is the 11th version of the series to fix the stacktrace with kretprobe on x86. > > The previous version is here; > > https://lore.kernel.org/all/162756755600.301564.4957591913842010341.stgit@devnote2/ > > This version is rebased on the latest tip/master branch and includes the kprobe cleanup > series[1][2]. No code change. > > [1] https://lore.kernel.org/bpf/162748615977.59465.13262421617578791515.stgit@devnote2/ > [2] https://lore.kernel.org/linux-csky/20210727133426.2919710-1-punitagrawal@gmail.com/ > > > With this series, unwinder can unwind stack correctly from ftrace as below; > > # cd /sys/kernel/debug/tracing > # echo > trace > # echo 1 > options/sym-offset > # 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 > # cat /sys/kernel/debug/kprobes/list > ffffffff813bedf0 r full_proxy_read+0x0 [FTRACE] > ffffffff812c13e0 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 > # ||| / _-=> migrate-disable > # |||| / delay > # TASK-PID CPU# ||||| TIMESTAMP FUNCTION > # | | | ||||| | | > cat-136 [000] ...1. 14.474966: r_full_proxy_read_0: (vfs_read+0x99/0x190 <- full_proxy_read) > cat-136 [000] ...1. 14.474970: <stack trace> > => kretprobe_trace_func+0x209/0x300 > => kretprobe_dispatcher+0x9d/0xb0 > => __kretprobe_trampoline_handler+0xd4/0x1b0 > => trampoline_handler+0x43/0x60 > => __kretprobe_trampoline+0x2a/0x50 > => vfs_read+0x99/0x190 > => ksys_read+0x68/0xe0 > => do_syscall_64+0x3b/0x90 > => entry_SYSCALL_64_after_hwframe+0x44/0xae > cat-136 [000] ...1. 14.474971: r_vfs_read_0: (ksys_read+0x68/0xe0 <- vfs_read) > > This shows the double return probes (vfs_read() and full_proxy_read()) on the stack > correctly unwinded. (vfs_read() returns to 'ksys_read+0x68' and full_proxy_read() > returns to 'vfs_read+0x99') > > This also 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, and can use > stack_trace_save_regs(). > > You can also get this series from > git://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git kprobes/kretprobe-stackfix-v11 > > > Thank you, > > --- > > Josh Poimboeuf (3): > objtool: Add frame-pointer-specific function ignore > objtool: Ignore unwind hints for ignored functions > x86/kprobes: Add UNWIND_HINT_FUNC on kretprobe_trampoline() > > Masami Hiramatsu (19): > kprobes: treewide: Cleanup the error messages for kprobes > kprobes: Fix coding style issues > kprobes: Use IS_ENABLED() instead of kprobes_built_in() > kprobes: Add assertions for required lock > kprobes: treewide: Use 'kprobe_opcode_t *' for the code address in get_optimized_kprobe() > kprobes: Use bool type for functions which returns boolean value > ia64: kprobes: Fix to pass correct trampoline address to the handler > kprobes: treewide: Replace arch_deref_entry_point() with dereference_symbol_descriptor() > kprobes: treewide: Remove trampoline_address from kretprobe_trampoline_handler() > kprobes: treewide: Make it harder to refer kretprobe_trampoline directly > kprobes: Add kretprobe_find_ret_addr() for searching return address > ARC: Add instruction_pointer_set() API > ia64: Add instruction_pointer_set() API > arm: kprobes: Make space for instruction pointer on stack > kprobes: Enable stacktrace from pt_regs in kretprobe 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 > x86/kprobes: Fixup return address in generic trampoline handler > > Punit Agrawal (5): > kprobes: Do not use local variable when creating debugfs file > kprobes: Use helper to parse boolean input from userspace > kprobe: Simplify prepare_kprobe() by dropping redundant version > csky: ftrace: Drop duplicate implementation of arch_check_ftrace_location() > kprobes: Make arch_check_ftrace_location static > > > arch/arc/include/asm/kprobes.h | 2 > arch/arc/include/asm/ptrace.h | 5 > arch/arc/kernel/kprobes.c | 13 - > arch/arm/probes/kprobes/core.c | 15 - > arch/arm/probes/kprobes/opt-arm.c | 7 > arch/arm64/include/asm/kprobes.h | 2 > arch/arm64/kernel/probes/kprobes.c | 10 > arch/arm64/kernel/probes/kprobes_trampoline.S | 4 > arch/csky/include/asm/kprobes.h | 2 > arch/csky/kernel/probes/ftrace.c | 7 > arch/csky/kernel/probes/kprobes.c | 14 - > arch/csky/kernel/probes/kprobes_trampoline.S | 4 > arch/ia64/include/asm/ptrace.h | 5 > arch/ia64/kernel/kprobes.c | 15 - > arch/mips/kernel/kprobes.c | 26 + > arch/parisc/kernel/kprobes.c | 6 > arch/powerpc/include/asm/kprobes.h | 2 > arch/powerpc/kernel/kprobes.c | 29 - > arch/powerpc/kernel/optprobes.c | 8 > arch/powerpc/kernel/stacktrace.c | 2 > arch/riscv/include/asm/kprobes.h | 2 > arch/riscv/kernel/probes/kprobes.c | 15 - > arch/riscv/kernel/probes/kprobes_trampoline.S | 4 > arch/s390/include/asm/kprobes.h | 2 > arch/s390/kernel/kprobes.c | 16 - > arch/s390/kernel/stacktrace.c | 2 > arch/sh/include/asm/kprobes.h | 2 > arch/sh/kernel/kprobes.c | 12 - > arch/sparc/include/asm/kprobes.h | 2 > arch/sparc/kernel/kprobes.c | 12 - > arch/x86/include/asm/kprobes.h | 1 > arch/x86/include/asm/unwind.h | 23 + > arch/x86/include/asm/unwind_hints.h | 5 > arch/x86/kernel/kprobes/core.c | 71 +++- > arch/x86/kernel/kprobes/opt.c | 6 > arch/x86/kernel/unwind_frame.c | 3 > arch/x86/kernel/unwind_guess.c | 3 > arch/x86/kernel/unwind_orc.c | 21 + > include/linux/kprobes.h | 113 ++++-- > include/linux/objtool.h | 12 + > kernel/kprobes.c | 502 ++++++++++++++----------- > kernel/trace/trace_kprobe.c | 2 > kernel/trace/trace_output.c | 17 - > lib/error-inject.c | 3 > tools/include/linux/objtool.h | 12 + > tools/objtool/check.c | 2 > 46 files changed, 607 insertions(+), 436 deletions(-) > > -- > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
On Tue, Sep 28, 2021 at 7:24 PM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > Hi Ingo, > > Can you merge this series to -tip tree since if I understand correctly, > all kprobes patches still should be merged via -tip tree. > If you don't think so anymore, I would like to handle the kprobe related > patches on my tree. Since many kprobes fixes/cleanups have not been > merged these months, it seems unhealthy now. > > Thank you, Linus, please suggest how to move these patches forward. We've been waiting for this fix for months now. > On Tue, 14 Sep 2021 23:38:27 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > Hello, > > > > This is the 11th version of the series to fix the stacktrace with kretprobe on x86. > > > > The previous version is here; > > > > https://lore.kernel.org/all/162756755600.301564.4957591913842010341.stgit@devnote2/ > > > > This version is rebased on the latest tip/master branch and includes the kprobe cleanup > > series[1][2]. No code change. > > > > [1] https://lore.kernel.org/bpf/162748615977.59465.13262421617578791515.stgit@devnote2/ > > [2] https://lore.kernel.org/linux-csky/20210727133426.2919710-1-punitagrawal@gmail.com/ > > > > > > With this series, unwinder can unwind stack correctly from ftrace as below;
On Thu, Sep 30 2021 at 11:17, Alexei Starovoitov wrote: > On Tue, Sep 28, 2021 at 7:24 PM Masami Hiramatsu <mhiramat@kernel.org> wrote: >> >> Hi Ingo, >> >> Can you merge this series to -tip tree since if I understand correctly, >> all kprobes patches still should be merged via -tip tree. >> If you don't think so anymore, I would like to handle the kprobe related >> patches on my tree. Since many kprobes fixes/cleanups have not been >> merged these months, it seems unhealthy now. >> >> Thank you, > > Linus, > > please suggest how to move these patches forward. > We've been waiting for this fix for months now. Sorry, I've not paying attention to those as they are usually handled by Ingo who seems to be lost in space. Masami, feel free to merge them over your tree. If not, let me know and I'll pick them up tomorrow morning. Thanks, tglx
On Thu, 30 Sep 2021 21:34:11 +0200 Thomas Gleixner <tglx@linutronix.de> wrote: > Masami, feel free to merge them over your tree. If not, let me know and > I'll pick them up tomorrow morning. Masami usually goes through my tree. Want me to take it or do you want to? -- Steve
On Thu, Sep 30 2021 at 17:22, Steven Rostedt wrote: > On Thu, 30 Sep 2021 21:34:11 +0200 > Thomas Gleixner <tglx@linutronix.de> wrote: > >> Masami, feel free to merge them over your tree. If not, let me know and >> I'll pick them up tomorrow morning. > > Masami usually goes through my tree. Want me to take it or do you want > to? Now I'm really confused. Masami poke Ingo to merge stuff which goes usually through your tree !?! But sure, feel free to pick it up. I have enough stuff on my plate already. Thanks, tglx
On Fri, 01 Oct 2021 01:11:12 +0200 Thomas Gleixner <tglx@linutronix.de> wrote: > On Thu, Sep 30 2021 at 17:22, Steven Rostedt wrote: > > On Thu, 30 Sep 2021 21:34:11 +0200 > > Thomas Gleixner <tglx@linutronix.de> wrote: > > > >> Masami, feel free to merge them over your tree. If not, let me know and > >> I'll pick them up tomorrow morning. > > > > Masami usually goes through my tree. Want me to take it or do you want > > to? > > Now I'm really confused. Masami poke Ingo to merge stuff which goes > usually through your tree !?! > > But sure, feel free to pick it up. I have enough stuff on my plate > already. Let me explain how the patches are usually merged. - kernel/kprobes.c related patches go through the tip tree. - kernel/trace/* patches go through the tracing tree. So traditionally(?) I think this series go through the tip tree, but since the biggest user of kprobes is tracing and the kprobes fix now involves tree-wide fixes as you can see in this series, I think it is a good timing to move kprobes to tracing tree. Thank you,
On Fri, 1 Oct 2021 08:27:33 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > Let me explain how the patches are usually merged. > > - kernel/kprobes.c related patches go through the tip tree. > - kernel/trace/* patches go through the tracing tree. And arch/*/kprobe* usually goes through tip as well. > > So traditionally(?) I think this series go through the tip tree, > but since the biggest user of kprobes is tracing and the kprobes fix > now involves tree-wide fixes as you can see in this series, I think > it is a good timing to move kprobes to tracing tree. I'll pick it up and take the burden off of Thomas. Just to confirm, this is for the next merge window, right? -- Steve
On Thu, 30 Sep 2021 21:34:11 +0200 Thomas Gleixner <tglx@linutronix.de> wrote: > On Thu, Sep 30 2021 at 11:17, Alexei Starovoitov wrote: > > > On Tue, Sep 28, 2021 at 7:24 PM Masami Hiramatsu <mhiramat@kernel.org> wrote: > >> > >> Hi Ingo, > >> > >> Can you merge this series to -tip tree since if I understand correctly, > >> all kprobes patches still should be merged via -tip tree. > >> If you don't think so anymore, I would like to handle the kprobe related > >> patches on my tree. Since many kprobes fixes/cleanups have not been > >> merged these months, it seems unhealthy now. > >> > >> Thank you, > > > > Linus, > > > > please suggest how to move these patches forward. > > We've been waiting for this fix for months now. > > Sorry, I've not paying attention to those as they are usually handled by > Ingo who seems to be lost in space. > > Masami, feel free to merge them over your tree. If not, let me know and > I'll pick them up tomorrow morning. Thank you Thomas for your proposal. As I send in the other mail, if Steve can pick this as a part of tracing, I think that will be better. Thanks again, > > Thanks, > > tglx
On Thu, 30 Sep 2021 19:37:08 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 1 Oct 2021 08:27:33 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > Let me explain how the patches are usually merged. > > > > - kernel/kprobes.c related patches go through the tip tree. > > - kernel/trace/* patches go through the tracing tree. > > And arch/*/kprobe* usually goes through tip as well. > > > > > So traditionally(?) I think this series go through the tip tree, > > but since the biggest user of kprobes is tracing and the kprobes fix > > now involves tree-wide fixes as you can see in this series, I think > > it is a good timing to move kprobes to tracing tree. > > I'll pick it up and take the burden off of Thomas. Thank you very much! > > Just to confirm, this is for the next merge window, right? Yes, please push it to the next merge window. Thanks, > > -- Steve