x86/unwind: an unwinder fix and debugging improvement
Subsystems (2)
cover x86/unwind: an unwinder fix and debugging improvement
Patch 1 is a fix for an unwinder issue found by Boris. Patch 2 is a debugging improvement which helped diagnose the problem.
Patch 1 needs to go into tip/urgent for 4.10. I'd argue that patch 2 should also go into urgent because it only affects the error path and it makes the new unwinder warnings much more useful.
Josh Poimboeuf (2):
x86/unwind: adjust last frame check for aligned function stacks x86/unwind: dump stack data on warnings
arch/x86/include/asm/unwind.h | 2 +- arch/x86/kernel/unwind_frame.c | 47 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/2 x86/unwind: adjust last frame check for aligned function stacks
Somehow, CONFIG_PARAVIRT=n convinces gcc to change the x86_64_start_kernel() prologue from:
0000000000000129 <x86_64_start_kernel>:
129: 55 push %rbp 12a: 48 89 e5 mov %rsp,%rbp
to:
0000000000000124 <x86_64_start_kernel>:
124: 4c 8d 54 24 08 lea 0x8(%rsp),%r10 129: 48 83 e4 f0 and $0xfffffffffffffff0,%rsp 12d: 41 ff 72 f8 pushq -0x8(%r10) 131: 55 push %rbp 132: 48 89 e5 mov %rsp,%rbp
This is an unusual pattern which aligns rsp (though in this case it's already aligned) and saves the start_cpu() return address again on the stack before storing the frame pointer.
The unwinder assumes the last stack frame header is at a certain offset, but the above code breaks that assumption, resulting in the following warning:
WARNING: kernel stack frame pointer at ffffffff82e03f40 in swapper:0 has bad value (null)
Fix it by checking for the last task stack frame at the aligned offset in addition to the normal unaligned offset.
--- arch/x86/kernel/unwind_frame.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
arch/x86/kernel/unwind_frame.c +8 -1
@@ -46,7 +46,14 @@ static bool is_last_task_frame(struct unwind_state *state)unsigned long bp = (unsigned long )state->bp;unsigned long regs = (unsigned long )task_pt_regs (state->task); -return bp == regs -FRAME_HEADER_SIZE ; +/* +* We have to check for the last task frame at two different locations +* because gcc can occasionally decide to realign the stack pointer and +* change the offset of the stack frame by a word in the prologue of a +* function called by head/entry code. +*/ +return bp == regs -FRAME_HEADER_SIZE || + bp == regs -FRAME_HEADER_SIZE -sizeof (long ); }/*
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/urgent] x86/unwind: Adjust last frame check for aligned function stacks
x86/unwind: Adjust last frame check for aligned function stacks
Commit-ID: 8023e0e2a48d45e8d5363081fad9f7ed4402f953 Gitweb: http://git.kernel.org/tip/8023e0e2a48d45e8d5363081fad9f7ed4402f953 Author: Josh Poimboeuf <[email protected]> AuthorDate: Fri, 16 Dec 2016 10:05:05 -0600 Committer: Thomas Gleixner <[email protected]> CommitDate: Mon, 19 Dec 2016 11:47:05 +0100
x86/unwind: Adjust last frame check for aligned function stacks
Somehow, CONFIG_PARAVIRT=n convinces gcc to change the x86_64_start_kernel() prologue from:
0000000000000129 <x86_64_start_kernel>:
129: 55 push %rbp 12a: 48 89 e5 mov %rsp,%rbp
to:
0000000000000124 <x86_64_start_kernel>:
124: 4c 8d 54 24 08 lea 0x8(%rsp),%r10 129: 48 83 e4 f0 and $0xfffffffffffffff0,%rsp 12d: 41 ff 72 f8 pushq -0x8(%r10) 131: 55 push %rbp 132: 48 89 e5 mov %rsp,%rbp
This is an unusual pattern which aligns rsp (though in this case it's already aligned) and saves the start_cpu() return address again on the stack before storing the frame pointer.
The unwinder assumes the last stack frame header is at a certain offset, but the above code breaks that assumption, resulting in the following warning:
WARNING: kernel stack frame pointer at ffffffff82e03f40 in swapper:0 has bad value (null)
Fix it by checking for the last task stack frame at the aligned offset in addition to the normal unaligned offset.
--- arch/x86/kernel/unwind_frame.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
arch/x86/kernel/unwind_frame.c +8 -1
@@ -46,7 +46,14 @@ static bool is_last_task_frame(struct unwind_state *state)unsigned long bp = (unsigned long )state->bp;unsigned long regs = (unsigned long )task_pt_regs (state->task); -return bp == regs -FRAME_HEADER_SIZE ; +/* +* We have to check for the last task frame at two different locations +* because gcc can occasionally decide to realign the stack pointer and +* change the offset of the stack frame by a word in the prologue of a +* function called by head/entry code. +*/ +return bp == regs -FRAME_HEADER_SIZE || + bp == regs -FRAME_HEADER_SIZE -sizeof (long ); }/*
PATCH 2/2 x86/unwind: dump stack data on warnings
In reply to Josh PoimboeufThe unwinder warnings are good at finding unexpected unwinder issues, but they often don't give enough data to be able to fully diagnose them. Print a one-time stack dump when a warning is detected.
--- arch/x86/include/asm/unwind.h | 2 +- arch/x86/kernel/unwind_frame.c | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-)
arch/x86/include/asm/unwind.h +1 -1
@@ -12,7 +12,7 @@ struct unwind_state {struct task_struct *task;int graph_idx;#ifdef CONFIG_FRAME_POINTER -unsigned long *bp; +unsigned long *bp, *orig_sp;struct pt_regs *regs;#else unsigned long *sp;
arch/x86/kernel/unwind_frame.c +38 -0
@@ -6,6 +6,37 @@#define FRAME_HEADER_SIZE (sizeof(long) * 2) +static void unwind_dump (struct unwind_state *state,unsigned long *sp) +{ +static bool dumped_before = false; +bool prev_zero, zero = false; +unsigned long word; + +if (dumped_before) +return ; + + dumped_before = true; + +printk_deferred ("unwind stack type:%d next_sp:%p mask:%lx graph_idx:%d\n" , + state->stack_info.type, state->stack_info.next_sp, + state->stack_mask, state->graph_idx); + +for (sp = state->orig_sp; sp < state->stack_info.end; sp++) { + word =READ_ONCE_NOCHECK (*sp); + + prev_zero = zero; + zero = word ==0 ; + +if (zero) { +if (!prev_zero) +printk_deferred ("%p: %016x ...\n" , sp,0 ); +continue ; + } + +printk_deferred ("%p: %016lx (%pB)\n" , sp, word, (void *)word); + } +} +unsigned long unwind_get_return_address (struct unwind_state *state) {unsigned long addr;
@@ -25,6 +56,7 @@ unsigned long unwind_get_return_address(struct unwind_state *state)"WARNING: unrecognized kernel stack return address %p at %p in %s:%d\n" , (void *)addr, addr_p, state->task->comm, state->task->pid); +unwind_dump (state, addr_p);return 0 ; }
@@ -74,6 +106,7 @@ static bool update_stack_state(struct unwind_state *state, void *addr, size_t len) {struct stack_info *info = &state->stack_info; +enum stack_type orig_type = info->type;/* * If addr isn't on the current stack, switch to the next one.
@@ -87,6 +120,9 @@ static bool update_stack_state(struct unwind_state *state, void *addr, &state->stack_mask))return false; +if (!state->orig_sp || info->type != orig_type) + state->orig_sp = addr; +return true; }
@@ -185,11 +221,13 @@ bool unwind_next_frame(struct unwind_state *state)"WARNING: kernel stack regs at %p in %s:%d has bad 'bp' value %p\n" , state->regs, state->task->comm, state->task->pid, next_frame); +unwind_dump (state, (unsigned long *)state->regs); }else {printk_deferred_once (KERN_WARNING "WARNING: kernel stack frame pointer at %p in %s:%d has bad value %p\n" , state->bp, state->task->comm, state->task->pid, next_frame); +unwind_dump (state, state->bp); } the_end: state->stack_info.type =STACK_TYPE_UNKNOWN ;
Signature · 1 line
2.7.4
tip-bot for Josh Poimboeuf
merge bot[tip:x86/urgent] x86/unwind: Dump stack data on warnings
x86/unwind: Dump stack data on warnings
Commit-ID: 8b5e99f02264130782a10ba5c0c759797fb064ee Gitweb: http://git.kernel.org/tip/8b5e99f02264130782a10ba5c0c759797fb064ee Author: Josh Poimboeuf <[email protected]> AuthorDate: Fri, 16 Dec 2016 10:05:06 -0600 Committer: Thomas Gleixner <[email protected]> CommitDate: Mon, 19 Dec 2016 11:47:05 +0100
x86/unwind: Dump stack data on warnings
The unwinder warnings are good at finding unexpected unwinder issues, but they often don't give enough data to be able to fully diagnose them. Print a one-time stack dump when a warning is detected.
--- arch/x86/include/asm/unwind.h | 2 +- arch/x86/kernel/unwind_frame.c | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-)
arch/x86/include/asm/unwind.h +1 -1
@@ -12,7 +12,7 @@ struct unwind_state {struct task_struct *task;int graph_idx;#ifdef CONFIG_FRAME_POINTER -unsigned long *bp; +unsigned long *bp, *orig_sp;struct pt_regs *regs;#else unsigned long *sp;
arch/x86/kernel/unwind_frame.c +38 -0
@@ -6,6 +6,37 @@#define FRAME_HEADER_SIZE (sizeof(long) * 2) +static void unwind_dump (struct unwind_state *state,unsigned long *sp) +{ +static bool dumped_before = false; +bool prev_zero, zero = false; +unsigned long word; + +if (dumped_before) +return ; + + dumped_before = true; + +printk_deferred ("unwind stack type:%d next_sp:%p mask:%lx graph_idx:%d\n" , + state->stack_info.type, state->stack_info.next_sp, + state->stack_mask, state->graph_idx); + +for (sp = state->orig_sp; sp < state->stack_info.end; sp++) { + word =READ_ONCE_NOCHECK (*sp); + + prev_zero = zero; + zero = word ==0 ; + +if (zero) { +if (!prev_zero) +printk_deferred ("%p: %016x ...\n" , sp,0 ); +continue ; + } + +printk_deferred ("%p: %016lx (%pB)\n" , sp, word, (void *)word); + } +} +unsigned long unwind_get_return_address (struct unwind_state *state) {unsigned long addr;
@@ -25,6 +56,7 @@ unsigned long unwind_get_return_address(struct unwind_state *state)"WARNING: unrecognized kernel stack return address %p at %p in %s:%d\n" , (void *)addr, addr_p, state->task->comm, state->task->pid); +unwind_dump (state, addr_p);return 0 ; }
@@ -74,6 +106,7 @@ static bool update_stack_state(struct unwind_state *state, void *addr, size_t len) {struct stack_info *info = &state->stack_info; +enum stack_type orig_type = info->type;/* * If addr isn't on the current stack, switch to the next one.
@@ -87,6 +120,9 @@ static bool update_stack_state(struct unwind_state *state, void *addr, &state->stack_mask))return false; +if (!state->orig_sp || info->type != orig_type) + state->orig_sp = addr; +return true; }
@@ -185,11 +221,13 @@ bad_address:"WARNING: kernel stack regs at %p in %s:%d has bad 'bp' value %p\n" , state->regs, state->task->comm, state->task->pid, next_frame); +unwind_dump (state, (unsigned long *)state->regs); }else {printk_deferred_once (KERN_WARNING "WARNING: kernel stack frame pointer at %p in %s:%d has bad value %p\n" , state->bp, state->task->comm, state->task->pid, next_frame); +unwind_dump (state, state->bp); } the_end: state->stack_info.type =STACK_TYPE_UNKNOWN ;