ARM: unwind: set frame.pc correctly for current-thread unwinding
authorRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
Wed, 9 Mar 2022 12:06:02 +0000 (12:06 +0000)
committerRussell King (Oracle) <rmk+kernel@armlinux.org.uk>
Fri, 11 Mar 2022 10:55:28 +0000 (10:55 +0000)
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 <ardb@kernel.org>
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
arch/arm/kernel/return_address.c
arch/arm/kernel/stacktrace.c
arch/arm/kernel/unwind.c

index 00c11579406cf08d473e05d740338eaec9b304c3..8aac1e10b117aa45393a6e2df86130c62b7426fa 100644 (file)
@@ -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;
index 75e905508f2791fa8393b5d9ded2e5999137ce3b..b5efecb3d73079e04718033dd914fe7ed1805690 100644 (file)
@@ -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;
index 04ccff9d9793a94123aa771f4e06ca4b1302f545..3cd8892ed52bf2842e036a781170388d9ed7a589 100644 (file)
@@ -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);