| Message ID | 163477770935.264901.1772964361191833681.stgit@devnote2 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kprobes: Make KUnit and add stacktrace on kretprobe tests | expand |
On Thu, Oct 21, 2021 at 09:55:09AM +0900, Masami Hiramatsu wrote: > Since the kretprobe replaces the function return address with > the kretprobe_trampoline on the stack, stack unwinder shows it > instead of the correct return address. > > This checks whether the next return address is the > __kretprobe_trampoline(), and if so, try to find the correct > return address from the kretprobe instance list. For this purpose > this adds 'kr_cur' loop cursor to memorize the current kretprobe > instance. > > With this fix, now arm64 can enable > CONFIG_ARCH_CORRECT_STACKTRACE_ON_KRETPROBE, and pass the > kprobe self tests. > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > Changes in v2: > - Add comment for kr_cur. > - Make the kretprobe related code depends on CONFIG_KRETPROBES. > - Initialize "kr_cur" directly in start_backtrace() instead > of clearing "frame" data structure by memset(). > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/stacktrace.h | 4 ++++ > arch/arm64/kernel/stacktrace.c | 7 +++++++ > 3 files changed, 12 insertions(+) Acked-by: Will Deacon <will@kernel.org> I'm not sure how you're planning to merge this, so please let me know if you want me to queue any of the arm64 bits. Will
On Thu, 21 Oct 2021 11:15:12 +0100 Will Deacon <will@kernel.org> wrote: > On Thu, Oct 21, 2021 at 09:55:09AM +0900, Masami Hiramatsu wrote: > > Since the kretprobe replaces the function return address with > > the kretprobe_trampoline on the stack, stack unwinder shows it > > instead of the correct return address. > > > > This checks whether the next return address is the > > __kretprobe_trampoline(), and if so, try to find the correct > > return address from the kretprobe instance list. For this purpose > > this adds 'kr_cur' loop cursor to memorize the current kretprobe > > instance. > > > > With this fix, now arm64 can enable > > CONFIG_ARCH_CORRECT_STACKTRACE_ON_KRETPROBE, and pass the > > kprobe self tests. > > > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > --- > > Changes in v2: > > - Add comment for kr_cur. > > - Make the kretprobe related code depends on CONFIG_KRETPROBES. > > - Initialize "kr_cur" directly in start_backtrace() instead > > of clearing "frame" data structure by memset(). > > --- > > arch/arm64/Kconfig | 1 + > > arch/arm64/include/asm/stacktrace.h | 4 ++++ > > arch/arm64/kernel/stacktrace.c | 7 +++++++ > > 3 files changed, 12 insertions(+) > > Acked-by: Will Deacon <will@kernel.org> Thank you! > > I'm not sure how you're planning to merge this, so please let me know if > you want me to queue any of the arm64 bits. Ah, good question. Since this part depends on the first 3 patches and Steve's tracing tree, these should go through the tracing tree. Is that OK for you? (Or, wait for merging the current tracing tree and merge rest of them. but this will take a long time.) Thank you,
On Thu, 21 Oct 2021 23:26:30 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > I'm not sure how you're planning to merge this, so please let me know if > > you want me to queue any of the arm64 bits. > > Ah, good question. Since this part depends on the first 3 patches and > Steve's tracing tree, these should go through the tracing tree. Is that > OK for you? > I'm OK with merging this. > (Or, wait for merging the current tracing tree and merge rest of them. > but this will take a long time.) And my linux-next is behind because my tests triggered a bug on one of my arcane configs, and I'm still debugging it. :-p -- Steve
On Thu, Oct 21, 2021 at 10:49:02AM -0400, Steven Rostedt wrote: > On Thu, 21 Oct 2021 23:26:30 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > > > > I'm not sure how you're planning to merge this, so please let me know if > > > you want me to queue any of the arm64 bits. > > > > Ah, good question. Since this part depends on the first 3 patches and > > Steve's tracing tree, these should go through the tracing tree. Is that > > OK for you? > > > > I'm OK with merging this. Ok, cool. I've acked the arm64 bits so I'll leave it in your hands! > > (Or, wait for merging the current tracing tree and merge rest of them. > > but this will take a long time.) > > And my linux-next is behind because my tests triggered a bug on one of my > arcane configs, and I'm still debugging it. :-p Happy debugging :) Will
On Thu, 21 Oct 2021 17:52:42 +0100 Will Deacon <will@kernel.org> wrote: > > I'm OK with merging this. > > Ok, cool. I've acked the arm64 bits so I'll leave it in your hands! Thanks! I'll pull them in. > > > > (Or, wait for merging the current tracing tree and merge rest of them. > > > but this will take a long time.) > > > > And my linux-next is behind because my tests triggered a bug on one of my > > arcane configs, and I'm still debugging it. :-p > > Happy debugging :) Found the bug. I'll restart my tests (takes around 13 hours more or less to complete) and when/if they succeed, I'll push it for inclusion in linux-next. -- Steve
On Thu, 21 Oct 2021 12:59:43 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > Happy debugging :) > > Found the bug. I'll restart my tests (takes around 13 hours more or less to > complete) and when/if they succeed, I'll push it for inclusion in > linux-next. And of course, more bugs appear. Nothing (yet) to do with this patch series, but as I started running tests I haven't run yet, it's triggering bugs in other places that I need to go sort out :-p -- Steve
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 5c7ae4c3954b..edde5171ffb2 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -11,6 +11,7 @@ config ARM64 select ACPI_PPTT if ACPI select ARCH_HAS_DEBUG_WX select ARCH_BINFMT_ELF_STATE + select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION select ARCH_ENABLE_MEMORY_HOTPLUG select ARCH_ENABLE_MEMORY_HOTREMOVE diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h index 8aebc00c1718..a4e046ef4568 100644 --- a/arch/arm64/include/asm/stacktrace.h +++ b/arch/arm64/include/asm/stacktrace.h @@ -9,6 +9,7 @@ #include <linux/sched.h> #include <linux/sched/task_stack.h> #include <linux/types.h> +#include <linux/llist.h> #include <asm/memory.h> #include <asm/ptrace.h> @@ -59,6 +60,9 @@ struct stackframe { #ifdef CONFIG_FUNCTION_GRAPH_TRACER int graph; #endif +#ifdef CONFIG_KRETPROBES + struct llist_node *kr_cur; +#endif }; extern int unwind_frame(struct task_struct *tsk, struct stackframe *frame); diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c index 8982a2b78acf..c30624fff6ac 100644 --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -41,6 +41,9 @@ void start_backtrace(struct stackframe *frame, unsigned long fp, #ifdef CONFIG_FUNCTION_GRAPH_TRACER frame->graph = 0; #endif +#ifdef CONFIG_KRETPROBES + frame->kr_cur = NULL; +#endif /* * Prime the first unwind. @@ -129,6 +132,10 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame) frame->pc = ret_stack->ret; } #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ +#ifdef CONFIG_KRETPROBES + if (is_kretprobe_trampoline(frame->pc)) + frame->pc = kretprobe_find_ret_addr(tsk, (void *)frame->fp, &frame->kr_cur); +#endif frame->pc = ptrauth_strip_insn_pac(frame->pc);
Since the kretprobe replaces the function return address with the kretprobe_trampoline on the stack, stack unwinder shows it instead of the correct return address. This checks whether the next return address is the __kretprobe_trampoline(), and if so, try to find the correct return address from the kretprobe instance list. For this purpose this adds 'kr_cur' loop cursor to memorize the current kretprobe instance. With this fix, now arm64 can enable CONFIG_ARCH_CORRECT_STACKTRACE_ON_KRETPROBE, and pass the kprobe self tests. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- Changes in v2: - Add comment for kr_cur. - Make the kretprobe related code depends on CONFIG_KRETPROBES. - Initialize "kr_cur" directly in start_backtrace() instead of clearing "frame" data structure by memset(). --- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/stacktrace.h | 4 ++++ arch/arm64/kernel/stacktrace.c | 7 +++++++ 3 files changed, 12 insertions(+)