x86/dumpstack: yet more stack dump improvements
Subsystems (3)
cover x86/dumpstack: yet more stack dump improvements
This is the last batch before the new unwinder.
Josh Poimboeuf (4):
x86/dumpstack: simplify in_exception_stack() x86/dumpstack: add get_stack_info() interface x86/dumpstack: support for unwinding empty irq stacks x86/dumpstack: add recursion checking for all stacks
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 | 130 ++++++++++++++++---- arch/x86/kernel/dumpstack_64.c | 251 ++++++++++++++++++-------------------- arch/x86/kernel/stacktrace.c | 2 +- arch/x86/oprofile/backtrace.c | 2 +- 7 files changed, 284 insertions(+), 184 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/4 x86/dumpstack: simplify in_exception_stack()
in_exception_stack() does some bad, bad things just so the unwinder can print different values for different areas of the debug exception stack.
There's no need to clarify where exactly on the stack it is. Just print "#DB" and be done with it.
--- arch/x86/kernel/dumpstack_64.c | 89 ++++++++++++------------------------------ 1 file changed, 26 insertions(+), 63 deletions(-)
arch/x86/kernel/dumpstack_64.c +26 -63
@@ -16,83 +16,46 @@#include <asm/stacktrace.h> +static char *exception_stack_names[N_EXCEPTION_STACKS ] = { + [DOUBLEFAULT_STACK -1 ] ="#DF" , + [NMI_STACK -1 ] ="NMI" , + [DEBUG_STACK -1 ] ="#DB" , + [MCE_STACK -1 ] ="#MC" , +}; -#define N_EXCEPTION_STACKS_END \ - (N_EXCEPTION_STACKS + DEBUG_STKSZ/EXCEPTION_STKSZ - 2) - -static char x86_stack_ids[][8 ] = { - [DEBUG_STACK -1 ] ="#DB" , - [NMI_STACK -1 ] ="NMI" , - [DOUBLEFAULT_STACK -1 ] ="#DF" , - [MCE_STACK -1 ] ="#MC" , -#if DEBUG_STKSZ > EXCEPTION_STKSZ - [ N_EXCEPTION_STACKS ... -N_EXCEPTION_STACKS_END ] ="#DB[?]" -#endif +static unsigned long exception_stack_sizes[N_EXCEPTION_STACKS ] = { + [0 ...N_EXCEPTION_STACKS -1 ] =EXCEPTION_STKSZ , + [DEBUG_STACK -1 ] =DEBUG_STKSZ };static unsigned long *in_exception_stack (unsigned long stack,unsigned *usedp,char **idp) { +unsigned long begin, end;unsigned k; -/* -* Iterate over all exception stacks, and figure out whether -* 'stack' is in one of them: -*/ +BUILD_BUG_ON (N_EXCEPTION_STACKS !=4 ); +for (k =0 ; k <N_EXCEPTION_STACKS ; k++) { -unsigned long end =raw_cpu_ptr (&orig_ist)->ist[k]; -/* -* Is 'stack' above this exception frame's end? -* If yes then skip to the next frame. -*/ -if (stack >= end) + end =raw_cpu_ptr (&orig_ist)->ist[k]; + begin = end - exception_stack_sizes[k]; + +if (stack < begin || stack >= end)continue ; +/* -* Is 'stack' above this exception frame's start address? -* If yes then we found the right frame. -*/ -if (stack >= end -EXCEPTION_STKSZ ) { -/* -* 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 out and return NULL: -*/ -if (*usedp & (1U << k)) -break ; - *usedp |=1U << k; - *idp = x86_stack_ids[k]; -return (unsigned long *)end; - } -/* -* If this is a debug stack, and if it has a larger size than -* the usual exception stacks, then 'stack' might still -* be within the lower portion of the debug stack: +* 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: */ -#if DEBUG_STKSZ >EXCEPTION_STKSZ -if (k ==DEBUG_STACK -1 && stack >= end -DEBUG_STKSZ ) { -unsigned j =N_EXCEPTION_STACKS -1 ; +if (*usedp & (1U << k)) +break ; + *usedp |=1U << k; -/* -* Black magic. A large debug stack is composed of -* multiple exception stack entries, which we -* iterate through now. Dont look: -*/ -do { - ++j; - end -=EXCEPTION_STKSZ ; - x86_stack_ids[j][4 ] ='1' + - (j -N_EXCEPTION_STACKS ); - }while (stack < end -EXCEPTION_STKSZ ); -if (*usedp & (1U << j)) -break ; - *usedp |=1U << j; - *idp = x86_stack_ids[j]; -return (unsigned long *)end; - } -#endif + *idp = exception_stack_names[k]; +return (unsigned long *)end; } +return NULL ; }
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] x86/dumpstack: Simplify in_exception_stack()
x86/dumpstack: Simplify in_exception_stack()
Commit-ID: 9c00390757fd9f5851f7973b2f0e1e41550bb3b8 Gitweb: http://git.kernel.org/tip/9c00390757fd9f5851f7973b2f0e1e41550bb3b8 Author: Josh Poimboeuf <[email protected]> AuthorDate: Wed, 14 Sep 2016 21:07:41 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 15 Sep 2016 08:13:14 +0200
x86/dumpstack: Simplify in_exception_stack()
in_exception_stack() does some bad, bad things just so the unwinder can print different values for different areas of the debug exception stack.
There's no need to clarify where exactly on the stack it is. Just print "#DB" and be done with it.
--- arch/x86/kernel/dumpstack_64.c | 89 ++++++++++++------------------------------ 1 file changed, 26 insertions(+), 63 deletions(-)
arch/x86/kernel/dumpstack_64.c +26 -63
@@ -16,83 +16,46 @@#include <asm/stacktrace.h> +static char *exception_stack_names[N_EXCEPTION_STACKS ] = { + [DOUBLEFAULT_STACK -1 ] ="#DF" , + [NMI_STACK -1 ] ="NMI" , + [DEBUG_STACK -1 ] ="#DB" , + [MCE_STACK -1 ] ="#MC" , +}; -#define N_EXCEPTION_STACKS_END \ - (N_EXCEPTION_STACKS + DEBUG_STKSZ/EXCEPTION_STKSZ - 2) - -static char x86_stack_ids[][8 ] = { - [DEBUG_STACK -1 ] ="#DB" , - [NMI_STACK -1 ] ="NMI" , - [DOUBLEFAULT_STACK -1 ] ="#DF" , - [MCE_STACK -1 ] ="#MC" , -#if DEBUG_STKSZ > EXCEPTION_STKSZ - [ N_EXCEPTION_STACKS ... -N_EXCEPTION_STACKS_END ] ="#DB[?]" -#endif +static unsigned long exception_stack_sizes[N_EXCEPTION_STACKS ] = { + [0 ...N_EXCEPTION_STACKS -1 ] =EXCEPTION_STKSZ , + [DEBUG_STACK -1 ] =DEBUG_STKSZ };static unsigned long *in_exception_stack (unsigned long stack,unsigned *usedp,char **idp) { +unsigned long begin, end;unsigned k; -/* -* Iterate over all exception stacks, and figure out whether -* 'stack' is in one of them: -*/ +BUILD_BUG_ON (N_EXCEPTION_STACKS !=4 ); +for (k =0 ; k <N_EXCEPTION_STACKS ; k++) { -unsigned long end =raw_cpu_ptr (&orig_ist)->ist[k]; -/* -* Is 'stack' above this exception frame's end? -* If yes then skip to the next frame. -*/ -if (stack >= end) + end =raw_cpu_ptr (&orig_ist)->ist[k]; + begin = end - exception_stack_sizes[k]; + +if (stack < begin || stack >= end)continue ; +/* -* Is 'stack' above this exception frame's start address? -* If yes then we found the right frame. -*/ -if (stack >= end -EXCEPTION_STKSZ ) { -/* -* 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 out and return NULL: -*/ -if (*usedp & (1U << k)) -break ; - *usedp |=1U << k; - *idp = x86_stack_ids[k]; -return (unsigned long *)end; - } -/* -* If this is a debug stack, and if it has a larger size than -* the usual exception stacks, then 'stack' might still -* be within the lower portion of the debug stack: +* 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: */ -#if DEBUG_STKSZ >EXCEPTION_STKSZ -if (k ==DEBUG_STACK -1 && stack >= end -DEBUG_STKSZ ) { -unsigned j =N_EXCEPTION_STACKS -1 ; +if (*usedp & (1U << k)) +break ; + *usedp |=1U << k; -/* -* Black magic. A large debug stack is composed of -* multiple exception stack entries, which we -* iterate through now. Dont look: -*/ -do { - ++j; - end -=EXCEPTION_STKSZ ; - x86_stack_ids[j][4 ] ='1' + - (j -N_EXCEPTION_STACKS ); - }while (stack < end -EXCEPTION_STKSZ ); -if (*usedp & (1U << j)) -break ; - *usedp |=1U << j; - *idp = x86_stack_ids[j]; -return (unsigned long *)end; - } -#endif + *idp = exception_stack_names[k]; +return (unsigned long *)end; } +return NULL ; }
PATCH 2/4 x86/dumpstack: add get_stack_info() interface
In reply to Josh Poimboeufvalid_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.
--- 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(-)
arch/x86/events/core.c +1 -1
@@ -2271,7 +2271,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 ; }
arch/x86/include/asm/stacktrace.h +37 -4
70 lines not on this page · raw message
arch/x86/kernel/dumpstack.c +22 -18
@@ -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 ;
arch/x86/kernel/dumpstack_32.c +81 -25
142 lines not on this page · raw message
arch/x86/kernel/dumpstack_64.c +91 -78
239 lines not on this page · raw message
arch/x86/kernel/stacktrace.c +1 -1
@@ -9,7 +9,7 @@#include <linux/uaccess.h> #include <asm/stacktrace.h> -static int save_stack_stack (void *data,char *name) +static int save_stack_stack (void *data,const char *name) {return 0 ; }
arch/x86/oprofile/backtrace.c +1 -1
@@ -17,7 +17,7 @@#include <asm/ptrace.h> #include <asm/stacktrace.h> -static int backtrace_stack (void *data,char *name) +static int backtrace_stack (void *data,const char *name) {/* Yes, we want all stacks */ return 0 ;
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] x86/dumpstack: Add get_stack_info() interface
x86/dumpstack: Add get_stack_info() interface
Commit-ID: cb76c93982404273d746f3ccd5085b47689099a8 Gitweb: http://git.kernel.org/tip/cb76c93982404273d746f3ccd5085b47689099a8 Author: Josh Poimboeuf <[email protected]> AuthorDate: Wed, 14 Sep 2016 21:07:42 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 15 Sep 2016 08:13:15 +0200
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.
--- 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(-)
arch/x86/events/core.c +1 -1
@@ -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 ; }
arch/x86/include/asm/stacktrace.h +37 -4
70 lines not on this page · raw message
arch/x86/kernel/dumpstack.c +22 -18
@@ -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 ;
arch/x86/kernel/dumpstack_32.c +81 -25
142 lines not on this page · raw message
arch/x86/kernel/dumpstack_64.c +91 -78
239 lines not on this page · raw message
arch/x86/kernel/stacktrace.c +1 -1
@@ -9,7 +9,7 @@#include <linux/uaccess.h> #include <asm/stacktrace.h> -static int save_stack_stack (void *data,char *name) +static int save_stack_stack (void *data,const char *name) {return 0 ; }
arch/x86/oprofile/backtrace.c +1 -1
@@ -17,7 +17,7 @@#include <asm/ptrace.h> #include <asm/stacktrace.h> -static int backtrace_stack (void *data,char *name) +static int backtrace_stack (void *data,const char *name) {/* Yes, we want all stacks */ return 0 ;
PATCH 3/4 x86/dumpstack: support for unwinding empty irq stacks
In reply to Josh PoimboeufWhen an interrupt happens in entry code while running on a software irq stack, and the irq stack was empty, regs->sp will contain the stack end address (e.g., irq_stack_ptr). If the regs are passed to dump_trace(), get_stack_info() will report STACK_TYPE_UNKNOWN, causing dump_trace() to return prematurely without trying to go to the next stack.
Update the bounds checking for software interrupt stacks so that the ending address is now considered part of the stack.
This means that it's now possible for the 'walk_stack' callbacks -- print_context_stack() and print_context_stack_bp() -- to be called with an empty stack. But that's fine; they're already prepared to deal with that due to their on_stack() checks.
--- arch/x86/kernel/dumpstack_32.c | 12 ++++++++++-- arch/x86/kernel/dumpstack_64.c | 6 +++++- 2 files changed, 15 insertions(+), 3 deletions(-)
arch/x86/kernel/dumpstack_32.c +10 -2
@@ -35,7 +35,11 @@ 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) +/* +* This is a software stack, so 'end' can be a valid stack pointer. +* It just means the stack is empty. +*/ +if (stack < begin || stack > end)return false; info->type =STACK_TYPE_IRQ ;
@@ -56,7 +60,11 @@ static bool in_softirq_stack(unsigned long *stack, struct stack_info *info)unsigned long *begin = (unsigned long *)this_cpu_read (softirq_stack);unsigned long *end = begin + (THREAD_SIZE /sizeof (long )); -if (stack < begin || stack >= end) +/* +* This is a software stack, so 'end' can be a valid stack pointer. +* It just means the stack is empty. +*/ +if (stack < begin || stack > end)return false; info->type =STACK_TYPE_SOFTIRQ ;
arch/x86/kernel/dumpstack_64.c +5 -1
@@ -90,7 +90,11 @@ static bool in_irq_stack(unsigned long *stack, struct stack_info *info)unsigned long *end = (unsigned long *)this_cpu_read (irq_stack_ptr);unsigned long *begin = end - (IRQ_STACK_SIZE /sizeof (long )); -if (stack < begin || stack >= end) +/* +* This is a software stack, so 'end' can be a valid stack pointer. +* It just means the stack is empty. +*/ +if (stack < begin || stack > end)return false; info->type =STACK_TYPE_IRQ ;
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] x86/dumpstack: Add support for unwinding empty IRQ stacks
x86/dumpstack: Add support for unwinding empty IRQ stacks
Commit-ID: 5fe599e02e41550c59831613a11c8ae057897c29 Gitweb: http://git.kernel.org/tip/5fe599e02e41550c59831613a11c8ae057897c29 Author: Josh Poimboeuf <[email protected]> AuthorDate: Wed, 14 Sep 2016 21:07:43 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 15 Sep 2016 08:13:15 +0200
x86/dumpstack: Add support for unwinding empty IRQ stacks
When an interrupt happens in entry code while running on a software IRQ stack, and the IRQ stack was empty, regs->sp will contain the stack end address (e.g., irq_stack_ptr). If the regs are passed to dump_trace(), get_stack_info() will report STACK_TYPE_UNKNOWN, causing dump_trace() to return prematurely without trying to go to the next stack.
Update the bounds checking for software interrupt stacks so that the ending address is now considered part of the stack.
This means that it's now possible for the 'walk_stack' callbacks -- print_context_stack() and print_context_stack_bp() -- to be called with an empty stack. But that's fine; they're already prepared to deal with that due to their on_stack() checks.
--- arch/x86/kernel/dumpstack_32.c | 12 ++++++++++-- arch/x86/kernel/dumpstack_64.c | 6 +++++- 2 files changed, 15 insertions(+), 3 deletions(-)
arch/x86/kernel/dumpstack_32.c +10 -2
@@ -35,7 +35,11 @@ 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) +/* +* This is a software stack, so 'end' can be a valid stack pointer. +* It just means the stack is empty. +*/ +if (stack < begin || stack > end)return false; info->type =STACK_TYPE_IRQ ;
@@ -56,7 +60,11 @@ static bool in_softirq_stack(unsigned long *stack, struct stack_info *info)unsigned long *begin = (unsigned long *)this_cpu_read (softirq_stack);unsigned long *end = begin + (THREAD_SIZE /sizeof (long )); -if (stack < begin || stack >= end) +/* +* This is a software stack, so 'end' can be a valid stack pointer. +* It just means the stack is empty. +*/ +if (stack < begin || stack > end)return false; info->type =STACK_TYPE_SOFTIRQ ;
arch/x86/kernel/dumpstack_64.c +5 -1
@@ -90,7 +90,11 @@ static bool in_irq_stack(unsigned long *stack, struct stack_info *info)unsigned long *end = (unsigned long *)this_cpu_read (irq_stack_ptr);unsigned long *begin = end - (IRQ_STACK_SIZE /sizeof (long )); -if (stack < begin || stack >= end) +/* +* This is a software stack, so 'end' can be a valid stack pointer. +* It just means the stack is empty. +*/ +if (stack < begin || stack > end)return false; info->type =STACK_TYPE_IRQ ;
PATCH 4/4 x86/dumpstack: add recursion checking for all stacks
In reply to Josh Poimboeufin_exception_stack() has some recursion checking which makes sure the stack trace code never traverses a given exception stack more than once. This prevents an infinite loop if corruption somehow causes a stack's "next stack" pointer to point to itself (directly or indirectly).
The recursion checking can be useful for other stacks in addition to the exception stack, so extend it to work for all stacks.
--- arch/x86/kernel/dumpstack_32.c | 22 +++++++++++++++++++--- arch/x86/kernel/dumpstack_64.c | 35 +++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 19 deletions(-)
arch/x86/kernel/dumpstack_32.c +19 -3
@@ -89,16 +89,32 @@ int get_stack_info(unsigned long *stack, struct task_struct *task, task = task ? : current;if (in_task_stack (stack, task, info)) -return 0 ; + goto recursion_check;if (task != current) goto unknown;if (in_hardirq_stack (stack, info)) -return 0 ; + goto recursion_check;if (in_softirq_stack (stack, info)) -return 0 ; + goto recursion_check; + + goto unknown; + +recursion_check: +/* +* Make sure we don't iterate through any given 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 (visit_mask) { +if (*visit_mask & (1UL << info->type)) + goto unknown; + *visit_mask |=1UL << info->type; + } + +return 0 ; unknown: info->type =STACK_TYPE_UNKNOWN ;
arch/x86/kernel/dumpstack_64.c +19 -16
@@ -47,8 +47,7 @@ void stack_type_str(enum stack_type type, const char **begin, const char **end) } } -static bool in_exception_stack (unsigned long *stack,struct stack_info *info, -unsigned long *visit_mask) +static bool in_exception_stack (unsigned long *stack,struct stack_info *info) {unsigned long *begin, *end;struct pt_regs *regs;
@@ -64,16 +63,6 @@ static bool in_exception_stack(unsigned long *stack, struct stack_info *info,if (stack < begin || stack >= end)continue ; -/* -* 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 (*visit_mask & (1U << k)) -break ; - *visit_mask |=1U << k; - info->type =STACK_TYPE_EXCEPTION + k; info->begin = begin; info->end = end;
@@ -119,16 +108,30 @@ int get_stack_info(unsigned long *stack, struct task_struct *task, task = task ? : current;if (in_task_stack (stack, task, info)) -return 0 ; + goto recursion_check;if (task != current) goto unknown; -if (in_exception_stack (stack, info, visit_mask)) -return 0 ; +if (in_exception_stack (stack, info)) + goto recursion_check;if (in_irq_stack (stack, info)) -return 0 ; + goto recursion_check; + + goto unknown; + +recursion_check: +/* +* Make sure we don't iterate through any given 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 (visit_mask) { +if (*visit_mask & (1UL << info->type)) + goto unknown; + *visit_mask |=1UL << info->type; + }return 0 ;
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/asm] x86/dumpstack: Add recursion checking for all stacks
x86/dumpstack: Add recursion checking for all stacks
Commit-ID: fcd709ef20a9d83bdb7524d27cd6719dac8690a0 Gitweb: http://git.kernel.org/tip/fcd709ef20a9d83bdb7524d27cd6719dac8690a0 Author: Josh Poimboeuf <[email protected]> AuthorDate: Wed, 14 Sep 2016 21:07:44 -0500 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 15 Sep 2016 08:13:15 +0200
x86/dumpstack: Add recursion checking for all stacks
in_exception_stack() has some recursion checking which makes sure the stack trace code never traverses a given exception stack more than once. This prevents an infinite loop if corruption somehow causes a stack's "next stack" pointer to point to itself (directly or indirectly).
The recursion checking can be useful for other stacks in addition to the exception stack, so extend it to work for all stacks.
--- arch/x86/kernel/dumpstack_32.c | 22 +++++++++++++++++++--- arch/x86/kernel/dumpstack_64.c | 35 +++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 19 deletions(-)
arch/x86/kernel/dumpstack_32.c +19 -3
@@ -89,16 +89,32 @@ int get_stack_info(unsigned long *stack, struct task_struct *task, task = task ? : current;if (in_task_stack (stack, task, info)) -return 0 ; + goto recursion_check;if (task != current) goto unknown;if (in_hardirq_stack (stack, info)) -return 0 ; + goto recursion_check;if (in_softirq_stack (stack, info)) -return 0 ; + goto recursion_check; + + goto unknown; + +recursion_check: +/* +* Make sure we don't iterate through any given 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 (visit_mask) { +if (*visit_mask & (1UL << info->type)) + goto unknown; + *visit_mask |=1UL << info->type; + } + +return 0 ; unknown: info->type =STACK_TYPE_UNKNOWN ;
arch/x86/kernel/dumpstack_64.c +19 -16
@@ -47,8 +47,7 @@ void stack_type_str(enum stack_type type, const char **begin, const char **end) } } -static bool in_exception_stack (unsigned long *stack,struct stack_info *info, -unsigned long *visit_mask) +static bool in_exception_stack (unsigned long *stack,struct stack_info *info) {unsigned long *begin, *end;struct pt_regs *regs;
@@ -64,16 +63,6 @@ static bool in_exception_stack(unsigned long *stack, struct stack_info *info,if (stack < begin || stack >= end)continue ; -/* -* 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 (*visit_mask & (1U << k)) -break ; - *visit_mask |=1U << k; - info->type =STACK_TYPE_EXCEPTION + k; info->begin = begin; info->end = end;
@@ -119,16 +108,30 @@ int get_stack_info(unsigned long *stack, struct task_struct *task, task = task ? : current;if (in_task_stack (stack, task, info)) -return 0 ; + goto recursion_check;if (task != current) goto unknown; -if (in_exception_stack (stack, info, visit_mask)) -return 0 ; +if (in_exception_stack (stack, info)) + goto recursion_check;if (in_irq_stack (stack, info)) -return 0 ; + goto recursion_check; + + goto unknown; + +recursion_check: +/* +* Make sure we don't iterate through any given 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 (visit_mask) { +if (*visit_mask & (1UL << info->type)) + goto unknown; + *visit_mask |=1UL << info->type; + }return 0 ;