From c46c2c9b43f4f08f20dc06417fbf7091e4ca6d34 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Wed, 9 Mar 2022 12:06:02 +0000 Subject: [PATCH] ARM: unwind: set frame.pc correctly for current-thread unwinding When e.g. a WARN_ON() is encountered, we attempt to unwind the current thread. To do this, we set frame.pc to unwind_backtrace, which means it points at the beginning of the function. However, the rest of the state is initialised from within the function, which means the function prologue has already been run. This can be confusing, and with a recent patch from Ard, can result in the unwinder misbehaving if we want to be strict about the PC value. If we correctly initialise the state so it is self-consistent (in other words, set frame.pc to the location we are initialising it) then we eliminate this confusion, and avoid possible future issues. Reviewed-by: Ard Biesheuvel Signed-off-by: Russell King (Oracle) --- arch/arm/kernel/return_address.c | 3 ++- arch/arm/kernel/stacktrace.c | 3 ++- arch/arm/kernel/unwind.c | 7 ++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/arch/arm/kernel/return_address.c b/arch/arm/kernel/return_address.c index 00c1157..8aac1e1 100644 --- a/arch/arm/kernel/return_address.c +++ b/arch/arm/kernel/return_address.c @@ -41,7 +41,8 @@ void *return_address(unsigned int level) frame.fp = (unsigned long)__builtin_frame_address(0); frame.sp = current_stack_pointer; frame.lr = (unsigned long)__builtin_return_address(0); - frame.pc = (unsigned long)return_address; +here: + frame.pc = (unsigned long)&&here; #ifdef CONFIG_KRETPROBES frame.kr_cur = NULL; frame.tsk = current; diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c index 75e9055..b5efecb 100644 --- a/arch/arm/kernel/stacktrace.c +++ b/arch/arm/kernel/stacktrace.c @@ -160,7 +160,8 @@ static noinline void __save_stack_trace(struct task_struct *tsk, frame.fp = (unsigned long)__builtin_frame_address(0); frame.sp = current_stack_pointer; frame.lr = (unsigned long)__builtin_return_address(0); - frame.pc = (unsigned long)__save_stack_trace; +here: + frame.pc = (unsigned long)&&here; } #ifdef CONFIG_KRETPROBES frame.kr_cur = NULL; diff --git a/arch/arm/kernel/unwind.c b/arch/arm/kernel/unwind.c index 04ccff9..3cd8892 100644 --- a/arch/arm/kernel/unwind.c +++ b/arch/arm/kernel/unwind.c @@ -501,7 +501,12 @@ void unwind_backtrace(struct pt_regs *regs, struct task_struct *tsk, frame.fp = (unsigned long)__builtin_frame_address(0); frame.sp = current_stack_pointer; frame.lr = (unsigned long)__builtin_return_address(0); - frame.pc = (unsigned long)unwind_backtrace; + /* We are saving the stack and execution state at this + * point, so we should ensure that frame.pc is within + * this block of code. + */ +here: + frame.pc = (unsigned long)&&here; } else { /* task blocked in __switch_to */ frame.fp = thread_saved_fp(tsk); -- 2.7.4