[PATCH 2/3] arm64: kasan: mte: use a constant kernel GCR_EL1 value
Andrey Konovalov
andreyknvl at gmail.com
Mon Aug 2 04:57:58 PDT 2021
On Wed, Jul 14, 2021 at 4:38 PM Mark Rutland <mark.rutland at arm.com> wrote:
>
> When KASAN_HW_TAGS is selected, KASAN is enabled at boot time, and the
> hardware supports MTE, we'll initialize `kernel_gcr_excl` with a value
> dependent on KASAN_TAG_MAX. While the resulting value is a constant
> which depends on KASAN_TAG_MAX, we have to perform some runtime work to
> generate the value, and have to read the value from memory during the
> exception entry path. It would be better if we could generate this as a
> constant at compile-time, and use it as such directly.
>
> Early in boot within __cpu_setup(), we initialize GCR_EL1 to a safe
> value, and later override this with the value required by KASAN. If
> CONFIG_KASAN_HW_TAGS is not selected, or if KASAN is disabeld at boot
> time, the kernel will not use IRG instructions, and so the initial value
> of GCR_EL1 is does not matter to the kernel. Thus, we can instead have
> __cpu_setup() initialize GCR_EL1 to a value consistent with
> KASAN_TAG_MAX, and avoid the need to re-initialize it during hotplug and
> resume form suspend.
>
> This patch makes arem64 use a compile-time constant KERNEL_GCR_EL1
> value, which is compatible with KASAN_HW_TAGS when this is selected.
> This removes the need to re-initialize GCR_EL1 dynamically, and acts as
> an optimization to the entry assembly, which no longer needs to load
> this value from memory. The redundant initialization hooks are removed.
>
> In order to do this, KASAN_TAG_MAX needs to be visible outside of the
> core KASAN code. To do this, I've moved the KASAN_TAG_* values into
> <linux/kasan-tags.h>.
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <mark.rutland at arm.com>
> Cc: Alexander Potapenko <glider at google.com>
> Cc: Andrey Konovalov <andreyknvl at gmail.com>
> Cc: Andrey Ryabinin <ryabinin.a.a at gmail.com>
> Cc: Catalin Marinas <catalin.marinas at arm.com>
> Cc: Dmitry Vyukov <dvyukov at google.com>
> Cc: Peter Collingbourne <pcc at google.com>
> Cc: Vincenzo Frascino <vincenzo.frascino at arm.com>
> Cc: Will Deacon <will at kernel.org>
> ---
> arch/arm64/include/asm/memory.h | 1 -
> arch/arm64/include/asm/mte-kasan.h | 5 -----
> arch/arm64/include/asm/mte.h | 6 ------
> arch/arm64/include/asm/sysreg.h | 16 ++++++++++++++++
> arch/arm64/kernel/entry.S | 5 ++---
> arch/arm64/kernel/mte.c | 31 -------------------------------
> arch/arm64/kernel/suspend.c | 1 -
> arch/arm64/mm/proc.S | 3 +--
> include/linux/kasan-tags.h | 15 +++++++++++++++
> mm/kasan/hw_tags.c | 2 --
> mm/kasan/kasan.h | 15 +--------------
> 11 files changed, 35 insertions(+), 65 deletions(-)
> create mode 100644 include/linux/kasan-tags.h
>
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 824a3655dd93..7f4e6a923aa6 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -245,7 +245,6 @@ static inline const void *__tag_set(const void *addr, u8 tag)
> #define arch_enable_tagging_async() mte_enable_kernel_async()
> #define arch_set_tagging_report_once(state) mte_set_report_once(state)
> #define arch_force_async_tag_fault() mte_check_tfsr_exit()
> -#define arch_init_tags(max_tag) mte_init_tags(max_tag)
> #define arch_get_random_tag() mte_get_random_tag()
> #define arch_get_mem_tag(addr) mte_get_mem_tag(addr)
> #define arch_set_mem_tag_range(addr, size, tag, init) \
> diff --git a/arch/arm64/include/asm/mte-kasan.h b/arch/arm64/include/asm/mte-kasan.h
> index d952352bd008..82fa4ac4ad4e 100644
> --- a/arch/arm64/include/asm/mte-kasan.h
> +++ b/arch/arm64/include/asm/mte-kasan.h
> @@ -130,7 +130,6 @@ static inline void mte_set_mem_tag_range(void *addr, size_t size, u8 tag,
>
> void mte_enable_kernel_sync(void);
> void mte_enable_kernel_async(void);
> -void mte_init_tags(u64 max_tag);
>
> void mte_set_report_once(bool state);
> bool mte_report_once(void);
> @@ -165,10 +164,6 @@ static inline void mte_enable_kernel_async(void)
> {
> }
>
> -static inline void mte_init_tags(u64 max_tag)
> -{
> -}
> -
> static inline void mte_set_report_once(bool state)
> {
> }
> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> index 58c7f80f5596..3f93b9e0b339 100644
> --- a/arch/arm64/include/asm/mte.h
> +++ b/arch/arm64/include/asm/mte.h
> @@ -16,8 +16,6 @@
>
> #include <asm/pgtable-types.h>
>
> -extern u64 gcr_kernel_excl;
> -
> void mte_clear_page_tags(void *addr);
> unsigned long mte_copy_tags_from_user(void *to, const void __user *from,
> unsigned long n);
> @@ -43,7 +41,6 @@ void mte_copy_page_tags(void *kto, const void *kfrom);
> void mte_thread_init_user(void);
> void mte_thread_switch(struct task_struct *next);
> void mte_suspend_enter(void);
> -void mte_suspend_exit(void);
> long set_mte_ctrl(struct task_struct *task, unsigned long arg);
> long get_mte_ctrl(struct task_struct *task);
> int mte_ptrace_copy_tags(struct task_struct *child, long request,
> @@ -72,9 +69,6 @@ static inline void mte_thread_switch(struct task_struct *next)
> static inline void mte_suspend_enter(void)
> {
> }
> -static inline void mte_suspend_exit(void)
> -{
> -}
> static inline long set_mte_ctrl(struct task_struct *task, unsigned long arg)
> {
> return 0;
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 7b9c3acba684..f6687f6f536b 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -11,6 +11,7 @@
>
> #include <linux/bits.h>
> #include <linux/stringify.h>
> +#include <linux/kasan-tags.h>
>
> /*
> * ARMv8 ARM reserves the following encoding for system registers:
> @@ -1067,6 +1068,21 @@
> #define SYS_GCR_EL1_RRND (BIT(16))
> #define SYS_GCR_EL1_EXCL_MASK 0xffffUL
>
> +#ifdef CONFIG_KASAN_HW_TAGS
> +/*
> + * KASAN always uses a whole byte for its tags. With CONFIG_KASAN_HW_TAGS it
> + * only uses tags in the range 0xF0-0xFF, which we map to MTE tags 0x0-0xF.
> + */
> +#define __MTE_TAG_MIN (KASAN_TAG_MIN & 0xf)
> +#define __MTE_TAG_MAX (KASAN_TAG_MAX & 0xf)
> +#define __MTE_TAG_INCL GENMASK(__MTE_TAG_MAX, __MTE_TAG_MIN)
> +#define KERNEL_GCR_EL1_EXCL (SYS_GCR_EL1_EXCL_MASK & ~__MTE_TAG_INCL)
> +#else
> +#define KERNEL_GCR_EL1_EXCL SYS_GCR_EL1_EXCL_MASK
> +#endif
> +
> +#define KERNEL_GCR_EL1 (SYS_GCR_EL1_RRND | KERNEL_GCR_EL1_EXCL)
> +
> /* RGSR_EL1 Definitions */
> #define SYS_RGSR_EL1_TAG_MASK 0xfUL
> #define SYS_RGSR_EL1_SEED_SHIFT 8
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 863d44f73028..247170bb5489 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -177,9 +177,8 @@ alternative_else_nop_endif
> alternative_if_not ARM64_MTE
> b 1f
> alternative_else_nop_endif
> - ldr_l \tmp, gcr_kernel_excl
> -
> - mte_set_gcr \tmp, \tmp2
> + mov \tmp, KERNEL_GCR_EL1
> + msr_s SYS_GCR_EL1, \tmp
> isb
> 1:
> #endif
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index 36f51b0e438a..c106d2ff8b1d 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -22,8 +22,6 @@
> #include <asm/ptrace.h>
> #include <asm/sysreg.h>
>
> -u64 gcr_kernel_excl __ro_after_init;
> -
> static bool report_fault_once = true;
>
> #ifdef CONFIG_KASAN_HW_TAGS
> @@ -101,26 +99,6 @@ int memcmp_pages(struct page *page1, struct page *page2)
> return ret;
> }
>
> -void mte_init_tags(u64 max_tag)
> -{
> - static bool gcr_kernel_excl_initialized;
> -
> - if (!gcr_kernel_excl_initialized) {
> - /*
> - * The format of the tags in KASAN is 0xFF and in MTE is 0xF.
> - * This conversion extracts an MTE tag from a KASAN tag.
> - */
> - u64 incl = GENMASK(FIELD_GET(MTE_TAG_MASK >> MTE_TAG_SHIFT,
> - max_tag), 0);
> -
> - gcr_kernel_excl = ~incl & SYS_GCR_EL1_EXCL_MASK;
> - gcr_kernel_excl_initialized = true;
> - }
> -
> - /* Enable the kernel exclude mask for random tags generation. */
> - write_sysreg_s(SYS_GCR_EL1_RRND | gcr_kernel_excl, SYS_GCR_EL1);
> -}
> -
> static inline void __mte_enable_kernel(const char *mode, unsigned long tcf)
> {
> /* Enable MTE Sync Mode for EL1. */
> @@ -248,15 +226,6 @@ void mte_suspend_enter(void)
> mte_check_tfsr_el1();
> }
>
> -void mte_suspend_exit(void)
> -{
> - if (!system_supports_mte())
> - return;
> -
> - sysreg_clear_set_s(SYS_GCR_EL1, SYS_GCR_EL1_EXCL_MASK, gcr_kernel_excl);
> - isb();
> -}
> -
> long set_mte_ctrl(struct task_struct *task, unsigned long arg)
> {
> u64 sctlr = task->thread.sctlr_user & ~SCTLR_EL1_TCF0_MASK;
> diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> index 938ce6fbee8a..19ee7c33769d 100644
> --- a/arch/arm64/kernel/suspend.c
> +++ b/arch/arm64/kernel/suspend.c
> @@ -76,7 +76,6 @@ void notrace __cpu_suspend_exit(void)
> spectre_v4_enable_mitigation(NULL);
>
> /* Restore additional feature-specific configuration */
> - mte_suspend_exit();
> ptrauth_suspend_exit();
> }
>
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 35936c5ae1ce..d35c90d2e47a 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -437,8 +437,7 @@ SYM_FUNC_START(__cpu_setup)
> mov x10, #MAIR_ATTR_NORMAL_TAGGED
> bfi mair, x10, #(8 * MT_NORMAL_TAGGED), #8
>
> - /* initialize GCR_EL1: all non-zero tags excluded by default */
> - mov x10, #(SYS_GCR_EL1_RRND | SYS_GCR_EL1_EXCL_MASK)
> + mov x10, #KERNEL_GCR_EL1
> msr_s SYS_GCR_EL1, x10
>
> /*
> diff --git a/include/linux/kasan-tags.h b/include/linux/kasan-tags.h
> new file mode 100644
> index 000000000000..4f85f562512c
> --- /dev/null
> +++ b/include/linux/kasan-tags.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_KASAN_TAGS_H
> +#define _LINUX_KASAN_TAGS_H
> +
> +#define KASAN_TAG_KERNEL 0xFF /* native kernel pointers tag */
> +#define KASAN_TAG_INVALID 0xFE /* inaccessible memory tag */
> +#define KASAN_TAG_MAX 0xFD /* maximum value for random tags */
> +
> +#ifdef CONFIG_KASAN_HW_TAGS
> +#define KASAN_TAG_MIN 0xF0 /* minimum value for random tags */
> +#else
> +#define KASAN_TAG_MIN 0x00 /* minimum value for random tags */
> +#endif
> +
> +#endif /* LINUX_KASAN_TAGS_H */
> diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
> index 4ea8c368b5b8..2c6c6c6ddfa2 100644
> --- a/mm/kasan/hw_tags.c
> +++ b/mm/kasan/hw_tags.c
> @@ -142,8 +142,6 @@ void kasan_init_hw_tags_cpu(void)
> if (kasan_arg == KASAN_ARG_OFF)
> return;
>
> - hw_init_tags(KASAN_TAG_MAX);
> -
> /*
> * Enable async mode only when explicitly requested through
> * the command line.
> diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
> index 98e3059bfea4..89587f762ab4 100644
> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -3,6 +3,7 @@
> #define __MM_KASAN_KASAN_H
>
> #include <linux/kasan.h>
> +#include <linux/kasan-tags.h>
> #include <linux/kfence.h>
> #include <linux/stackdepot.h>
>
> @@ -50,16 +51,6 @@ extern bool kasan_flag_async __ro_after_init;
>
> #define KASAN_MEMORY_PER_SHADOW_PAGE (KASAN_GRANULE_SIZE << PAGE_SHIFT)
>
> -#define KASAN_TAG_KERNEL 0xFF /* native kernel pointers tag */
> -#define KASAN_TAG_INVALID 0xFE /* inaccessible memory tag */
> -#define KASAN_TAG_MAX 0xFD /* maximum value for random tags */
> -
> -#ifdef CONFIG_KASAN_HW_TAGS
> -#define KASAN_TAG_MIN 0xF0 /* minimum value for random tags */
> -#else
> -#define KASAN_TAG_MIN 0x00 /* minimum value for random tags */
> -#endif
> -
> #ifdef CONFIG_KASAN_GENERIC
> #define KASAN_FREE_PAGE 0xFF /* page was freed */
> #define KASAN_PAGE_REDZONE 0xFE /* redzone for kmalloc_large allocations */
> @@ -298,9 +289,6 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
> #ifndef arch_enable_tagging_async
> #define arch_enable_tagging_async()
> #endif
> -#ifndef arch_init_tags
> -#define arch_init_tags(max_tag)
> -#endif
> #ifndef arch_set_tagging_report_once
> #define arch_set_tagging_report_once(state)
> #endif
> @@ -319,7 +307,6 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
>
> #define hw_enable_tagging_sync() arch_enable_tagging_sync()
> #define hw_enable_tagging_async() arch_enable_tagging_async()
> -#define hw_init_tags(max_tag) arch_init_tags(max_tag)
> #define hw_set_tagging_report_once(state) arch_set_tagging_report_once(state)
> #define hw_force_async_tag_fault() arch_force_async_tag_fault()
> #define hw_get_random_tag() arch_get_random_tag()
> --
> 2.11.0
>
This kind of breaks the idea of having a standalone MTE API, and
having KASAN use that API to set everything up. But the change does
simplify the logic quite a bit. So if ARM maintainers are happy with
it, I'm fine with it as well.
For KASAN parts:
Reviewed-by: Andrey Konovalov <andreyknvl at gmail.com>
Tested-by: Andrey Konovalov <andreyknvl at gmail.com>
More information about the linux-arm-kernel
mailing list