[PATCH] ARM: unwind: set frame.pc correctly for current-thread unwinding

[Ard Biesheuvel]

On Wed, 9 Mar 2022 at 13:10, Russell King (Oracle)
<rmk+kernel at armlinux.org.uk> wrote:
>
> 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.
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel at armlinux.org.uk>

Reviewed-by: Ard Biesheuvel <ardb at kernel.org>

We'll need something similar for __save_stack_trace(), and for
return_address() if we ever decide to re-enable it for
CONFIG_ARM_UNWIND=y




> ---
>  arch/arm/kernel/unwind.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/kernel/unwind.c b/arch/arm/kernel/unwind.c
> index a007af0f0209..0e2244e26f37 100644
> --- a/arch/arm/kernel/unwind.c
> +++ b/arch/arm/kernel/unwind.c
> @@ -487,7 +487,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.30.2
>