[PATCH 0/3] arm64: MTE cleanups

[PATCH 0/3] arm64: MTE cleanups

[Mark Rutland]

While working on moving the MTE entry code over to C, I spotted a few things
that could be improved more generally.

The first patch is a fix for a potential issue in some kernel configurations,
the second avoids redundant work to generate and load a compile-time constant,
and the third removes some redundant infrastructure for suprressing fault
reporting which made the architectural fault handling code confusing.

The entry bits may conflict with Peter's concurrent rework there; I can rebase
this series as required.

Thanks,
Mark.

Mark Rutland (3):
  arm64: mte: fix restoration of GCR_EL1 from suspend
  arm64: kasan: mte: use a constant kernel GCR_EL1 value
  arm64: kasan: mte: remove redundant mte_report_once logic

 arch/arm64/include/asm/memory.h    |  2 --
 arch/arm64/include/asm/mte-kasan.h | 17 ------------
 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            | 54 --------------------------------------
 arch/arm64/kernel/suspend.c        |  1 -
 arch/arm64/mm/fault.c              | 15 +----------
 arch/arm64/mm/proc.S               |  3 +--
 include/linux/kasan-tags.h         | 15 +++++++++++
 lib/test_kasan.c                   |  2 --
 mm/kasan/hw_tags.c                 |  8 ------
 mm/kasan/kasan.h                   | 22 +---------------
 13 files changed, 36 insertions(+), 130 deletions(-)
 create mode 100644 include/linux/kasan-tags.h

-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] arm64: mte: fix restoration of GCR_EL1 from suspend

[Mark Rutland]

Since commit:

  bad1e1c663e0a72f ("arm64: mte: switch GCR_EL1 in kernel entry and exit")

... we' saved/restored the user GCR_EL1 value at exception boundaries,
and update_gcr_el1_excl() is no longer used for this. However it is used
to restore the kernel's GCR_EL1 value when returning from a suspend
state. Thus, the comment is misleading (and an ISB is necessary).

When restoring the kernel's GCR value, we need an ISB to ensure this is
used by subsequent instructions. We don't necessarily get an ISB by
other means (e.g. if the kernel is built without support for pointer
authentication). As __cpu_setup() initialised GCR_EL1.Exclude to 0xffff,
until a context synchronization event, allocation tag 0 may be used
rather than the desired set of tags.

This patch drops the misleading comment, adds the missing ISB, and for
clarity folds update_gcr_el1_excl() into its only user.

Fixes: bad1e1c663e0a72f ("arm64: mte: switch GCR_EL1 in kernel entry and exit")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andrey Konovalov <andreyknvl@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/mte.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 69b3fde8759e..36f51b0e438a 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -193,18 +193,6 @@ void mte_check_tfsr_el1(void)
 }
 #endif
 
-static void update_gcr_el1_excl(u64 excl)
-{
-
-	/*
-	 * Note that the mask controlled by the user via prctl() is an
-	 * include while GCR_EL1 accepts an exclude mask.
-	 * No need for ISB since this only affects EL0 currently, implicit
-	 * with ERET.
-	 */
-	sysreg_clear_set_s(SYS_GCR_EL1, SYS_GCR_EL1_EXCL_MASK, excl);
-}
-
 static void set_gcr_el1_excl(u64 excl)
 {
 	current->thread.gcr_user_excl = excl;
@@ -265,7 +253,8 @@ void mte_suspend_exit(void)
 	if (!system_supports_mte())
 		return;
 
-	update_gcr_el1_excl(gcr_kernel_excl);
+	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)
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] arm64: kasan: mte: use a constant kernel GCR_EL1 value

[Mark Rutland]

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@arm.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Konovalov <andreyknvl@gmail.com>
Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Peter Collingbourne <pcc@google.com>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Will Deacon <will@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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] arm64: kasan: mte: remove redundant mte_report_once logic

[Mark Rutland]

We have special logic to suppress MTE tag check fault reporting, based
on a global `mte_report_once` and `reported` variables. These can be
used to suppress calling kasan_report() when taking a tag check fault,
but do not prevent taking the fault in the first place, nor does they
affect the way we disable tag checks upon taking a fault.

The core KASAN code already defaults to reporting a single fault, and
has a `multi_shot` control to permit reporting multiple faults. The only
place we transiently alter `mte_report_once` is in lib/test_kasan.c,
where we also the `multi_shot` state as the same time. Thus
`mte_report_once` and `reported` are redundant, and can be removed.

When a tag check fault is taken, tag checking will be disabled by
`do_tag_recovery` and must be explicitly re-enabled if desired. The test
code does this by calling kasan_enable_tagging_sync().

This patch removes the redundant mte_report_once() logic and associated
variables.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Konovalov <andreyknvl@gmail.com>
Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Will Deacon <will@kernel.org>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/include/asm/memory.h    |  1 -
 arch/arm64/include/asm/mte-kasan.h | 12 ------------
 arch/arm64/kernel/mte.c            | 12 ------------
 arch/arm64/mm/fault.c              | 15 +--------------
 lib/test_kasan.c                   |  2 --
 mm/kasan/hw_tags.c                 |  6 ------
 mm/kasan/kasan.h                   |  7 -------
 7 files changed, 1 insertion(+), 54 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 7f4e6a923aa6..f1745a843414 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -243,7 +243,6 @@ static inline const void *__tag_set(const void *addr, u8 tag)
 #ifdef CONFIG_KASAN_HW_TAGS
 #define arch_enable_tagging_sync()		mte_enable_kernel_sync()
 #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_get_random_tag()			mte_get_random_tag()
 #define arch_get_mem_tag(addr)			mte_get_mem_tag(addr)
diff --git a/arch/arm64/include/asm/mte-kasan.h b/arch/arm64/include/asm/mte-kasan.h
index 82fa4ac4ad4e..22420e1f8c03 100644
--- a/arch/arm64/include/asm/mte-kasan.h
+++ b/arch/arm64/include/asm/mte-kasan.h
@@ -131,9 +131,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_set_report_once(bool state);
-bool mte_report_once(void);
-
 #else /* CONFIG_ARM64_MTE */
 
 static inline u8 mte_get_ptr_tag(void *ptr)
@@ -164,15 +161,6 @@ static inline void mte_enable_kernel_async(void)
 {
 }
 
-static inline void mte_set_report_once(bool state)
-{
-}
-
-static inline bool mte_report_once(void)
-{
-	return false;
-}
-
 #endif /* CONFIG_ARM64_MTE */
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index c106d2ff8b1d..f6dae3e49d0f 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>
 
-static bool report_fault_once = true;
-
 #ifdef CONFIG_KASAN_HW_TAGS
 /* Whether the MTE asynchronous mode is enabled. */
 DEFINE_STATIC_KEY_FALSE(mte_async_mode);
@@ -138,16 +136,6 @@ void mte_enable_kernel_async(void)
 }
 #endif
 
