[PATCH v3 07/13] ARM: unwind: track location of LR value in stack frame

[Nick Desaulniers]

On Thu, Feb 3, 2022 at 12:22 AM Ard Biesheuvel <ardb at kernel.org> wrote:
>
> The ftrace graph tracer needs to override the return address of an
> instrumented function, in order to install a hook that gets invoked when
> the function returns again.
>
> Currently, we only support this when building for ARM using GCC with
> frame pointers, as in this case, it is guaranteed that the function will
> reload LR from [FP, #-4] in all cases, and we can simply pass that
> address to the ftrace code.
>
> In order to support this for configurations that rely on the EABI
> unwinder, such as Thumb2 builds, make the unwinder keep track of the
> address from which LR was unwound, permitting ftrace to make use of this
> in a subsequent patch.
>
> Drop the call to is_kernel_text_address(), which is problematic in terms
> of ftrace recursion, given that it may be instrumented itself. The call
> is redundant anyway, as no unwind directives will be found unless the PC
> points to memory that is known to contain executable code.
>
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>


Looks like patch 8/13 consumes lr_addr.

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

> ---
>  arch/arm/include/asm/stacktrace.h | 3 +++
>  arch/arm/kernel/Makefile          | 1 +
>  arch/arm/kernel/unwind.c          | 7 ++++---
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/stacktrace.h b/arch/arm/include/asm/stacktrace.h
> index d87d60532b86..e56503fd9447 100644
> --- a/arch/arm/include/asm/stacktrace.h
> +++ b/arch/arm/include/asm/stacktrace.h
> @@ -14,6 +14,9 @@ struct stackframe {
>         unsigned long sp;
>         unsigned long lr;
>         unsigned long pc;
> +
> +       /* address of the LR value on the stack */
> +       unsigned long *lr_addr;
>  #ifdef CONFIG_KRETPROBES
>         struct llist_node *kr_cur;
>         struct task_struct *tsk;
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index ae295a3bcfef..56511856ff9d 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -10,6 +10,7 @@ ifdef CONFIG_FUNCTION_TRACER
>  CFLAGS_REMOVE_ftrace.o = -pg
>  CFLAGS_REMOVE_insn.o = -pg
>  CFLAGS_REMOVE_patch.o = -pg
> +CFLAGS_REMOVE_unwind.o = -pg
>  endif
>
>  CFLAGS_REMOVE_return_address.o = -pg
> diff --git a/arch/arm/kernel/unwind.c b/arch/arm/kernel/unwind.c
> index c5ea328c428d..b4e468a7674b 100644
> --- a/arch/arm/kernel/unwind.c
> +++ b/arch/arm/kernel/unwind.c
> @@ -55,6 +55,7 @@ struct unwind_ctrl_block {
>         const unsigned long *insn;      /* pointer to the current instructions word */
>         unsigned long sp_low;           /* lowest value of sp allowed */
>         unsigned long sp_high;          /* highest value of sp allowed */
> +       unsigned long *lr_addr;         /* address of LR value on the stack */
>         /*
>          * 1 : check for stack overflow for each register pop.
>          * 0 : save overhead if there is plenty of stack remaining.
> @@ -239,6 +240,8 @@ static int unwind_pop_register(struct unwind_ctrl_block *ctrl,
>          * from being tracked by KASAN.
>          */
>         ctrl->vrs[reg] = READ_ONCE_NOCHECK(*(*vsp));
> +       if (reg == 14)
> +               ctrl->lr_addr = *vsp;
>         (*vsp)++;
>         return URC_OK;
>  }
> @@ -395,9 +398,6 @@ int unwind_frame(struct stackframe *frame)
>         pr_debug("%s(pc = %08lx lr = %08lx sp = %08lx)\n", __func__,
>                  frame->pc, frame->lr, frame->sp);
>
> -       if (!kernel_text_address(frame->pc))
> -               return -URC_FAILURE;
> -
>         idx = unwind_find_idx(frame->pc);
>         if (!idx) {
>                 pr_warn("unwind: Index not found %08lx\n", frame->pc);
> @@ -476,6 +476,7 @@ int unwind_frame(struct stackframe *frame)
>         frame->lr = ctrl.vrs[LR];
>         frame->pc = ctrl.vrs[PC];
>         frame->sp_low = ctrl.sp_low;
> +       frame->lr_addr = ctrl.lr_addr;
>
>         return URC_OK;
>  }
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers