Skip to content

Commit cac5bc4

Browse files
mrutland-armAlex Shi
authored andcommitted
arm64: fix dump_backtrace/unwind_frame with NULL tsk
In some places, dump_backtrace() is called with a NULL tsk parameter, e.g. in bug_handler() in arch/arm64, or indirectly via show_stack() in core code. The expectation is that this is treated as if current were passed instead of NULL. Similar is true of unwind_frame(). Commit a80a0eb70c358f8c ("arm64: make irq_stack_ptr more robust") didn't take this into account. In dump_backtrace() it compares tsk against current *before* we check if tsk is NULL, and in unwind_frame() we never set tsk if it is NULL. Due to this, we won't initialise irq_stack_ptr in either function. In dump_backtrace() this results in calling dump_mem() for memory immediately above the IRQ stack range, rather than for the relevant range on the task stack. In unwind_frame we'll reject unwinding frames on the IRQ stack. In either case this results in incomplete or misleading backtrace information, but is not otherwise problematic. The initial percpu areas (including the IRQ stacks) are allocated in the linear map, and dump_mem uses __get_user(), so we shouldn't access anything with side-effects, and will handle holes safely. This patch fixes the issue by having both functions handle the NULL tsk case before doing anything else with tsk. Signed-off-by: Mark Rutland <mark.rutland@arm.com> Fixes: a80a0eb70c358f8c ("arm64: make irq_stack_ptr more robust") Acked-by: James Morse <james.morse@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Yang Shi <yang.shi@linaro.org> Signed-off-by: Will Deacon <will.deacon@arm.com> (cherry picked from commit b5e7307d9d5a340d2c9fabbe1cee137d4c682c71) Signed-off-by: Alex Shi <alex.shi@linaro.org>
1 parent 21a48ff commit cac5bc4

2 files changed

Lines changed: 9 additions & 6 deletions

File tree

arch/arm64/kernel/stacktrace.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
4343
unsigned long fp = frame->fp;
4444
unsigned long irq_stack_ptr;
4545

46+
if (!tsk)
47+
tsk = current;
48+
4649
/*
4750
* Switching between stacks is valid when tracing current and in
4851
* non-preemptible context.
@@ -67,7 +70,7 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
6770
frame->pc = *(unsigned long *)(fp + 8);
6871

6972
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
70-
if (tsk && tsk->ret_stack &&
73+
if (tsk->ret_stack &&
7174
(frame->pc == (unsigned long)return_to_handler)) {
7275
/*
7376
* This is a case where function graph tracer has

arch/arm64/kernel/traps.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,11 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
149149
unsigned long irq_stack_ptr;
150150
int skip;
151151

152+
pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);
153+
154+
if (!tsk)
155+
tsk = current;
156+
152157
/*
153158
* Switching between stacks is valid when tracing current and in
154159
* non-preemptible context.
@@ -158,11 +163,6 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
158163
else
159164
irq_stack_ptr = 0;
160165

161-
pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);
162-
163-
if (!tsk)
164-
tsk = current;
165-
166166
if (tsk == current) {
167167
frame.fp = (unsigned long)__builtin_frame_address(0);
168168
frame.sp = current_stack_pointer;

0 commit comments

Comments
 (0)