-void mte_set_report_once(bool state)
-{
-	WRITE_ONCE(report_fault_once, state);
-}
-
-bool mte_report_once(void)
-{
-	return READ_ONCE(report_fault_once);
-}
-
 #ifdef CONFIG_KASAN_HW_TAGS
 void mte_check_tfsr_el1(void)
 {
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 349c488765ca..9ae24e3b72be 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -309,24 +309,11 @@ static void die_kernel_fault(const char *msg, unsigned long addr,
 static void report_tag_fault(unsigned long addr, unsigned int esr,
 			     struct pt_regs *regs)
 {
-	static bool reported;
-	bool is_write;
-
-	if (READ_ONCE(reported))
-		return;
-
-	/*
-	 * This is used for KASAN tests and assumes that no MTE faults
-	 * happened before running the tests.
-	 */
-	if (mte_report_once())
-		WRITE_ONCE(reported, true);
-
 	/*
 	 * SAS bits aren't set for all faults reported in EL1, so we can't
 	 * find out access size.
 	 */
-	is_write = !!(esr & ESR_ELx_WNR);
+	bool is_write = !!(esr & ESR_ELx_WNR);
 	kasan_report(addr, 0, is_write, regs->pc);
 }
 #else
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 8f7b0b2f6e11..8be9d4b3b259 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -53,7 +53,6 @@ static int kasan_test_init(struct kunit *test)
 	}
 
 	multishot = kasan_save_enable_multi_shot();
-	kasan_set_tagging_report_once(false);
 	fail_data.report_found = false;
 	kunit_add_named_resource(test, NULL, NULL, &resource,
 					"kasan_data", &fail_data);
@@ -62,7 +61,6 @@ static int kasan_test_init(struct kunit *test)
 
 static void kasan_test_exit(struct kunit *test)
 {
-	kasan_set_tagging_report_once(true);
 	kasan_restore_multi_shot(multishot);
 	KUNIT_EXPECT_FALSE(test, fail_data.report_found);
 }
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index 2c6c6c6ddfa2..e4c16f6b6680 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -248,12 +248,6 @@ void kasan_free_pages(struct page *page, unsigned int order)
 
 #if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
 
-void kasan_set_tagging_report_once(bool state)
-{
-	hw_set_tagging_report_once(state);
-}
-EXPORT_SYMBOL_GPL(kasan_set_tagging_report_once);
-
 void kasan_enable_tagging_sync(void)
 {
 	hw_enable_tagging_sync();
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 89587f762ab4..a8d3bb19b909 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -289,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_set_tagging_report_once
-#define arch_set_tagging_report_once(state)
-#endif
 #ifndef arch_force_async_tag_fault
 #define arch_force_async_tag_fault()
 #endif
@@ -307,7 +304,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_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()
 #define hw_get_mem_tag(addr)			arch_get_mem_tag(addr)
@@ -318,19 +314,16 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 
 #define hw_enable_tagging_sync()
 #define hw_enable_tagging_async()
-#define hw_set_tagging_report_once(state)
 
 #endif /* CONFIG_KASAN_HW_TAGS */
 
 #if defined(CONFIG_KASAN_HW_TAGS) && IS_ENABLED(CONFIG_KASAN_KUNIT_TEST)
 
-void kasan_set_tagging_report_once(bool state);
 void kasan_enable_tagging_sync(void);
 void kasan_force_async_fault(void);
 
 #else /* CONFIG_KASAN_HW_TAGS || CONFIG_KASAN_KUNIT_TEST */
 
-static inline void kasan_set_tagging_report_once(bool state) { }
 static inline void kasan_enable_tagging_sync(void) { }
 static inline void kasan_force_async_fault(void) { }
 
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] arm64: MTE cleanups

[Will Deacon]

On Wed, 14 Jul 2021 15:38:40 +0100, Mark Rutland wrote:
> While working on moving the MTE entry code over to C, I spotted a few things
> that could be improved more generally.
> 
> The first patch is a fix for a potential issue in some kernel configurations,
> the second avoids redundant work to generate and load a compile-time constant,
> and the third removes some redundant infrastructure for suprressing fault
> reporting which made the architectural fault handling code confusing.
> 
> [...]

Applied patch 1 only to arm64 (for-next/fixes), thanks!

[1/3] arm64: mte: fix restoration of GCR_EL1 from suspend
      https://git.kernel.org/arm64/c/59f44069e052

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] arm64: kasan: mte: use a constant kernel GCR_EL1 value

[Catalin Marinas]

On Wed, Jul 14, 2021 at 03:38:42PM +0100, Mark Rutland 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@arm.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Andrey Konovalov <andreyknvl@gmail.com>
> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Peter Collingbourne <pcc@google.com>
> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Cc: Will Deacon <will@kernel.org>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

> 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 */

If the kasan folk are happy with this change, I can take the patches
through the arm64 tree.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: kasan: mte: remove redundant mte_report_once logic

[Catalin Marinas]

