From 0797a8d0d79769574550caa5ca5d89c237723250 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Fri, 18 May 2018 08:47:08 +0200 Subject: x86/stacktrace: Do not unwind after user regs Josh pointed out, that there is no way a frame can be after user regs. So remove the last unwind and the check. Signed-off-by: Jiri Slaby Acked-by: Josh Poimboeuf Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Denys Vlasenko Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: https://lkml.kernel.org/lkml/20180518064713.26440-1-jslaby@suse.cz Signed-off-by: Ingo Molnar --- arch/x86/kernel/stacktrace.c | 9 --------- 1 file changed, 9 deletions(-) (limited to 'arch/x86/kernel/stacktrace.c') diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c index 093f2ea5dd56..8948b7d9c064 100644 --- a/arch/x86/kernel/stacktrace.c +++ b/arch/x86/kernel/stacktrace.c @@ -113,15 +113,6 @@ __save_stack_trace_reliable(struct stack_trace *trace, if (!user_mode(regs)) return -EINVAL; - /* - * The last frame contains the user mode syscall - * pt_regs. Skip it and finish the unwind. - */ - unwind_next_frame(&state); - if (!unwind_done(&state)) { - STACKTRACE_DUMP_ONCE(task); - return -EINVAL; - } break; } -- cgit v1.2.3 From 17426923b03f098da83b8c1e044934a34959f69b Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Fri, 18 May 2018 08:47:09 +0200 Subject: x86/stacktrace: Remove STACKTRACE_DUMP_ONCE The stack unwinding can sometimes fail yet. Especially with the generated debug info. So do not yell at users -- live patching (the only user of this interface) will inform the user about the failure gracefully. And given this was the only user of the macro, remove the macro proper too. Signed-off-by: Jiri Slaby Acked-by: Josh Poimboeuf Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Denys Vlasenko Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: https://lkml.kernel.org/lkml/20180518064713.26440-2-jslaby@suse.cz Signed-off-by: Ingo Molnar --- arch/x86/kernel/stacktrace.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) (limited to 'arch/x86/kernel/stacktrace.c') diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c index 8948b7d9c064..f9dacf6d4667 100644 --- a/arch/x86/kernel/stacktrace.c +++ b/arch/x86/kernel/stacktrace.c @@ -81,16 +81,6 @@ EXPORT_SYMBOL_GPL(save_stack_trace_tsk); #ifdef CONFIG_HAVE_RELIABLE_STACKTRACE -#define STACKTRACE_DUMP_ONCE(task) ({ \ - static bool __section(.data.unlikely) __dumped; \ - \ - if (!__dumped) { \ - __dumped = true; \ - WARN_ON(1); \ - show_stack(task, NULL); \ - } \ -}) - static int __always_inline __save_stack_trace_reliable(struct stack_trace *trace, struct task_struct *task) @@ -123,20 +113,16 @@ __save_stack_trace_reliable(struct stack_trace *trace, * generated code which __kernel_text_address() doesn't know * about. */ - if (!addr) { - STACKTRACE_DUMP_ONCE(task); + if (!addr) return -EINVAL; - } if (save_stack_address(trace, addr, false)) return -EINVAL; } /* Check for stack corruption */ - if (unwind_error(&state)) { - STACKTRACE_DUMP_ONCE(task); + if (unwind_error(&state)) return -EINVAL; - } if (trace->nr_entries < trace->max_entries) trace->entries[trace->nr_entries++] = ULONG_MAX; -- cgit v1.2.3 From 441ccc3580f45340715fd8f5c4db795b06326404 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Fri, 18 May 2018 08:47:10 +0200 Subject: x86/stacktrace: Clarify the reliable success paths Make clear which path is for user tasks and for kthreads and idle tasks. This will allow easier plug-in of the ORC unwinder in the next patches. Note that we added a check for unwind error to the top of the loop, so that an error is returned also for user tasks (the 'goto success' would skip the check after the loop otherwise). Signed-off-by: Jiri Slaby Acked-by: Josh Poimboeuf Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Denys Vlasenko Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: https://lkml.kernel.org/lkml/20180518064713.26440-3-jslaby@suse.cz Signed-off-by: Ingo Molnar --- arch/x86/kernel/stacktrace.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'arch/x86/kernel/stacktrace.c') diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c index f9dacf6d4667..6acf1d5ca832 100644 --- a/arch/x86/kernel/stacktrace.c +++ b/arch/x86/kernel/stacktrace.c @@ -89,21 +89,24 @@ __save_stack_trace_reliable(struct stack_trace *trace, struct pt_regs *regs; unsigned long addr; - for (unwind_start(&state, task, NULL, NULL); !unwind_done(&state); + for (unwind_start(&state, task, NULL, NULL); + !unwind_done(&state) && !unwind_error(&state); unwind_next_frame(&state)) { regs = unwind_get_entry_regs(&state, NULL); if (regs) { + /* Success path for user tasks */ + if (user_mode(regs)) + goto success; + /* * Kernel mode registers on the stack indicate an * in-kernel interrupt or exception (e.g., preemption * or a page fault), which can make frame pointers * unreliable. */ - if (!user_mode(regs)) - return -EINVAL; - break; + return -EINVAL; } addr = unwind_get_return_address(&state); @@ -124,6 +127,11 @@ __save_stack_trace_reliable(struct stack_trace *trace, if (unwind_error(&state)) return -EINVAL; + /* Success path for non-user tasks, i.e. kthreads and idle tasks */ + if (!(task->flags & (PF_KTHREAD | PF_IDLE))) + return -EINVAL; + +success: if (trace->nr_entries < trace->max_entries) trace->entries[trace->nr_entries++] = ULONG_MAX; -- cgit v1.2.3 From 0c414367c04eeb00c3ebfee0b74c9e7f3b95fd62 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Fri, 18 May 2018 08:47:11 +0200 Subject: x86/stacktrace: Do not fail for ORC with regs on stack save_stack_trace_reliable now returns "non reliable" when there are kernel pt_regs on stack. This means an interrupt or exception happened somewhere down the route. It is a problem for the frame pointer unwinder, because the frame might not have been set up yet when the irq happened, so the unwinder might fail to unwind from the interrupted function. With ORC, this is not a problem, as ORC has out-of-band data. We can find ORC data even for the IP in the interrupted function and always unwind one level up reliably. So lift the check to apply only when CONFIG_FRAME_POINTER=y is enabled. Signed-off-by: Jiri Slaby Acked-by: Josh Poimboeuf Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Denys Vlasenko Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: https://lkml.kernel.org/lkml/20180518064713.26440-4-jslaby@suse.cz Signed-off-by: Ingo Molnar --- arch/x86/kernel/stacktrace.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'arch/x86/kernel/stacktrace.c') diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c index 6acf1d5ca832..7627455047c2 100644 --- a/arch/x86/kernel/stacktrace.c +++ b/arch/x86/kernel/stacktrace.c @@ -106,7 +106,8 @@ __save_stack_trace_reliable(struct stack_trace *trace, * unreliable. */ - return -EINVAL; + if (IS_ENABLED(CONFIG_FRAME_POINTER)) + return -EINVAL; } addr = unwind_get_return_address(&state); -- cgit v1.2.3