[PATCH 3/8] ARM: ftrace: use trampolines to keep .init.text in branching range

[Nick Desaulniers]

On Tue, Jan 25, 2022 at 7:37 AM Ard Biesheuvel <ardb at kernel.org> wrote:
>
> Kernel images that are large in comparison to the range of a direct
> branch may fail to work as expected with ftrace, as patching a direct
> branch to one of the core ftrace routines may not be possible from the
> .init.text section, if it is emitted too far away from the normal .text
> section.
>
> This is more likely to affect Thumb2 builds, given that its range is
> only -/+ 16 MiB (as opposed to ARM which has -/+ 32 MiB), but may occur
> in either ISA.
>
> To work around this, add a couple of trampolines to .init.text and
> swap these in when the ftrace patching code is operating on callers in
> .init.text.
>
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> ---
>  arch/arm/kernel/entry-ftrace.S | 16 ++++++++++++++++
>  arch/arm/kernel/ftrace.c       | 19 +++++++++++++++++--
>  2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
> index dca12a09322a..237d435e29aa 100644
> --- a/arch/arm/kernel/entry-ftrace.S
> +++ b/arch/arm/kernel/entry-ftrace.S
> @@ -270,3 +270,19 @@ ENTRY(ftrace_stub)
>  .Lftrace_stub:
>         ret     lr
>  ENDPROC(ftrace_stub)
> +
> +#ifdef CONFIG_DYNAMIC_FTRACE
> +
> +       __INIT
> +
> +       .macro  init_tramp, dst:req
> +ENTRY(\dst\()_from_init)
> +       ldr     pc, =\dst
> +ENDPROC(\dst\()_from_init)
> +       .endm
> +
> +       init_tramp      ftrace_caller
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +       init_tramp      ftrace_regs_caller
> +#endif

Consider adding __FINIT here; while it's not necessary now, folks who
append new code to this file might be surprised to find new code in
.init.text.  I would also tighten up the __INIT/__FINIT to only
surround the invocations of init_tramp and not the macro definition
itself.

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

> +#endif
> diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
> index db72d3a6522d..d2326794fd09 100644
> --- a/arch/arm/kernel/ftrace.c
> +++ b/arch/arm/kernel/ftrace.c
> @@ -51,9 +51,18 @@ static unsigned long ftrace_nop_replace(struct dyn_ftrace *rec)
>         return NOP;
>  }
>
> +void ftrace_caller_from_init(void);
> +void ftrace_regs_caller_from_init(void);
> +
>  static unsigned long adjust_address(struct dyn_ftrace *rec, unsigned long addr)
>  {
> -       return addr;
> +       if (!IS_ENABLED(CONFIG_DYNAMIC_FTRACE) ||
> +           likely(!is_kernel_inittext(rec->ip)))
> +               return addr;
> +       if (!IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS) ||
> +           addr == (unsigned long)&ftrace_caller)
> +               return (unsigned long)&ftrace_caller_from_init;
> +       return (unsigned long)&ftrace_regs_caller_from_init;
>  }
>
>  int ftrace_arch_code_modify_prepare(void)
> @@ -189,7 +198,13 @@ int ftrace_make_nop(struct module *mod,
>  #endif
>
>         new = ftrace_nop_replace(rec);
> -       ret = ftrace_modify_code(ip, old, new, true);
> +       /*
> +        * Locations in .init.text may call __gnu_mcount_mc via a linker
> +        * emitted veneer if they are too far away from its implementation, and
> +        * so validation may fail spuriously in such cases. Let's work around
> +        * this by omitting those from validation.
> +        */
> +       ret = ftrace_modify_code(ip, old, new, !is_kernel_inittext(ip));
>
>         return ret;
>  }
> --
> 2.30.2
>


-- 
Thanks,
~Nick Desaulniers