Re: [PATCH mm v11 27/42] arm64: mte: Add in-kernel tag fault handler

From: Catalin Marinas
Date: Thu Dec 03 2020 - 05:27:41 EST


On Mon, Nov 23, 2020 at 09:07:51PM +0100, Andrey Konovalov wrote:
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 385a189f7d39..d841a560fae7 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -200,13 +200,36 @@ do { \
> CONFIG_ARM64_PAN)); \
> } while (0)
>
> +/*
> + * The Tag Check Flag (TCF) mode for MTE is per EL, hence TCF0
> + * affects EL0 and TCF affects EL1 irrespective of which TTBR is
> + * used.
> + * The kernel accesses TTBR0 usually with LDTR/STTR instructions
> + * when UAO is available, so these would act as EL0 accesses using
> + * TCF0.
> + * However futex.h code uses exclusives which would be executed as
> + * EL1, this can potentially cause a tag check fault even if the
> + * user disables TCF0.
> + *
> + * To address the problem we set the PSTATE.TCO bit in uaccess_enable()
> + * and reset it in uaccess_disable().
> + *
> + * The Tag check override (TCO) bit disables temporarily the tag checking
> + * preventing the issue.
> + */
> static inline void uaccess_disable(void)
> {
> + asm volatile(ALTERNATIVE("nop", SET_PSTATE_TCO(0),
> + ARM64_MTE, CONFIG_KASAN_HW_TAGS));
> +
> __uaccess_disable(ARM64_HAS_PAN);
> }
>
> static inline void uaccess_enable(void)
> {
> + asm volatile(ALTERNATIVE("nop", SET_PSTATE_TCO(1),
> + ARM64_MTE, CONFIG_KASAN_HW_TAGS));
> +
> __uaccess_enable(ARM64_HAS_PAN);
> }

I think that's insufficient if CONFIG_ARM64_PAN is disabled. In the !PAN
case, the get/put_user() accessors use standard LDR/STR instructions
which would follow the TCF rather than TCF0 mode checking. However, they
don't use the above uaccess_disable/enable() functions.

The current user space support is affected as well but luckily we just
skip tag checking on the uaccess routines if !PAN since the kernel TCF
is 0. With the in-kernel MTE, TCF may be more strict than TCF0.

My suggestion is to simply make CONFIG_ARM64_MTE depend on (or select)
PAN. Architecturally this should work since PAN is required for ARMv8.1,
so present with any MTE implementation. This patch is on top of -next,
though it has a Fixes tag in 5.10:

--------------------------8<---------------------------