On Wed, Jul 14, 2021 at 03:38:43PM +0100, Mark Rutland wrote:
> We have special logic to suppress MTE tag check fault reporting, based
> on a global `mte_report_once` and `reported` variables. These can be
> used to suppress calling kasan_report() when taking a tag check fault,
> but do not prevent taking the fault in the first place, nor does they
> affect the way we disable tag checks upon taking a fault.
> 
> The core KASAN code already defaults to reporting a single fault, and
> has a `multi_shot` control to permit reporting multiple faults. The only
> place we transiently alter `mte_report_once` is in lib/test_kasan.c,
> where we also the `multi_shot` state as the same time. Thus
> `mte_report_once` and `reported` are redundant, and can be removed.
> 
> When a tag check fault is taken, tag checking will be disabled by
> `do_tag_recovery` and must be explicitly re-enabled if desired. The test
> code does this by calling kasan_enable_tagging_sync().
> 
> This patch removes the redundant mte_report_once() logic and associated
> variables.

The first "reported" variable was added to avoid calling kasan_report on
each CPU as we are lazily disabling tag checking when faults are
triggered. The subsequent mte_report_once() was added to avoid setting
"reported" during the kasan tests and missing the real faults later on.
I wasn't aware that kasan already has its own logic to prevent multiple
reports.

The kasan tests enable multi-shot first, so KASAN_BIT_REPORTED wouldn't
be set by report_enabled(). I think this patch makes sense.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

(and I can queue them through the arm64 tree if I get the acks from the
kasan maintainers)

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: kasan: mte: remove redundant mte_report_once logic

[Andrey Konovalov]

On Tue, Jul 27, 2021 at 8:54 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Wed, Jul 14, 2021 at 03:38:43PM +0100, Mark Rutland wrote:
> > We have special logic to suppress MTE tag check fault reporting, based
> > on a global `mte_report_once` and `reported` variables. These can be
> > used to suppress calling kasan_report() when taking a tag check fault,
> > but do not prevent taking the fault in the first place, nor does they
> > affect the way we disable tag checks upon taking a fault.
> >
> > The core KASAN code already defaults to reporting a single fault, and
> > has a `multi_shot` control to permit reporting multiple faults. The only
> > place we transiently alter `mte_report_once` is in lib/test_kasan.c,
> > where we also the `multi_shot` state as the same time. Thus
> > `mte_report_once` and `reported` are redundant, and can be removed.
> >
> > When a tag check fault is taken, tag checking will be disabled by
> > `do_tag_recovery` and must be explicitly re-enabled if desired. The test
> > code does this by calling kasan_enable_tagging_sync().
> >
> > This patch removes the redundant mte_report_once() logic and associated
> > variables.
>
> The first "reported" variable was added to avoid calling kasan_report on
> each CPU as we are lazily disabling tag checking when faults are
> triggered.

So we do not need "reported" anymore? Why?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: kasan: mte: remove redundant mte_report_once logic

[Catalin Marinas]

On Sat, Jul 31, 2021 at 01:01:25AM +0200, Andrey Konovalov wrote:
> On Tue, Jul 27, 2021 at 8:54 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Wed, Jul 14, 2021 at 03:38:43PM +0100, Mark Rutland wrote:
> > > We have special logic to suppress MTE tag check fault reporting, based
> > > on a global `mte_report_once` and `reported` variables. These can be
> > > used to suppress calling kasan_report() when taking a tag check fault,
> > > but do not prevent taking the fault in the first place, nor does they
> > > affect the way we disable tag checks upon taking a fault.
> > >
> > > The core KASAN code already defaults to reporting a single fault, and
> > > has a `multi_shot` control to permit reporting multiple faults. The only
> > > place we transiently alter `mte_report_once` is in lib/test_kasan.c,
> > > where we also the `multi_shot` state as the same time. Thus
> > > `mte_report_once` and `reported` are redundant, and can be removed.
> > >
> > > When a tag check fault is taken, tag checking will be disabled by
> > > `do_tag_recovery` and must be explicitly re-enabled if desired. The test
> > > code does this by calling kasan_enable_tagging_sync().
> > >
> > > This patch removes the redundant mte_report_once() logic and associated
> > > variables.
> >
> > The first "reported" variable was added to avoid calling kasan_report on
> > each CPU as we are lazily disabling tag checking when faults are
> > triggered.
> 
> So we do not need "reported" anymore? Why?

Because kasan has its own mechanism with KASAN_BIT_REPORTED.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: kasan: mte: remove redundant mte_report_once logic

[Andrey Konovalov]

On Sat, Jul 31, 2021 at 12:44 PM Catalin Marinas
<catalin.marinas@arm.com> wrote:
>
> On Sat, Jul 31, 2021 at 01:01:25AM +0200, Andrey Konovalov wrote:
> > On Tue, Jul 27, 2021 at 8:54 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > On Wed, Jul 14, 2021 at 03:38:43PM +0100, Mark Rutland wrote:
> > > > We have special logic to suppress MTE tag check fault reporting, based
> > > > on a global `mte_report_once` and `reported` variables. These can be
> > > > used to suppress calling kasan_report() when taking a tag check fault,
> > > > but do not prevent taking the fault in the first place, nor does they
> > > > affect the way we disable tag checks upon taking a fault.
> > > >
> > > > The core KASAN code already defaults to reporting a single fault, and
> > > > has a `multi_shot` control to permit reporting multiple faults. The only
> > > > place we transiently alter `mte_report_once` is in lib/test_kasan.c,
> > > > where we also the `multi_shot` state as the same time. Thus
> > > > `mte_report_once` and `reported` are redundant, and can be removed.
> > > >
> > > > When a tag check fault is taken, tag checking will be disabled by
> > > > `do_tag_recovery` and must be explicitly re-enabled if desired. The test
> > > > code does this by calling kasan_enable_tagging_sync().
> > > >
> > > > This patch removes the redundant mte_report_once() logic and associated
> > > > variables.
> > >
> > > The first "reported" variable was added to avoid calling kasan_report on
> > > each CPU as we are lazily disabling tag checking when faults are
> > > triggered.
> >
> > So we do not need "reported" anymore? Why?
>
> Because kasan has its own mechanism with KASAN_BIT_REPORTED.

This mechanism only works when multi_shot is disabled. But milti_shot
is always enabled when running the tests. Yet I tested the series, and
I don't see multiple reports for each test, so everything somehow
works. But I don't understand how.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: kasan: mte: remove redundant mte_report_once logic

[Catalin Marinas]

On Sat, Jul 31, 2021 at 01:28:53PM +0200, Andrey Konovalov wrote:
> On Sat, Jul 31, 2021 at 12:44 PM Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> > On Sat, Jul 31, 2021 at 01:01:25AM +0200, Andrey Konovalov wrote:
> > > On Tue, Jul 27, 2021 at 8:54 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > > On Wed, Jul 14, 2021 at 03:38:43PM +0100, Mark Rutland wrote:
> > > > > We have special logic to suppress MTE tag check fault reporting, based
> > > > > on a global `mte_report_once` and `reported` variables. These can be
> > > > > used to suppress calling kasan_report() when taking a tag check fault,
> > > > > but do not prevent taking the fault in the first place, nor does they
> > > > > affect the way we disable tag checks upon taking a fault.
> > > > >
> > > > > The core KASAN code already defaults to reporting a single fault, and
> > > > > has a `multi_shot` control to permit reporting multiple faults. The only
> > > > > place we transiently alter `mte_report_once` is in lib/test_kasan.c,
> > > > > where we also the `multi_shot` state as the same time. Thus
> > > > > `mte_report_once` and `reported` are redundant, and can be removed.
> > > > >
> > > > > When a tag check fault is taken, tag checking will be disabled by
> > > > > `do_tag_recovery` and must be explicitly re-enabled if desired. The test
> > > > > code does this by calling kasan_enable_tagging_sync().
> > > > >
> > > > > This patch removes the redundant mte_report_once() logic and associated
> > > > > variables.
> > > >
> > > > The first "reported" variable was added to avoid calling kasan_report on
> > > > each CPU as we are lazily disabling tag checking when faults are
> > > > triggered.
> > >
> > > So we do not need "reported" anymore? Why?
> >
> > Because kasan has its own mechanism with KASAN_BIT_REPORTED.
> 
> This mechanism only works when multi_shot is disabled. But milti_shot
> is always enabled when running the tests. Yet I tested the series, and
> I don't see multiple reports for each test, so everything somehow
> works. But I don't understand how.

When running tests, "reported" variable update is also disabled, so it
wouldn't make any difference.

When a fault happens in synchronous mode, to be able to continue safely
the tag checking is disabled on the current CPU (and lazily on the other
CPUs). So you won't be able to get another report on that CPU anyway. In
the unlikely event that the test thread migrates to another CPU before
completion, it may trigger another tag check fault on the new CPU and a
report.

We probably don't see multiple reports during a single test because (a)
kasan tests have preemption disabled, (b) they are unlikely to migrate
during the short time they run or (c) the kasan tests only do a single
faulty access.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: kasan: mte: remove redundant mte_report_once logic

[Andrey Konovalov]

On Sat, Jul 31, 2021 at 1:49 PM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> > > Because kasan has its own mechanism with KASAN_BIT_REPORTED.
> >
> > This mechanism only works when multi_shot is disabled. But milti_shot
> > is always enabled when running the tests. Yet I tested the series, and
> > I don't see multiple reports for each test, so everything somehow
> > works. But I don't understand how.
>
> When running tests, "reported" variable update is also disabled, so it
> wouldn't make any difference.
>
> When a fault happens in synchronous mode, to be able to continue safely
> the tag checking is disabled on the current CPU (and lazily on the other
> CPUs). So you won't be able to get another report on that CPU anyway. In
> the unlikely event that the test thread migrates to another CPU before
> completion, it may trigger another tag check fault on the new CPU and a
> report.
>
> We probably don't see multiple reports during a single test because (a)
> kasan tests have preemption disabled, (b) they are unlikely to migrate
> during the short time they run or (c) the kasan tests only do a single
> faulty access.

I've got a feeling there was some other reason for this "reported"
check, but I don't remember what that was. Maybe this was related to a
bug in the earlier version of the MTE patches.

Anyway, the code looks good and the tests work for me as expected, so:

Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Tested-by: Andrey Konovalov <andreyknvl@gmail.com>

Thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] arm64: kasan: mte: use a constant kernel GCR_EL1 value

[Andrey Konovalov]

On Wed, Jul 14, 2021 at 4:38 PM Mark Rutland <mark.rutland@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@arm.com>
> Cc: Alexander Potapenko <glider@google.com>
> Cc: Andrey Konovalov <andreyknvl@gmail.com>
> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Peter Collingbourne <pcc@google.com>
> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Cc: Will Deacon <will@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@gmail.com>
Tested-by: Andrey Konovalov <andreyknvl@gmail.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: (subset) [PATCH 0/3] arm64: MTE cleanups

[Catalin Marinas]

On Wed, 14 Jul 2021 15:38:40 +0100, Mark Rutland wrote:
> While working on moving the MTE entry code over to C, I spotted a few things
> that could be improved more generally.
> 
> The first patch is a fix for a potential issue in some kernel configurations,
> the second avoids redundant work to generate and load a compile-time constant,
> and the third removes some redundant infrastructure for suprressing fault
> reporting which made the architectural fault handling code confusing.
> 
> [...]

Applied to arm64 (for-next/mte) and tweaked a bit because of the
conflicts. Thanks!

[2/3] arm64: kasan: mte: use a constant kernel GCR_EL1 value
      https://git.kernel.org/arm64/c/82868247897b
[3/3] arm64: kasan: mte: remove redundant mte_report_once logic
      https://git.kernel.org/arm64/c/767215030150

-- 
Catalin


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel