From bf255bdaada6d497536aadee5406f6ded318978b Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Thu, 18 Aug 2016 10:59:01 -0500 Subject: x86/dumpstack: Remove show_trace() There are a bewildering array of options for dumping the stack. Simplify things a little by removing show_trace(), which is unused. Signed-off-by: Josh Poimboeuf Reviewed-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Byungchul Park Cc: Denys Vlasenko Cc: Frederic Weisbecker Cc: H. Peter Anvin Cc: Kees Cook Cc: Linus Torvalds Cc: Nilay Vaish Cc: Peter Zijlstra Cc: Steven Rostedt Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/fe02292eac9d409001ec0cf6d06f90ced242570d.1471535549.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/kernel/dumpstack.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index 92e8f0a7159c..5f49c043500a 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -182,12 +182,6 @@ show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, dump_trace(task, regs, stack, bp, &print_trace_ops, log_lvl); } -void show_trace(struct task_struct *task, struct pt_regs *regs, - unsigned long *stack, unsigned long bp) -{ - show_trace_log_lvl(task, regs, stack, bp, ""); -} - void show_stack(struct task_struct *task, unsigned long *sp) { unsigned long bp = 0; -- cgit v1.2.3 From 408fe5de2f2767059a9561e0ae6d4385d1b39dac Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Fri, 19 Aug 2016 06:52:59 -0500 Subject: x86/dumpstack/ftrace: Convert dump_trace() callbacks to use ftrace_graph_ret_addr() Convert print_context_stack() and print_context_stack_bp() to use the arch-independent ftrace_graph_ret_addr() helper. Signed-off-by: Josh Poimboeuf Acked-by: Steven Rostedt Cc: Andy Lutomirski Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Byungchul Park Cc: Denys Vlasenko Cc: Frederic Weisbecker Cc: H. Peter Anvin Cc: Kees Cook Cc: Linus Torvalds Cc: Nilay Vaish Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/56ec97cafc1bf2e34d1119e6443d897db406da86.1471607358.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/kernel/dumpstack.c | 65 +++++++++++++++------------------------------ 1 file changed, 22 insertions(+), 43 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index 5f49c043500a..9bf3d021609c 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -38,38 +38,6 @@ void printk_address(unsigned long address) pr_cont(" [<%p>] %pS\n", (void *)address, (void *)address); } -#ifdef CONFIG_FUNCTION_GRAPH_TRACER -static void -print_ftrace_graph_addr(unsigned long addr, void *data, - const struct stacktrace_ops *ops, - struct task_struct *task, int *graph) -{ - unsigned long ret_addr; - int index; - - if (addr != (unsigned long)return_to_handler) - return; - - index = task->curr_ret_stack; - - if (!task->ret_stack || index < *graph) - return; - - index -= *graph; - ret_addr = task->ret_stack[index].ret; - - ops->address(data, ret_addr, 1); - - (*graph)++; -} -#else -static inline void -print_ftrace_graph_addr(unsigned long addr, void *data, - const struct stacktrace_ops *ops, - struct task_struct *task, int *graph) -{ } -#endif - /* * x86-64 can have up to three kernel stacks: * process stack @@ -107,18 +75,24 @@ print_context_stack(struct task_struct *task, stack = (unsigned long *)task_stack_page(task); while (valid_stack_ptr(task, stack, sizeof(*stack), end)) { - unsigned long addr; + unsigned long addr = *stack; - addr = *stack; if (__kernel_text_address(addr)) { + unsigned long real_addr; + int reliable = 0; + if ((unsigned long) stack == bp + sizeof(long)) { - ops->address(data, addr, 1); + reliable = 1; frame = frame->next_frame; bp = (unsigned long) frame; - } else { - ops->address(data, addr, 0); } - print_ftrace_graph_addr(addr, data, ops, task, graph); + + ops->address(data, addr, reliable); + + real_addr = ftrace_graph_ret_addr(task, graph, addr, + stack); + if (real_addr != addr) + ops->address(data, real_addr, 1); } stack++; } @@ -133,19 +107,24 @@ print_context_stack_bp(struct task_struct *task, unsigned long *end, int *graph) { struct stack_frame *frame = (struct stack_frame *)bp; - unsigned long *ret_addr = &frame->return_address; + unsigned long *retp = &frame->return_address; - while (valid_stack_ptr(task, ret_addr, sizeof(*ret_addr), end)) { - unsigned long addr = *ret_addr; + while (valid_stack_ptr(task, retp, sizeof(*retp), end)) { + unsigned long addr = *retp; + unsigned long real_addr; if (!__kernel_text_address(addr)) break; if (ops->address(data, addr, 1)) break; + + real_addr = ftrace_graph_ret_addr(task, graph, addr, retp); + if (real_addr != addr) + ops->address(data, real_addr, 1); + frame = frame->next_frame; - ret_addr = &frame->return_address; - print_ftrace_graph_addr(addr, data, ops, task, graph); + retp = &frame->return_address; } return (unsigned long)frame; -- cgit v1.2.3 From 6f727b84e23421721025f4eb1b4f6cea1d4d723a Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Fri, 19 Aug 2016 06:53:01 -0500 Subject: x86/dumpstack/ftrace: Mark function graph handler function as unreliable When function graph tracing is enabled for a function, its return address on the stack is replaced with the address of an ftrace handler (return_to_handler). Currently 'return_to_handler' can be reported as reliable. That's not ideal, and can actually be misleading. When saving or dumping the stack, you normally only care about what led up to that point (the call path), rather than what will happen in the future (the return path). That's especially true in the non-oops stack trace case, which isn't used for debugging. For example, in a perf profiling operation, reporting return_to_handler() in the trace would just be confusing. And in the oops case, where debugging is important, "unreliable" is also more appropriate there because it serves as a hint that graph tracing was involved, instead of trying to imply that return_to_handler() was the real caller. Signed-off-by: Josh Poimboeuf Acked-by: Steven Rostedt Cc: Andy Lutomirski Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Byungchul Park Cc: Denys Vlasenko Cc: Frederic Weisbecker Cc: H. Peter Anvin Cc: Kees Cook Cc: Linus Torvalds Cc: Nilay Vaish Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/f8af15749c7d632d3e7f815995831d5b7f82950d.1471607358.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/kernel/dumpstack.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index 9bf3d021609c..6aad8d4e2ea6 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -87,12 +87,21 @@ print_context_stack(struct task_struct *task, bp = (unsigned long) frame; } - ops->address(data, addr, reliable); - + /* + * When function graph tracing is enabled for a + * function, its return address on the stack is + * replaced with the address of an ftrace handler + * (return_to_handler). In that case, before printing + * the "real" address, we want to print the handler + * address as an "unreliable" hint that function graph + * tracing was involved. + */ real_addr = ftrace_graph_ret_addr(task, graph, addr, stack); if (real_addr != addr) - ops->address(data, real_addr, 1); + ops->address(data, addr, 0); + + ops->address(data, real_addr, reliable); } stack++; } @@ -116,12 +125,11 @@ print_context_stack_bp(struct task_struct *task, if (!__kernel_text_address(addr)) break; - if (ops->address(data, addr, 1)) - break; - real_addr = ftrace_graph_ret_addr(task, graph, addr, retp); - if (real_addr != addr) - ops->address(data, real_addr, 1); + if (real_addr != addr && ops->address(data, addr, 0)) + break; + if (ops->address(data, real_addr, 1)) + break; frame = frame->next_frame; retp = &frame->return_address; -- cgit v1.2.3 From 13e25bab7e51bdd4ba7df1ef2388961294bb565e Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Fri, 19 Aug 2016 06:53:02 -0500 Subject: x86/dumpstack/ftrace: Don't print unreliable addresses in print_context_stack_bp() When function graph tracing is enabled, print_context_stack_bp() can report return_to_handler() as an unreliable address, which is confusing and misleading: return_to_handler() is really only useful as a hint for debugging, whereas print_context_stack_bp() users only care about the actual 'reliable' call path. Signed-off-by: Josh Poimboeuf Acked-by: Steven Rostedt Cc: Andy Lutomirski Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Byungchul Park Cc: Denys Vlasenko Cc: Frederic Weisbecker Cc: H. Peter Anvin Cc: Kees Cook Cc: Linus Torvalds Cc: Nilay Vaish Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/c51aef578d8027791b38d2ad9bac0c7f499fde91.1471607358.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/kernel/dumpstack.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index 6aad8d4e2ea6..01072e9e165e 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -126,8 +126,6 @@ print_context_stack_bp(struct task_struct *task, break; real_addr = ftrace_graph_ret_addr(task, graph, addr, retp); - if (real_addr != addr && ops->address(data, addr, 0)) - break; if (ops->address(data, real_addr, 1)) break; -- cgit v1.2.3 From d438f5fda30ec087512355e405e9c8955d8bd337 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 24 Aug 2016 11:50:16 -0500 Subject: x86/dumpstack: Make printk_stack_address() more generally useful Change printk_stack_address() to be useful when called by an unwinder outside the context of dump_trace(). Specifically: - printk_stack_address()'s 'data' argument is always used as the log level string. Make that explicit. - Call touch_nmi_watchdog(). Signed-off-by: Josh Poimboeuf Cc: Andy Lutomirski Cc: Brian Gerst Cc: Byungchul Park Cc: Frederic Weisbecker Cc: Kees Cook Cc: Linus Torvalds Cc: Nilay Vaish Cc: Peter Zijlstra Cc: Steven Rostedt Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/9fbe0db05bacf66d337c162edbf61450d0cff1e2.1472057064.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/kernel/dumpstack.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index 01072e9e165e..f0ddf855957e 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -26,10 +26,11 @@ int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE; static int die_counter; static void printk_stack_address(unsigned long address, int reliable, - void *data) + char *log_lvl) { + touch_nmi_watchdog(); printk("%s [<%p>] %s%pB\n", - (char *)data, (void *)address, reliable ? "" : "? ", + log_lvl, (void *)address, reliable ? "" : "? ", (void *)address); } @@ -148,7 +149,6 @@ static int print_trace_stack(void *data, char *name) */ static int print_trace_address(void *data, unsigned long addr, int reliable) { - touch_nmi_watchdog(); printk_stack_address(addr, reliable, data); return 0; } -- cgit v1.2.3 From 4b8afafbe743be1a81c96ddcd75b19c534d5e262 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 24 Aug 2016 11:50:17 -0500 Subject: x86/dumpstack: Add get_stack_pointer() and get_frame_pointer() The various functions involved in dumping the stack all do similar things with regard to getting the stack pointer and the frame pointer based on the regs and task arguments. Create helper functions to do that instead. Signed-off-by: Josh Poimboeuf Reviewed-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Brian Gerst Cc: Byungchul Park Cc: Frederic Weisbecker Cc: Kees Cook Cc: Linus Torvalds Cc: Nilay Vaish Cc: Peter Zijlstra Cc: Steven Rostedt Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/f448914885a35f333fe04da1b97a6c2cc1f80974.1472057064.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/include/asm/stacktrace.h | 41 ++++++++++++++++++++++----------------- arch/x86/kernel/dumpstack.c | 5 ++--- arch/x86/kernel/dumpstack_32.c | 25 ++++-------------------- arch/x86/kernel/dumpstack_64.c | 30 ++++------------------------ 4 files changed, 33 insertions(+), 68 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h index 7646fb2772f8..3552f5e7189e 100644 --- a/arch/x86/include/asm/stacktrace.h +++ b/arch/x86/include/asm/stacktrace.h @@ -50,36 +50,41 @@ void dump_trace(struct task_struct *tsk, struct pt_regs *regs, #ifdef CONFIG_X86_32 #define STACKSLOTS_PER_LINE 8 -#define get_bp(bp) asm("movl %%ebp, %0" : "=r" (bp) :) #else #define STACKSLOTS_PER_LINE 4 -#define get_bp(bp) asm("movq %%rbp, %0" : "=r" (bp) :) #endif #ifdef CONFIG_FRAME_POINTER -static inline unsigned long -stack_frame(struct task_struct *task, struct pt_regs *regs) +static inline unsigned long * +get_frame_pointer(struct task_struct *task, struct pt_regs *regs) { - unsigned long bp; - if (regs) - return regs->bp; + return (unsigned long *)regs->bp; - if (task == current) { - /* Grab bp right from our regs */ - get_bp(bp); - return bp; - } + if (!task || task == current) + return __builtin_frame_address(0); - return ((struct inactive_task_frame *)task->thread.sp)->bp; + return (unsigned long *)((struct inactive_task_frame *)task->thread.sp)->bp; } #else -static inline unsigned long -stack_frame(struct task_struct *task, struct pt_regs *regs) +static inline unsigned long * +get_frame_pointer(struct task_struct *task, struct pt_regs *regs) { - return 0; + return NULL; +} +#endif /* CONFIG_FRAME_POINTER */ + +static inline unsigned long * +get_stack_pointer(struct task_struct *task, struct pt_regs *regs) +{ + if (regs) + return (unsigned long *)kernel_stack_pointer(regs); + + if (!task || task == current) + return __builtin_frame_address(0); + + return (unsigned long *)task->thread.sp; } -#endif extern void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, @@ -106,7 +111,7 @@ static inline unsigned long caller_frame_pointer(void) { struct stack_frame *frame; - get_bp(frame); + frame = __builtin_frame_address(0); #ifdef CONFIG_FRAME_POINTER frame = frame->next_frame; diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index f0ddf855957e..6d6f46837eea 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -170,15 +170,14 @@ show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, void show_stack(struct task_struct *task, unsigned long *sp) { unsigned long bp = 0; - unsigned long stack; /* * Stack frames below this one aren't interesting. Don't show them * if we're printing for %current. */ if (!sp && (!task || task == current)) { - sp = &stack; - bp = stack_frame(current, NULL); + sp = get_stack_pointer(current, NULL); + bp = (unsigned long)get_frame_pointer(current, NULL); } show_stack_log_lvl(task, NULL, sp, bp, ""); diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c index 09675712eba8..358fe1cd4e5b 100644 --- a/arch/x86/kernel/dumpstack_32.c +++ b/arch/x86/kernel/dumpstack_32.c @@ -46,19 +46,9 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs, int graph = 0; u32 *prev_esp; - if (!task) - task = current; - - if (!stack) { - unsigned long dummy; - - stack = &dummy; - if (task != current) - stack = (unsigned long *)task->thread.sp; - } - - if (!bp) - bp = stack_frame(task, regs); + task = task ? : current; + stack = stack ? : get_stack_pointer(task, regs); + bp = bp ? : (unsigned long)get_frame_pointer(task, regs); for (;;) { void *end_stack; @@ -95,14 +85,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, unsigned long *stack; int i; - if (sp == NULL) { - if (regs) - sp = (unsigned long *)regs->sp; - else if (task) - sp = (unsigned long *)task->thread.sp; - else - sp = (unsigned long *)&sp; - } + sp = sp ? : get_stack_pointer(task, regs); stack = sp; for (i = 0; i < kstack_depth_to_print; i++) { diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c index 066eb5c77fd6..7f3b8066f719 100644 --- a/arch/x86/kernel/dumpstack_64.c +++ b/arch/x86/kernel/dumpstack_64.c @@ -151,25 +151,14 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs, { const unsigned cpu = get_cpu(); unsigned long *irq_stack = (unsigned long *)per_cpu(irq_stack_ptr, cpu); - unsigned long dummy; unsigned used = 0; int graph = 0; int done = 0; - if (!task) - task = current; + task = task ? : current; + stack = stack ? : get_stack_pointer(task, regs); + bp = bp ? : (unsigned long)get_frame_pointer(task, regs); - if (!stack) { - if (regs) - stack = (unsigned long *)regs->sp; - else if (task != current) - stack = (unsigned long *)task->thread.sp; - else - stack = &dummy; - } - - if (!bp) - bp = stack_frame(task, regs); /* * Print function call entries in all stacks, starting at the * current stack address. If the stacks consist of nested @@ -256,18 +245,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, irq_stack_end = (unsigned long *)(per_cpu(irq_stack_ptr, cpu)); irq_stack = irq_stack_end - (IRQ_STACK_SIZE / sizeof(long)); - /* - * Debugging aid: "show_stack(NULL, NULL);" prints the - * back trace for this cpu: - */ - if (sp == NULL) { - if (regs) - sp = (unsigned long *)regs->sp; - else if (task) - sp = (unsigned long *)task->thread.sp; - else - sp = (unsigned long *)&sp; - } + sp = sp ? : get_stack_pointer(task, regs); stack = sp; for (i = 0; i < kstack_depth_to_print; i++) { -- cgit v1.2.3 From 5a8ff54c260ecfed3de9b8d1272eb87826935df8 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 24 Aug 2016 11:50:18 -0500 Subject: x86/dumpstack: Remove unnecessary stack pointer arguments When calling show_stack_log_lvl() or dump_trace() with a regs argument, providing a stack pointer or frame pointer is redundant. Signed-off-by: Josh Poimboeuf d Reviewed-by: Andy Lutomirski Cc: Andy Lutomirski Cc: Brian Gerst Cc: Byungchul Park Cc: Frederic Weisbecker Cc: Kees Cook Cc: Linus Torvalds Cc: Nilay Vaish Cc: Peter Zijlstra Cc: Steven Rostedt Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/1694e2e955e3b9a73a3c3d5ba2634344014dd550.1472057064.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/kernel/dumpstack.c | 2 +- arch/x86/kernel/dumpstack_32.c | 2 +- arch/x86/kernel/dumpstack_64.c | 5 +---- arch/x86/oprofile/backtrace.c | 5 +---- 4 files changed, 4 insertions(+), 10 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index 6d6f46837eea..c6c6c39c367f 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -185,7 +185,7 @@ void show_stack(struct task_struct *task, unsigned long *sp) void show_stack_regs(struct pt_regs *regs) { - show_stack_log_lvl(current, regs, (unsigned long *)regs->sp, regs->bp, ""); + show_stack_log_lvl(current, regs, NULL, 0, ""); } static arch_spinlock_t die_lock = __ARCH_SPIN_LOCK_UNLOCKED; diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c index 358fe1cd4e5b..c533b8b5a373 100644 --- a/arch/x86/kernel/dumpstack_32.c +++ b/arch/x86/kernel/dumpstack_32.c @@ -122,7 +122,7 @@ void show_regs(struct pt_regs *regs) u8 *ip; pr_emerg("Stack:\n"); - show_stack_log_lvl(NULL, regs, ®s->sp, 0, KERN_EMERG); + show_stack_log_lvl(NULL, regs, NULL, 0, KERN_EMERG); pr_emerg("Code:"); diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c index 7f3b8066f719..b243352c779e 100644 --- a/arch/x86/kernel/dumpstack_64.c +++ b/arch/x86/kernel/dumpstack_64.c @@ -283,9 +283,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, void show_regs(struct pt_regs *regs) { int i; - unsigned long sp; - sp = regs->sp; show_regs_print_info(KERN_DEFAULT); __show_regs(regs, 1); @@ -300,8 +298,7 @@ void show_regs(struct pt_regs *regs) u8 *ip; printk(KERN_DEFAULT "Stack:\n"); - show_stack_log_lvl(NULL, regs, (unsigned long *)sp, - 0, KERN_DEFAULT); + show_stack_log_lvl(NULL, regs, NULL, 0, KERN_DEFAULT); printk(KERN_DEFAULT "Code: "); diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c index 2ef6c8b56311..d950f9ea9a8c 100644 --- a/arch/x86/oprofile/backtrace.c +++ b/arch/x86/oprofile/backtrace.c @@ -113,8 +113,6 @@ x86_backtrace(struct pt_regs * const regs, unsigned int depth) struct stack_frame *head = (struct stack_frame *)frame_pointer(regs); if (!user_mode(regs)) { - unsigned long stack = kernel_stack_pointer(regs); - if (!depth) return; @@ -122,8 +120,7 @@ x86_backtrace(struct pt_regs * const regs, unsigned int depth) if (!--depth) return; - dump_trace(NULL, regs, (unsigned long *)stack, 0, - &backtrace_ops, &depth); + dump_trace(NULL, regs, NULL, 0, &backtrace_ops, &depth); return; } -- cgit v1.2.3 From cb76c93982404273d746f3ccd5085b47689099a8 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 14 Sep 2016 21:07:42 -0500 Subject: x86/dumpstack: Add get_stack_info() interface valid_stack_ptr() is buggy: it assumes that all stacks are of size THREAD_SIZE, which is not true for exception stacks. So the walk_stack() callbacks will need to know the location of the beginning of the stack as well as the end. Another issue is that in general the various features of a stack (type, size, next stack pointer, description string) are scattered around in various places throughout the stack dump code. Encapsulate all that information in a single place with a new stack_info struct and a get_stack_info() interface. Signed-off-by: Josh Poimboeuf Cc: Andy Lutomirski Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Byungchul Park Cc: Denys Vlasenko Cc: Frederic Weisbecker Cc: H. Peter Anvin Cc: Kees Cook Cc: Linus Torvalds Cc: Nilay Vaish Cc: Peter Zijlstra Cc: Steven Rostedt Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/8164dd0db96b7e6a279fa17ae5e6dc375eecb4a9.1473905218.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/events/core.c | 2 +- arch/x86/include/asm/stacktrace.h | 41 ++++++++- arch/x86/kernel/dumpstack.c | 40 +++++---- arch/x86/kernel/dumpstack_32.c | 106 ++++++++++++++++++------ arch/x86/kernel/dumpstack_64.c | 169 ++++++++++++++++++++------------------ arch/x86/kernel/stacktrace.c | 2 +- arch/x86/oprofile/backtrace.c | 2 +- 7 files changed, 234 insertions(+), 128 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c index c1319ac19ebb..477dc38b62b1 100644 --- a/arch/x86/events/core.c +++ b/arch/x86/events/core.c @@ -2251,7 +2251,7 @@ void arch_perf_update_userpage(struct perf_event *event, * callchain support */ -static int backtrace_stack(void *data, char *name) +static int backtrace_stack(void *data, const char *name) { return 0; } diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h index 3552f5e7189e..780a83efcfd3 100644 --- a/arch/x86/include/asm/stacktrace.h +++ b/arch/x86/include/asm/stacktrace.h @@ -10,6 +10,39 @@ #include #include +enum stack_type { + STACK_TYPE_UNKNOWN, + STACK_TYPE_TASK, + STACK_TYPE_IRQ, + STACK_TYPE_SOFTIRQ, + STACK_TYPE_EXCEPTION, + STACK_TYPE_EXCEPTION_LAST = STACK_TYPE_EXCEPTION + N_EXCEPTION_STACKS-1, +}; + +struct stack_info { + enum stack_type type; + unsigned long *begin, *end, *next_sp; +}; + +bool in_task_stack(unsigned long *stack, struct task_struct *task, + struct stack_info *info); + +int get_stack_info(unsigned long *stack, struct task_struct *task, + struct stack_info *info, unsigned long *visit_mask); + +void stack_type_str(enum stack_type type, const char **begin, + const char **end); + +static inline bool on_stack(struct stack_info *info, void *addr, size_t len) +{ + void *begin = info->begin; + void *end = info->end; + + return (info->type != STACK_TYPE_UNKNOWN && + addr >= begin && addr < end && + addr + len > begin && addr + len <= end); +} + extern int kstack_depth_to_print; struct thread_info; @@ -20,27 +53,27 @@ typedef unsigned long (*walk_stack_t)(struct task_struct *task, unsigned long bp, const struct stacktrace_ops *ops, void *data, - unsigned long *end, + struct stack_info *info, int *graph); extern unsigned long print_context_stack(struct task_struct *task, unsigned long *stack, unsigned long bp, const struct stacktrace_ops *ops, void *data, - unsigned long *end, int *graph); + struct stack_info *info, int *graph); extern unsigned long print_context_stack_bp(struct task_struct *task, unsigned long *stack, unsigned long bp, const struct stacktrace_ops *ops, void *data, - unsigned long *end, int *graph); + struct stack_info *info, int *graph); /* Generic stack tracer with callbacks */ struct stacktrace_ops { int (*address)(void *data, unsigned long address, int reliable); /* On negative return stop dumping */ - int (*stack)(void *data, char *name); + int (*stack)(void *data, const char *name); walk_stack_t walk_stack; }; diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index c6c6c39c367f..aa208e565b03 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -25,6 +25,23 @@ unsigned int code_bytes = 64; int kstack_depth_to_print = 3 * STACKSLOTS_PER_LINE; static int die_counter; +bool in_task_stack(unsigned long *stack, struct task_struct *task, + struct stack_info *info) +{ + unsigned long *begin = task_stack_page(task); + unsigned long *end = task_stack_page(task) + THREAD_SIZE; + + if (stack < begin || stack >= end) + return false; + + info->type = STACK_TYPE_TASK; + info->begin = begin; + info->end = end; + info->next_sp = NULL; + + return true; +} + static void printk_stack_address(unsigned long address, int reliable, char *log_lvl) { @@ -46,24 +63,11 @@ void printk_address(unsigned long address) * severe exception (double fault, nmi, stack fault, debug, mce) hardware stack */ -static inline int valid_stack_ptr(struct task_struct *task, - void *p, unsigned int size, void *end) -{ - void *t = task_stack_page(task); - if (end) { - if (p < end && p >= (end-THREAD_SIZE)) - return 1; - else - return 0; - } - return p >= t && p < t + THREAD_SIZE - size; -} - unsigned long print_context_stack(struct task_struct *task, unsigned long *stack, unsigned long bp, const struct stacktrace_ops *ops, void *data, - unsigned long *end, int *graph) + struct stack_info *info, int *graph) { struct stack_frame *frame = (struct stack_frame *)bp; @@ -75,7 +79,7 @@ print_context_stack(struct task_struct *task, PAGE_SIZE) stack = (unsigned long *)task_stack_page(task); - while (valid_stack_ptr(task, stack, sizeof(*stack), end)) { + while (on_stack(info, stack, sizeof(*stack))) { unsigned long addr = *stack; if (__kernel_text_address(addr)) { @@ -114,12 +118,12 @@ unsigned long print_context_stack_bp(struct task_struct *task, unsigned long *stack, unsigned long bp, const struct stacktrace_ops *ops, void *data, - unsigned long *end, int *graph) + struct stack_info *info, int *graph) { struct stack_frame *frame = (struct stack_frame *)bp; unsigned long *retp = &frame->return_address; - while (valid_stack_ptr(task, retp, sizeof(*retp), end)) { + while (on_stack(info, stack, sizeof(*stack) * 2)) { unsigned long addr = *retp; unsigned long real_addr; @@ -138,7 +142,7 @@ print_context_stack_bp(struct task_struct *task, } EXPORT_SYMBOL_GPL(print_context_stack_bp); -static int print_trace_stack(void *data, char *name) +static int print_trace_stack(void *data, const char *name) { printk("%s <%s> ", (char *)data, name); return 0; diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c index da5cd62f93ab..c92da5a4d663 100644 --- a/arch/x86/kernel/dumpstack_32.c +++ b/arch/x86/kernel/dumpstack_32.c @@ -16,61 +16,117 @@ #include -static void *is_irq_stack(void *p, void *irq) +void stack_type_str(enum stack_type type, const char **begin, const char **end) { - if (p < irq || p >= (irq + THREAD_SIZE)) - return NULL; - return irq + THREAD_SIZE; + switch (type) { + case STACK_TYPE_IRQ: + case STACK_TYPE_SOFTIRQ: + *begin = "IRQ"; + *end = "EOI"; + break; + default: + *begin = NULL; + *end = NULL; + } } +static bool in_hardirq_stack(unsigned long *stack, struct stack_info *info) +{ + unsigned long *begin = (unsigned long *)this_cpu_read(hardirq_stack); + unsigned long *end = begin + (THREAD_SIZE / sizeof(long)); + + if (stack < begin || stack >= end) + return false; + + info->type = STACK_TYPE_IRQ; + info->begin = begin; + info->end = end; + + /* + * See irq_32.c -- the next stack pointer is stored at the beginning of + * the stack. + */ + info->next_sp = (unsigned long *)*begin; + + return true; +} -static void *is_hardirq_stack(unsigned long *stack) +static bool in_softirq_stack(unsigned long *stack, struct stack_info *info) { - void *irq = this_cpu_read(hardirq_stack); + unsigned long *begin = (unsigned long *)this_cpu_read(softirq_stack); + unsigned long *end = begin + (THREAD_SIZE / sizeof(long)); + + if (stack < begin || stack >= end) + return false; + + info->type = STACK_TYPE_SOFTIRQ; + info->begin = begin; + info->end = end; + + /* + * The next stack pointer is stored at the beginning of the stack. + * See irq_32.c. + */ + info->next_sp = (unsigned long *)*begin; - return is_irq_stack(stack, irq); + return true; } -static void *is_softirq_stack(unsigned long *stack) +int get_stack_info(unsigned long *stack, struct task_struct *task, + struct stack_info *info, unsigned long *visit_mask) { - void *irq = this_cpu_read(softirq_stack); + if (!stack) + goto unknown; - return is_irq_stack(stack, irq); + task = task ? : current; + + if (in_task_stack(stack, task, info)) + return 0; + + if (task != current) + goto unknown; + + if (in_hardirq_stack(stack, info)) + return 0; + + if (in_softirq_stack(stack, info)) + return 0; + +unknown: + info->type = STACK_TYPE_UNKNOWN; + return -EINVAL; } void dump_trace(struct task_struct *task, struct pt_regs *regs, unsigned long *stack, unsigned long bp, const struct stacktrace_ops *ops, void *data) { + unsigned long visit_mask = 0; int graph = 0; - u32 *prev_esp; task = task ? : current; stack = stack ? : get_stack_pointer(task, regs); bp = bp ? : (unsigned long)get_frame_pointer(task, regs); for (;;) { - void *end_stack; + const char *begin_str, *end_str; + struct stack_info info; - end_stack = is_hardirq_stack(stack); - if (!end_stack) - end_stack = is_softirq_stack(stack); + if (get_stack_info(stack, task, &info, &visit_mask)) + break; - bp = ops->walk_stack(task, stack, bp, ops, data, - end_stack, &graph); + stack_type_str(info.type, &begin_str, &end_str); - /* Stop if not on irq stack */ - if (!end_stack) + if (begin_str && ops->stack(data, begin_str) < 0) break; - /* The previous esp is saved on the bottom of the stack */ - prev_esp = (u32 *)(end_stack - THREAD_SIZE); - stack = (unsigned long *)*prev_esp; - if (!stack) - break; + bp = ops->walk_stack(task, stack, bp, ops, data, &info, &graph); - if (ops->stack(data, "IRQ") < 0) + if (end_str && ops->stack(data, end_str) < 0) break; + + stack = info.next_sp; + touch_nmi_watchdog(); } } diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c index 904fb46d7d65..41813abc7380 100644 --- a/arch/x86/kernel/dumpstack_64.c +++ b/arch/x86/kernel/dumpstack_64.c @@ -28,76 +28,109 @@ static unsigned long exception_stack_sizes[N_EXCEPTION_STACKS] = { [DEBUG_STACK - 1] = DEBUG_STKSZ }; -static unsigned long *in_exception_stack(unsigned long stack, unsigned *usedp, - char **idp) +void stack_type_str(enum stack_type type, const char **begin, const char **end) { - unsigned long begin, end; + BUILD_BUG_ON(N_EXCEPTION_STACKS != 4); + + switch (type) { + case STACK_TYPE_IRQ: + *begin = "IRQ"; + *end = "EOI"; + break; + case STACK_TYPE_EXCEPTION ... STACK_TYPE_EXCEPTION_LAST: + *begin = exception_stack_names[type - STACK_TYPE_EXCEPTION]; + *end = "EOE"; + break; + default: + *begin = NULL; + *end = NULL; + } +} + +static bool in_exception_stack(unsigned long *stack, struct stack_info *info, + unsigned long *visit_mask) +{ + unsigned long *begin, *end; + struct pt_regs *regs; unsigned k; BUILD_BUG_ON(N_EXCEPTION_STACKS != 4); for (k = 0; k < N_EXCEPTION_STACKS; k++) { - end = raw_cpu_ptr(&orig_ist)->ist[k]; - begin = end - exception_stack_sizes[k]; + end = (unsigned long *)raw_cpu_ptr(&orig_ist)->ist[k]; + begin = end - (exception_stack_sizes[k] / sizeof(long)); + regs = (struct pt_regs *)end - 1; if (stack < begin || stack >= end) continue; /* - * Make sure we only iterate through an exception stack once. - * If it comes up for the second time then there's something - * wrong going on - just break and return NULL: + * Make sure we don't iterate through an exception stack more + * than once. If it comes up a second time then there's + * something wrong going on - just break out and report an + * unknown stack type. */ - if (*usedp & (1U << k)) + if (*visit_mask & (1U << k)) break; - *usedp |= 1U << k; + *visit_mask |= 1U << k; - *idp = exception_stack_names[k]; - return (unsigned long *)end; + info->type = STACK_TYPE_EXCEPTION + k; + info->begin = begin; + info->end = end; + info->next_sp = (unsigned long *)regs->sp; + + return true; } - return NULL; + return false; } -static inline int -in_irq_stack(unsigned long *stack, unsigned long *irq_stack, - unsigned long *irq_stack_end) +static bool in_irq_stack(unsigned long *stack, struct stack_info *info) { - return (stack >= irq_stack && stack < irq_stack_end); -} + unsigned long *end = (unsigned long *)this_cpu_read(irq_stack_ptr); + unsigned long *begin = end - (IRQ_STACK_SIZE / sizeof(long)); -enum stack_type { - STACK_IS_UNKNOWN, - STACK_IS_NORMAL, - STACK_IS_EXCEPTION, - STACK_IS_IRQ, -}; + if (stack < begin || stack >= end) + return false; + + info->type = STACK_TYPE_IRQ; + info->begin = begin; + info->end = end; + + /* + * The next stack pointer is the first thing pushed by the entry code + * after switching to the irq stack. + */ + info->next_sp = (unsigned long *)*(end - 1); + + return true; +} -static enum stack_type -analyze_stack(struct task_struct *task, unsigned long *stack, - unsigned long **stack_end, unsigned long *irq_stack, - unsigned *used, char **id) +int get_stack_info(unsigned long *stack, struct task_struct *task, + struct stack_info *info, unsigned long *visit_mask) { - unsigned long addr; + if (!stack) + goto unknown; - addr = ((unsigned long)stack & (~(THREAD_SIZE - 1))); - if ((unsigned long)task_stack_page(task) == addr) - return STACK_IS_NORMAL; + task = task ? : current; + + if (in_task_stack(stack, task, info)) + return 0; - *stack_end = in_exception_stack((unsigned long)stack, used, id); - if (*stack_end) - return STACK_IS_EXCEPTION; + if (task != current) + goto unknown; - if (!irq_stack) - return STACK_IS_NORMAL; + if (in_exception_stack(stack, info, visit_mask)) + return 0; - *stack_end = irq_stack; - irq_stack -= (IRQ_STACK_SIZE / sizeof(long)); + if (in_irq_stack(stack, info)) + return 0; - if (in_irq_stack(stack, irq_stack, *stack_end)) - return STACK_IS_IRQ; + return 0; - return STACK_IS_UNKNOWN; +unknown: + info->type = STACK_TYPE_UNKNOWN; + return -EINVAL; } /* @@ -111,8 +144,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs, unsigned long *stack, unsigned long bp, const struct stacktrace_ops *ops, void *data) { - unsigned long *irq_stack = (unsigned long *)this_cpu_read(irq_stack_ptr); - unsigned used = 0; + unsigned long visit_mask = 0; + struct stack_info info; int graph = 0; int done = 0; @@ -126,57 +159,37 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs, * exceptions */ while (!done) { - unsigned long *stack_end; - enum stack_type stype; - char *id; + const char *begin_str, *end_str; - stype = analyze_stack(task, stack, &stack_end, irq_stack, &used, - &id); + get_stack_info(stack, task, &info, &visit_mask); /* Default finish unless specified to continue */ done = 1; - switch (stype) { + switch (info.type) { /* Break out early if we are on the thread stack */ - case STACK_IS_NORMAL: + case STACK_TYPE_TASK: break; - case STACK_IS_EXCEPTION: + case STACK_TYPE_IRQ: + case STACK_TYPE_EXCEPTION ... STACK_TYPE_EXCEPTION_LAST: + + stack_type_str(info.type, &begin_str, &end_str); - if (ops->stack(data, id) < 0) + if (ops->stack(data, begin_str) < 0) break; bp = ops->walk_stack(task, stack, bp, ops, - data, stack_end, &graph); - ops->stack(data, "EOE"); - /* - * We link to the next stack via the - * second-to-last pointer (index -2 to end) in the - * exception stack: - */ - stack = (unsigned long *) stack_end[-2]; - done = 0; - break; + data, &info, &graph); - case STACK_IS_IRQ: + ops->stack(data, end_str); - if (ops->stack(data, "IRQ") < 0) - break; - bp = ops->walk_stack(task, stack, bp, - ops, data, stack_end, &graph); - /* - * We link to the next stack (which would be - * the process stack normally) the last - * pointer (index -1 to end) in the IRQ stack: - */ - stack = (unsigned long *) (stack_end[-1]); - irq_stack = NULL; - ops->stack(data, "EOI"); + stack = info.next_sp; done = 0; break; - case STACK_IS_UNKNOWN: + default: ops->stack(data, "UNK"); break; } @@ -185,7 +198,7 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs, /* * This handles the process stack: */ - bp = ops->walk_stack(task, stack, bp, ops, data, NULL, &graph); + bp = ops->walk_stack(task, stack, bp, ops, data, &info, &graph); } EXPORT_SYMBOL(dump_trace); diff --git a/arch/x86/kernel/stacktrace.c b/arch/x86/kernel/stacktrace.c index 4738f5e0f2ab..785aef1c7ef5 100644 --- a/arch/x86/kernel/stacktrace.c +++ b/arch/x86/kernel/stacktrace.c @@ -9,7 +9,7 @@ #include #include -static int save_stack_stack(void *data, char *name) +static int save_stack_stack(void *data, const char *name) { return 0; } diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c index d950f9ea9a8c..75391488130b 100644 --- a/arch/x86/oprofile/backtrace.c +++ b/arch/x86/oprofile/backtrace.c @@ -17,7 +17,7 @@ #include #include -static int backtrace_stack(void *data, char *name) +static int backtrace_stack(void *data, const char *name) { /* Yes, we want all stacks */ return 0; -- cgit v1.2.3 From 81539169f283329fd8bc58457cc15754f683ba69 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Fri, 16 Sep 2016 08:05:20 -0500 Subject: x86/dumpstack: Remove NULL task pointer convention show_stack_log_lvl() and friends allow a NULL pointer for the task_struct to indicate the current task. This creates confusion and can cause sneaky bugs. Instead require the caller to pass 'current' directly. This only changes the internal workings of the dumpstack code. The dump_trace() and show_stack() interfaces still allow a NULL task pointer. Those interfaces should also probably be fixed as well. Signed-off-by: Josh Poimboeuf Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Signed-off-by: Ingo Molnar --- arch/x86/include/asm/stacktrace.h | 4 ++-- arch/x86/kernel/dumpstack.c | 4 +++- arch/x86/kernel/dumpstack_32.c | 2 +- arch/x86/kernel/dumpstack_64.c | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h index 780a83efcfd3..ed2be1b5ada8 100644 --- a/arch/x86/include/asm/stacktrace.h +++ b/arch/x86/include/asm/stacktrace.h @@ -94,7 +94,7 @@ get_frame_pointer(struct task_struct *task, struct pt_regs *regs) if (regs) return (unsigned long *)regs->bp; - if (!task || task == current) + if (task == current) return __builtin_frame_address(0); return (unsigned long *)((struct inactive_task_frame *)task->thread.sp)->bp; @@ -113,7 +113,7 @@ get_stack_pointer(struct task_struct *task, struct pt_regs *regs) if (regs) return (unsigned long *)kernel_stack_pointer(regs); - if (!task || task == current) + if (task == current) return __builtin_frame_address(0); return (unsigned long *)task->thread.sp; diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index aa208e565b03..e0648f755158 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -175,11 +175,13 @@ void show_stack(struct task_struct *task, unsigned long *sp) { unsigned long bp = 0; + task = task ? : current; + /* * Stack frames below this one aren't interesting. Don't show them * if we're printing for %current. */ - if (!sp && (!task || task == current)) { + if (!sp && task == current) { sp = get_stack_pointer(current, NULL); bp = (unsigned long)get_frame_pointer(current, NULL); } diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c index 122f37d7bb7e..4ff000811e03 100644 --- a/arch/x86/kernel/dumpstack_32.c +++ b/arch/x86/kernel/dumpstack_32.c @@ -205,7 +205,7 @@ void show_regs(struct pt_regs *regs) u8 *ip; pr_emerg("Stack:\n"); - show_stack_log_lvl(NULL, regs, NULL, 0, KERN_EMERG); + show_stack_log_lvl(current, regs, NULL, 0, KERN_EMERG); pr_emerg("Code:"); diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c index 16c0d5f89b5e..008a29837cab 100644 --- a/arch/x86/kernel/dumpstack_64.c +++ b/arch/x86/kernel/dumpstack_64.c @@ -278,7 +278,7 @@ void show_regs(struct pt_regs *regs) u8 *ip; printk(KERN_DEFAULT "Stack:\n"); - show_stack_log_lvl(NULL, regs, NULL, 0, KERN_DEFAULT); + show_stack_log_lvl(current, regs, NULL, 0, KERN_DEFAULT); printk(KERN_DEFAULT "Code: "); -- cgit v1.2.3 From e18bcccd1a4ecb41e99678e002ef833586185bf1 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Fri, 16 Sep 2016 14:18:16 -0500 Subject: x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder Convert show_trace_log_lvl() to use the new unwinder. dump_trace() has been deprecated. show_trace_log_lvl() is special compared to other users of the unwinder. It's the only place where both reliable *and* unreliable addresses are needed. With frame pointers enabled, most callers of the unwinder don't want to know about unreliable addresses. But in this case, when we're dumping the stack to the console because something presumably went wrong, the unreliable addresses are useful: - They show stale data on the stack which can provide useful clues. - If something goes wrong with the unwinder, or if frame pointers are corrupt or missing, all the stack addresses still get shown. So in order to show all addresses on the stack, and at the same time figure out which addresses are reliable, we have to do the scanning and the unwinding in parallel. The scanning is done with the help of get_stack_info() to traverse the stacks. The unwinding is done separately by the new unwinder. In theory we could simplify show_trace_log_lvl() by instead pushing some of this logic into the unwind code. But then we would need some kind of "fake" frame logic in the unwinder which would add a lot of complexity and wouldn't be worth it in order to support only one user. Another benefit of this approach is that once we have a DWARF unwinder, we should be able to just plug it in with minimal impact to this code. Another change here is that callers of show_trace_log_lvl() don't need to provide the 'bp' argument. The unwinder already finds the relevant frame pointer by unwinding until it reaches the first frame after the provided stack pointer. Signed-off-by: Josh Poimboeuf Cc: Andy Lutomirski Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Byungchul Park Cc: Denys Vlasenko Cc: Frederic Weisbecker Cc: H. Peter Anvin Cc: Kees Cook Cc: Linus Torvalds Cc: Nilay Vaish Cc: Peter Zijlstra Cc: Steven Rostedt Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/703b5998604c712a1f801874b43f35d6dac52ede.1474045023.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/include/asm/stacktrace.h | 10 ++- arch/x86/kernel/dumpstack.c | 126 ++++++++++++++++++++++++++++---------- arch/x86/kernel/dumpstack_32.c | 9 ++- arch/x86/kernel/dumpstack_64.c | 9 ++- 4 files changed, 107 insertions(+), 47 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h index ed2be1b5ada8..c9ccf0676ca6 100644 --- a/arch/x86/include/asm/stacktrace.h +++ b/arch/x86/include/asm/stacktrace.h @@ -119,13 +119,11 @@ get_stack_pointer(struct task_struct *task, struct pt_regs *regs) return (unsigned long *)task->thread.sp; } -extern void -show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *stack, unsigned long bp, char *log_lvl); +void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, + unsigned long *stack, char *log_lvl); -extern void -show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *sp, unsigned long bp, char *log_lvl); +void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, + unsigned long *sp, char *log_lvl); extern unsigned int code_bytes; diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index e0648f755158..c08f32ab8ace 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -17,7 +17,7 @@ #include #include - +#include int panic_on_unrecovered_nmi; int panic_on_io_nmi; @@ -142,56 +142,120 @@ print_context_stack_bp(struct task_struct *task, } EXPORT_SYMBOL_GPL(print_context_stack_bp); -static int print_trace_stack(void *data, const char *name) +void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, + unsigned long *stack, char *log_lvl) { - printk("%s <%s> ", (char *)data, name); - return 0; -} + struct unwind_state state; + struct stack_info stack_info = {0}; + unsigned long visit_mask = 0; + int graph_idx = 0; -/* - * Print one address/symbol entries per line. - */ -static int print_trace_address(void *data, unsigned long addr, int reliable) -{ - printk_stack_address(addr, reliable, data); - return 0; -} + printk("%sCall Trace:\n", log_lvl); -static const struct stacktrace_ops print_trace_ops = { - .stack = print_trace_stack, - .address = print_trace_address, - .walk_stack = print_context_stack, -}; + unwind_start(&state, task, regs, stack); -void -show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *stack, unsigned long bp, char *log_lvl) -{ - printk("%sCall Trace:\n", log_lvl); - dump_trace(task, regs, stack, bp, &print_trace_ops, log_lvl); + /* + * Iterate through the stacks, starting with the current stack pointer. + * Each stack has a pointer to the next one. + * + * x86-64 can have several stacks: + * - task stack + * - interrupt stack + * - HW exception stacks (double fault, nmi, debug, mce) + * + * x86-32 can have up to three stacks: + * - task stack + * - softirq stack + * - hardirq stack + */ + for (; stack; stack = stack_info.next_sp) { + const char *str_begin, *str_end; + + /* + * If we overflowed the task stack into a guard page, jump back + * to the bottom of the usable stack. + */ + if (task_stack_page(task) - (void *)stack < PAGE_SIZE) + stack = task_stack_page(task); + + if (get_stack_info(stack, task, &stack_info, &visit_mask)) + break; + + stack_type_str(stack_info.type, &str_begin, &str_end); + if (str_begin) + printk("%s <%s> ", log_lvl, str_begin); + + /* + * Scan the stack, printing any text addresses we find. At the + * same time, follow proper stack frames with the unwinder. + * + * Addresses found during the scan which are not reported by + * the unwinder are considered to be additional clues which are + * sometimes useful for debugging and are prefixed with '?'. + * This also serves as a failsafe option in case the unwinder + * goes off in the weeds. + */ + for (; stack < stack_info.end; stack++) { + unsigned long real_addr; + int reliable = 0; + unsigned long addr = *stack; + unsigned long *ret_addr_p = + unwind_get_return_address_ptr(&state); + + if (!__kernel_text_address(addr)) + continue; + + if (stack == ret_addr_p) + reliable = 1; + + /* + * When function graph tracing is enabled for a + * function, its return address on the stack is + * replaced with the address of an ftrace handler + * (return_to_handler). In that case, before printing + * the "real" address, we want to print the handler + * address as an "unreliable" hint that function graph + * tracing was involved. + */ + real_addr = ftrace_graph_ret_addr(task, &graph_idx, + addr, stack); + if (real_addr != addr) + printk_stack_address(addr, 0, log_lvl); + printk_stack_address(real_addr, reliable, log_lvl); + + if (!reliable) + continue; + + /* + * Get the next frame from the unwinder. No need to + * check for an error: if anything goes wrong, the rest + * of the addresses will just be printed as unreliable. + */ + unwind_next_frame(&state); + } + + if (str_end) + printk("%s <%s> ", log_lvl, str_end); + } } void show_stack(struct task_struct *task, unsigned long *sp) { - unsigned long bp = 0; - task = task ? : current; /* * Stack frames below this one aren't interesting. Don't show them * if we're printing for %current. */ - if (!sp && task == current) { + if (!sp && task == current) sp = get_stack_pointer(current, NULL); - bp = (unsigned long)get_frame_pointer(current, NULL); - } - show_stack_log_lvl(task, NULL, sp, bp, ""); + show_stack_log_lvl(current, NULL, sp, ""); } void show_stack_regs(struct pt_regs *regs) { - show_stack_log_lvl(current, regs, NULL, 0, ""); + show_stack_log_lvl(current, regs, NULL, ""); } static arch_spinlock_t die_lock = __ARCH_SPIN_LOCK_UNLOCKED; diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c index 4ff000811e03..e476eb774278 100644 --- a/arch/x86/kernel/dumpstack_32.c +++ b/arch/x86/kernel/dumpstack_32.c @@ -156,9 +156,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs, } EXPORT_SYMBOL(dump_trace); -void -show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *sp, unsigned long bp, char *log_lvl) +void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, + unsigned long *sp, char *log_lvl) { unsigned long *stack; int i; @@ -181,7 +180,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, touch_nmi_watchdog(); } pr_cont("\n"); - show_trace_log_lvl(task, regs, sp, bp, log_lvl); + show_trace_log_lvl(task, regs, sp, log_lvl); put_task_stack(task); } @@ -205,7 +204,7 @@ void show_regs(struct pt_regs *regs) u8 *ip; pr_emerg("Stack:\n"); - show_stack_log_lvl(current, regs, NULL, 0, KERN_EMERG); + show_stack_log_lvl(current, regs, NULL, KERN_EMERG); pr_emerg("Code:"); diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c index 008a29837cab..4e9f2cf64ac8 100644 --- a/arch/x86/kernel/dumpstack_64.c +++ b/arch/x86/kernel/dumpstack_64.c @@ -209,9 +209,8 @@ void dump_trace(struct task_struct *task, struct pt_regs *regs, } EXPORT_SYMBOL(dump_trace); -void -show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, - unsigned long *sp, unsigned long bp, char *log_lvl) +void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, + unsigned long *sp, char *log_lvl) { unsigned long *irq_stack_end; unsigned long *irq_stack; @@ -255,7 +254,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, } pr_cont("\n"); - show_trace_log_lvl(task, regs, sp, bp, log_lvl); + show_trace_log_lvl(task, regs, sp, log_lvl); put_task_stack(task); } @@ -278,7 +277,7 @@ void show_regs(struct pt_regs *regs) u8 *ip; printk(KERN_DEFAULT "Stack:\n"); - show_stack_log_lvl(current, regs, NULL, 0, KERN_DEFAULT); + show_stack_log_lvl(current, regs, NULL, KERN_DEFAULT); printk(KERN_DEFAULT "Code: "); -- cgit v1.2.3 From c8fe4609827aedc9c4b45de80e7cdc8ccfa8541b Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Fri, 16 Sep 2016 14:18:17 -0500 Subject: x86/dumpstack: Remove dump_trace() and related callbacks All previous users of dump_trace() have been converted to use the new unwind interfaces, so we can remove it and the related print_context_stack() and print_context_stack_bp() callback functions. Signed-off-by: Josh Poimboeuf Cc: Andy Lutomirski Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Byungchul Park Cc: Denys Vlasenko Cc: Frederic Weisbecker Cc: H. Peter Anvin Cc: Kees Cook Cc: Linus Torvalds Cc: Nilay Vaish Cc: Peter Zijlstra Cc: Steven Rostedt Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/5b97da3572b40b5a4d8e185cf2429308d0987a13.1474045023.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/include/asm/stacktrace.h | 36 ---------------- arch/x86/kernel/dumpstack.c | 86 --------------------------------------- arch/x86/kernel/dumpstack_32.c | 35 ---------------- arch/x86/kernel/dumpstack_64.c | 69 ------------------------------- 4 files changed, 226 deletions(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h index c9ccf0676ca6..37f2e0b377ad 100644 --- a/arch/x86/include/asm/stacktrace.h +++ b/arch/x86/include/asm/stacktrace.h @@ -45,42 +45,6 @@ static inline bool on_stack(struct stack_info *info, void *addr, size_t len) extern int kstack_depth_to_print; -struct thread_info; -struct stacktrace_ops; - -typedef unsigned long (*walk_stack_t)(struct task_struct *task, - unsigned long *stack, - unsigned long bp, - const struct stacktrace_ops *ops, - void *data, - struct stack_info *info, - int *graph); - -extern unsigned long -print_context_stack(struct task_struct *task, - unsigned long *stack, unsigned long bp, - const struct stacktrace_ops *ops, void *data, - struct stack_info *info, int *graph); - -extern unsigned long -print_context_stack_bp(struct task_struct *task, - unsigned long *stack, unsigned long bp, - const struct stacktrace_ops *ops, void *data, - struct stack_info *info, int *graph); - -/* Generic stack tracer with callbacks */ - -struct stacktrace_ops { - int (*address)(void *data, unsigned long address, int reliable); - /* On negative return stop dumping */ - int (*stack)(void *data, const char *name); - walk_stack_t walk_stack; -}; - -void dump_trace(struct task_struct *tsk, struct pt_regs *regs, - unsigned long *stack, unsigned long bp, - const struct stacktrace_ops *ops, void *data); - #ifdef CONFIG_X86_32 #define STACKSLOTS_PER_LINE 8 #else diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index c08f32ab8ace..999de3b3f7f4 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -56,92 +56,6 @@ void printk_address(unsigned long address) pr_cont(" [<%p>] %pS\n", (void *)address, (void *)address); } -/* - * x86-64 can have up to three kernel stacks: - * process stack - * interrupt stack - * severe exception (double fault, nmi, stack fault, debug, mce) hardware stack - */ - -unsigned long -print_context_stack(struct task_struct *task, - unsigned long *stack, unsigned long bp, - const struct stacktrace_ops *ops, void *data, - struct stack_info *info, int *graph) -{ - struct stack_frame *frame = (struct stack_frame *)bp; - - /* - * If we overflowed the stack into a guard page, jump back to the - * bottom of the usable stack. - */ - if ((unsigned long)task_stack_page(task) - (unsigned long)stack < - PAGE_SIZE) - stack = (unsigned long *)task_stack_page(task); - - while (on_stack(info, stack, sizeof(*stack))) { - unsigned long addr = *stack; - - if (__kernel_text_address(addr)) { - unsigned long real_addr; - int reliable = 0; - - if ((unsigned long) stack == bp + sizeof(long)) { - reliable = 1; - frame = frame->next_frame; - bp = (unsigned long) frame; - } - - /* - * When function graph tracing is enabled for a - * function, its return address on the stack is - * replaced with the address of an ftrace handler - * (return_to_handler). In that case, before printing - * the "real" address, we want to print the handler - * address as an "unreliable" hint that function graph - * tracing was involved. - */ - real_addr = ftrace_graph_ret_addr(task, graph, addr, - stack); - if (real_addr != addr) - ops->address(data, addr, 0); - - ops->address(data, real_addr, reliable); - } - stack++; - } - return bp; -} -EXPORT_SYMBOL_GPL(print_context_stack); - -unsigned long -print_context_stack_bp(struct task_struct *task, - unsigned long *stack, unsigned long bp, - const struct stacktrace_ops *ops, void *data, - struct stack_info *info, int *graph) -{ - struct stack_frame *frame = (struct stack_frame *)bp; - unsigned long *retp = &frame->return_address; - - while (on_stack(info, stack, sizeof(*stack) * 2)) { - unsigned long addr = *retp; - unsigned long real_addr; - - if (!__kernel_text_address(addr)) - break; - - real_addr = ftrace_graph_ret_addr(task, graph, addr, retp); - if (ops->address(data, real_addr, 1)) - break; - - frame = frame->next_frame; - retp = &frame->return_address; - } - - return (unsigned long)frame; -} -EXPORT_SYMBOL_GPL(print_context_stack_bp); - void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, unsigned long *stack, char *log_lvl) { diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c index e476eb774278..06eb322b5f9f 100644 --- a/arch/x86/kernel/dumpstack_32.c +++ b/arch/x86/kernel/dumpstack_32.c @@ -121,41 +121,6 @@ unknown: return -EINVAL; } -void dump_trace(struct task_struct *task, struct pt_regs *regs, - unsigned long *stack, unsigned long bp, - const struct stacktrace_ops *ops, void *data) -{ - unsigned long visit_mask = 0; - int graph = 0; - - task = task ? : current; - stack = stack ? : get_stack_pointer(task, regs); - bp = bp ? : (unsigned long)get_frame_pointer(task, regs); - - for (;;) { - const char *begin_str, *end_str; - struct stack_info info; - - if (get_stack_info(stack, task, &info, &visit_mask)) - break; - - stack_type_str(info.type, &begin_str, &end_str); - - if (begin_str && ops->stack(data, begin_str) < 0) - break; - - bp = ops->walk_stack(task, stack, bp, ops, data, &info, &graph); - - if (end_str && ops->stack(data, end_str) < 0) - break; - - stack = info.next_sp; - - touch_nmi_watchdog(); - } -} -EXPORT_SYMBOL(dump_trace); - void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, unsigned long *sp, char *log_lvl) { diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c index 4e9f2cf64ac8..36cf1a498227 100644 --- a/arch/x86/kernel/dumpstack_64.c +++ b/arch/x86/kernel/dumpstack_64.c @@ -140,75 +140,6 @@ unknown: return -EINVAL; } -/* - * x86-64 can have up to three kernel stacks: - * process stack - * interrupt stack - * severe exception (double fault, nmi, stack fault, debug, mce) hardware stack - */ - -void dump_trace(struct task_struct *task, struct pt_regs *regs, - unsigned long *stack, unsigned long bp, - const struct stacktrace_ops *ops, void *data) -{ - unsigned long visit_mask = 0; - struct stack_info info; - int graph = 0; - int done = 0; - - task = task ? : current; - stack = stack ? : get_stack_pointer(task, regs); - bp = bp ? : (unsigned long)get_frame_pointer(task, regs); - - /* - * Print function call entries in all stacks, starting at the - * current stack address. If the stacks consist of nested - * exceptions - */ - while (!done) { - const char *begin_str, *end_str; - - get_stack_info(stack, task, &info, &visit_mask); - - /* Default finish unless specified to continue */ - done = 1; - - switch (info.type) { - - /* Break out early if we are on the thread stack */ - case STACK_TYPE_TASK: - break; - - case STACK_TYPE_IRQ: - case STACK_TYPE_EXCEPTION ... STACK_TYPE_EXCEPTION_LAST: - - stack_type_str(info.type, &begin_str, &end_str); - - if (ops->stack(data, begin_str) < 0) - break; - - bp = ops->walk_stack(task, stack, bp, ops, - data, &info, &graph); - - ops->stack(data, end_str); - - stack = info.next_sp; - done = 0; - break; - - default: - ops->stack(data, "UNK"); - break; - } - } - - /* - * This handles the process stack: - */ - bp = ops->walk_stack(task, stack, bp, ops, data, &info, &graph); -} -EXPORT_SYMBOL(dump_trace); - void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs, unsigned long *sp, char *log_lvl) { -- cgit v1.2.3 From 71f5443ebb1227c22e8decbcd28a1ea6deaf8257 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Tue, 20 Sep 2016 10:53:40 -0500 Subject: x86/dumpstack: Fix show_stack() task pointer regression With the following commit: e18bcccd1a4e ("x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder") The task pointer argument to show_stack_log_lvl() in show_stack() was inadvertently changed to 'current'. Signed-off-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 Cc: byungchul.park@lge.com Cc: fweisbec@gmail.com Cc: keescook@chromium.org Cc: linux-tip-commits@vger.kernel.org Cc: luto@amacapital.net Cc: nilayvaish@gmail.com Cc: rostedt@goodmis.org Cc: tip-bot for Josh Poimboeuf Fixes: e18bcccd1a4e ("x86/dumpstack: Convert show_trace_log_lvl() to use the new unwinder") Link: http://lkml.kernel.org/r/20160920155340.yhewlx7vmgmov5fb@treble Signed-off-by: Ingo Molnar --- arch/x86/kernel/dumpstack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch/x86/kernel/dumpstack.c') diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c index 999de3b3f7f4..9b7cf5c28f5f 100644 --- a/arch/x86/kernel/dumpstack.c +++ b/arch/x86/kernel/dumpstack.c @@ -164,7 +164,7 @@ void show_stack(struct task_struct *task, unsigned long *sp) if (!sp && task == current) sp = get_stack_pointer(current, NULL); - show_stack_log_lvl(current, NULL, sp, ""); + show_stack_log_lvl(task, NULL, sp, ""); } void show_stack_regs(struct pt_regs *regs) -- cgit v1.2.3