[PATCH v3 07/10] ARM: backtrace-clang: avoid crash on bogus frame pointer

[Nick Desaulniers]

On Sun, Oct 17, 2021 at 6:17 AM Ard Biesheuvel <ardb at kernel.org> wrote:
>
> The Clang backtrace code dereferences the link register value pulled
> from the stack to decide whether the caller was a branch-and-link
> instruction, in order to subsequently decode the offset to find the
> start of the calling function. Unlike other loads in this routine, this
> one is not protected by a fixup, and may therefore cause a crash if the
> address in question is bogus.
>
> So let's fix this, by treating the fault as a failure to decode the 'bl'
> instruction. To avoid a label renum, reuse a fixup label that guards an
> instruction that cannot fault to begin with.

Thanks for fixing this.  My understanding of fixups was non-existent
when I initially reviewed this code. (Taking the time to write
http://nickdesaulniers.github.io/blog/2020/04/06/off-by-two/ helped a
lot).

Reviewed-by: Nick Desaulniers <ndesaulniers at google.com>

>
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> ---
>  arch/arm/lib/backtrace-clang.S | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/lib/backtrace-clang.S b/arch/arm/lib/backtrace-clang.S
> index 5b2cdb1003e3..5b4bca85d06d 100644
> --- a/arch/arm/lib/backtrace-clang.S
> +++ b/arch/arm/lib/backtrace-clang.S
> @@ -144,7 +144,7 @@ for_each_frame:     tst     frame, mask             @ Check for address exceptions
>   */
>  1003:          ldr     sv_lr, [sv_fp, #4]      @ get saved lr from next frame
>
> -               ldr     r0, [sv_lr, #-4]        @ get call instruction
> +1004:          ldr     r0, [sv_lr, #-4]        @ get call instruction
>                 ldr     r3, .Lopcode+4
>                 and     r2, r3, r0              @ is this a bl call
>                 teq     r2, r3
> @@ -164,7 +164,7 @@ finished_setup:
>  /*
>   * Print the function (sv_pc) and where it was called from (sv_lr).
>   */
> -1004:          mov     r0, sv_pc
> +               mov     r0, sv_pc
>
>                 mov     r1, sv_lr
>                 mov     r2, frame
> @@ -210,7 +210,7 @@ ENDPROC(c_backtrace)
>                 .long   1001b, 1006b
>                 .long   1002b, 1006b
>                 .long   1003b, 1006b
> -               .long   1004b, 1006b
> +               .long   1004b, finished_setup
>                 .long   1005b, 1006b
>                 .popsection
>
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers