| From 479b966c9a5590a63494aa23395a2c3327f88443 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 25 Mar 2025 03:01:23 +0100 |
| Subject: x86/dumpstack: Fix inaccurate unwinding from exception stacks due to |
| misplaced assignment |
| |
| From: Jann Horn <jannh@google.com> |
| |
| [ Upstream commit 2c118f50d7fd4d9aefc4533a26f83338b2906b7a ] |
| |
| Commit: |
| |
| 2e4be0d011f2 ("x86/show_trace_log_lvl: Ensure stack pointer is aligned, again") |
| |
| was intended to ensure alignment of the stack pointer; but it also moved |
| the initialization of the "stack" variable down into the loop header. |
| |
| This was likely intended as a no-op cleanup, since the commit |
| message does not mention it; however, this caused a behavioral change |
| because the value of "regs" is different between the two places. |
| |
| Originally, get_stack_pointer() used the regs provided by the caller; after |
| that commit, get_stack_pointer() instead uses the regs at the top of the |
| stack frame the unwinder is looking at. Often, there are no such regs at |
| all, and "regs" is NULL, causing get_stack_pointer() to fall back to the |
| task's current stack pointer, which is not what we want here, but probably |
| happens to mostly work. Other times, the original regs will point to |
| another regs frame - in that case, the linear guess unwind logic in |
| show_trace_log_lvl() will start unwinding too far up the stack, causing the |
| first frame found by the proper unwinder to never be visited, resulting in |
| a stack trace consisting purely of guess lines. |
| |
| Fix it by moving the "stack = " assignment back where it belongs. |
| |
| Fixes: 2e4be0d011f2 ("x86/show_trace_log_lvl: Ensure stack pointer is aligned, again") |
| Signed-off-by: Jann Horn <jannh@google.com> |
| Signed-off-by: Ingo Molnar <mingo@kernel.org> |
| Link: https://lore.kernel.org/r/20250325-2025-03-unwind-fixes-v1-2-acd774364768@google.com |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| arch/x86/kernel/dumpstack.c | 5 ++--- |
| 1 file changed, 2 insertions(+), 3 deletions(-) |
| |
| diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c |
| index f18ca44c904b7..52dc5839d1e8e 100644 |
| --- a/arch/x86/kernel/dumpstack.c |
| +++ b/arch/x86/kernel/dumpstack.c |
| @@ -195,6 +195,7 @@ static void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, |
| printk("%sCall Trace:\n", log_lvl); |
| |
| unwind_start(&state, task, regs, stack); |
| + stack = stack ?: get_stack_pointer(task, regs); |
| regs = unwind_get_entry_regs(&state, &partial); |
| |
| /* |
| @@ -213,9 +214,7 @@ static void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs, |
| * - hardirq stack |
| * - entry stack |
| */ |
| - for (stack = stack ?: get_stack_pointer(task, regs); |
| - stack; |
| - stack = stack_info.next_sp) { |
| + for (; stack; stack = stack_info.next_sp) { |
| const char *stack_name; |
| |
| stack = PTR_ALIGN(stack, sizeof(long)); |
| -- |
| 2.39.5 |
| |