[PATCH v2] ARM: entry: omit FP emulation for UND exceptions taken in kernel mode

Nick Desaulniers ndesaulniers at google.com
Wed Nov 18 11:42:05 EST 2020


On Wed, Nov 18, 2020 at 5:09 AM Ard Biesheuvel <ardb at kernel.org> wrote:
>
> There are a couple of problems with the exception entry code that deals
> with FP exceptions (which are reported as UND exceptions) when building
> the kernel in Thumb2 mode:
> - the conditional branch to vfp_kmode_exception in vfp_support_entry()
>   may be out of range for its target, depending on how the linker decides
>   to arrange the sections;
> - when the UND exception is taken in kernel mode, the emulation handling
>   logic is entered via the 'call_fpe' label, which means we end up using
>   the wrong value/mask pairs to match and detect the NEON opcodes.
>
> Since UND exceptions in kernel mode are unlikely to occur on a hot path
> (as opposed to the user mode version which is invoked for VFP support
> code and lazy restore), we can use the existing undef hook machinery for

Right, I'd expect these maybe from userspace, but within the kernel?

> any kernel mode instruction emulation that is needed, including calling
> the existing vfp_kmode_exception() routine for unexpected cases. So drop
> the call to call_fpe, and instead, install an undef hook that will get
> called for NEON and VFP instructions that trigger an UND exception in
> kernel mode.
>
> While at it, make sure that the PC correction is accurate for the
> execution mode where the exception was taken, by checking the PSR
> Thumb bit.
>
> Cc: Dmitry Osipenko <digetx at gmail.com>
> Cc: Kees Cook <keescook at chromium.org>
> Cc: Nick Desaulniers <ndesaulniers at google.com>
> Fixes: eff8728fe698 ("vmlinux.lds.h: Add PGO and AutoFDO input sections")
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> Reviewed-by: Linus Walleij <linus.walleij at linaro.org>
> ---
> NOTE: this supersedes 9018/2 "vfp: force non-conditional encoding for external
>       Thumb2" which is currently queued in the patch system - the out-of-range
>       branch to vfp_kmode_exception() is dropped entirely in this patch
>
> v2: - use the PSR T bit to select the right PC correction
>     - add Linus's ack
>
>  arch/arm/kernel/entry-armv.S | 25 +---------
>  arch/arm/vfp/vfphw.S         |  5 --
>  arch/arm/vfp/vfpmodule.c     | 49 +++++++++++++++++++-
>  3 files changed, 49 insertions(+), 30 deletions(-)
>
> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index c4220f51fcf3..0ea8529a4872 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -252,31 +252,10 @@ __und_svc:
>  #else
>         svc_entry
>  #endif
> -       @
> -       @ call emulation code, which returns using r9 if it has emulated
> -       @ the instruction, or the more conventional lr if we are to treat
> -       @ this as a real undefined instruction
> -       @
> -       @  r0 - instruction
> -       @
> -#ifndef CONFIG_THUMB2_KERNEL
> -       ldr     r0, [r4, #-4]
> -#else
> -       mov     r1, #2
> -       ldrh    r0, [r4, #-2]                   @ Thumb instruction at LR - 2
> -       cmp     r0, #0xe800                     @ 32-bit instruction if xx >= 0
> -       blo     __und_svc_fault
> -       ldrh    r9, [r4]                        @ bottom 16 bits
> -       add     r4, r4, #2
> -       str     r4, [sp, #S_PC]
> -       orr     r0, r9, r0, lsl #16
> -#endif
> -       badr    r9, __und_svc_finish
> -       mov     r2, r4
> -       bl      call_fpe
>
>         mov     r1, #4                          @ PC correction to apply
> -__und_svc_fault:
> + THUMB(        tst     r5, #PSR_T_BIT          )       @ exception taken in Thumb mode?

Question: what's in r5 at this point?

> + THUMB(        movne   r1, #2                  )       @ if so, fix up PC correction
>         mov     r0, sp                          @ struct pt_regs *regs
>         bl      __und_fault
>
> diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
> index 4fcff9f59947..d5837bf05a9a 100644
> --- a/arch/arm/vfp/vfphw.S
> +++ b/arch/arm/vfp/vfphw.S
> @@ -79,11 +79,6 @@ ENTRY(vfp_support_entry)
>         DBGSTR3 "instr %08x pc %08x state %p", r0, r2, r10
>
>         .fpu    vfpv2
> -       ldr     r3, [sp, #S_PSR]        @ Neither lazy restore nor FP exceptions
> -       and     r3, r3, #MODE_MASK      @ are supported in kernel mode
> -       teq     r3, #USR_MODE
> -       bne     vfp_kmode_exception     @ Returns through lr
> -
>         VFPFMRX r1, FPEXC               @ Is the VFP enabled?
>         DBGSTR1 "fpexc %08x", r1
>         tst     r1, #FPEXC_EN
> diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
> index 8c9e7f9f0277..c3b6451c18bd 100644
> --- a/arch/arm/vfp/vfpmodule.c
> +++ b/arch/arm/vfp/vfpmodule.c
> @@ -23,6 +23,7 @@
>  #include <asm/cputype.h>
>  #include <asm/system_info.h>
>  #include <asm/thread_notify.h>
> +#include <asm/traps.h>
>  #include <asm/vfp.h>
>
>  #include "vfpinstr.h"
> @@ -642,7 +643,9 @@ static int vfp_starting_cpu(unsigned int unused)
>         return 0;
>  }
>
> -void vfp_kmode_exception(void)
> +#ifdef CONFIG_KERNEL_MODE_NEON
> +
> +static int vfp_kmode_exception(struct pt_regs *regs, unsigned int instr)
>  {
>         /*
>          * If we reach this point, a floating point exception has been raised
> @@ -660,9 +663,51 @@ void vfp_kmode_exception(void)
>                 pr_crit("BUG: unsupported FP instruction in kernel mode\n");
>         else
>                 pr_crit("BUG: FP instruction issued in kernel mode with FP unit disabled\n");
> +       pr_crit("FPEXC == 0x%08x\n", fmrx(FPEXC));
> +       return 1;
>  }
>
> -#ifdef CONFIG_KERNEL_MODE_NEON
> +static struct undef_hook vfp_kmode_exception_hook[] = {{
> +       .instr_mask     = 0xfe000000,
> +       .instr_val      = 0xf2000000,
> +       .cpsr_mask      = MODE_MASK | PSR_T_BIT,
> +       .cpsr_val       = SVC_MODE,
> +       .fn             = vfp_kmode_exception,
> +}, {
> +       .instr_mask     = 0xff100000,
> +       .instr_val      = 0xf4000000,
> +       .cpsr_mask      = MODE_MASK | PSR_T_BIT,
> +       .cpsr_val       = SVC_MODE,
> +       .fn             = vfp_kmode_exception,
> +}, {
> +       .instr_mask     = 0xef000000,
> +       .instr_val      = 0xef000000,
> +       .cpsr_mask      = MODE_MASK | PSR_T_BIT,
> +       .cpsr_val       = SVC_MODE | PSR_T_BIT,
> +       .fn             = vfp_kmode_exception,
> +}, {
> +       .instr_mask     = 0xff100000,
> +       .instr_val      = 0xf9000000,
> +       .cpsr_mask      = MODE_MASK | PSR_T_BIT,
> +       .cpsr_val       = SVC_MODE | PSR_T_BIT,
> +       .fn             = vfp_kmode_exception,
> +}, {
> +       .instr_mask     = 0x0c000e00,
> +       .instr_val      = 0x0c000a00,
> +       .cpsr_mask      = MODE_MASK,
> +       .cpsr_val       = SVC_MODE,
> +       .fn             = vfp_kmode_exception,
> +}};

I don't plan on verifying these instruction masks, but I wanted to
check that the first two should not be bitwise OR'ing PSR_T_BIT for
the .cpsr_val like the next two structs do?  Patch looks reasonable to
me otherwise, just some naive questions in case these differences were
unintentional.  Would comments be helpful for each mask for what kind
of opcode they're handling?

> +
> +static int __init vfp_kmode_exception_hook_init(void)
> +{
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(vfp_kmode_exception_hook); i++)
> +               register_undef_hook(&vfp_kmode_exception_hook[i]);
> +       return 0;
> +}
> +core_initcall(vfp_kmode_exception_hook_init);
>
>  /*
>   * Kernel-side NEON support functions
> --
> 2.17.1
>


-- 
Thanks,
~Nick Desaulniers



More information about the linux-arm-kernel mailing list