linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] arm64: ARMv8.5-A: MTE: Add async mode support
@ 2021-01-07 17:29 Vincenzo Frascino
  2021-01-07 17:29 ` [PATCH v2 1/4] kasan, arm64: Add KASAN light mode Vincenzo Frascino
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Vincenzo Frascino @ 2021-01-07 17:29 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov,
	Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

This patchset implements the asynchronous mode support for ARMv8.5-A
Memory Tagging Extension (MTE), which is a debugging feature that allows
to detect with the help of the architecture the C and C++ programmatic
memory errors like buffer overflow, use-after-free, use-after-return, etc.

MTE is built on top of the AArch64 v8.0 virtual address tagging TBI
(Top Byte Ignore) feature and allows a task to set a 4 bit tag on any
subset of its address space that is multiple of a 16 bytes granule. MTE
is based on a lock-key mechanism where the lock is the tag associated to
the physical memory and the key is the tag associated to the virtual
address.
When MTE is enabled and tags are set for ranges of address space of a task,
the PE will compare the tag related to the physical memory with the tag
related to the virtual address (tag check operation). Access to the memory
is granted only if the two tags match. In case of mismatch the PE will raise
an exception.

The exception can be handled synchronously or asynchronously. When the
asynchronous mode is enabled:
  - Upon fault the PE updates the TFSR_EL1 register.
  - The kernel detects the change during one of the following:
    - Context switching
    - Return to user/EL0
    - Kernel entry from EL1
    - Kernel exit to EL1
  - If the register has been updated by the PE the kernel clears it and
    reports the error.

The series contains as well an optimization to mte_assign_mem_tag_range().

The series is based on linux 5.11-rc2.

To simplify the testing a tree with the new patches on top has been made
available at [1].

[1] https://git.gitlab.arm.com/linux-arm/linux-vf.git mte/v10.async

Changes:
--------
v2:
  - Fixed a compilation issue reported by krobot.
  - General cleanup.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Evgenii Stepanov <eugenis@google.com>
Cc: Branislav Rankov <Branislav.Rankov@arm.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

Vincenzo Frascino (4):
  kasan, arm64: Add KASAN light mode
  arm64: mte: Add asynchronous mode support
  arm64: mte: Enable async tag check fault
  arm64: mte: Optimize mte_assign_mem_tag_range()

 arch/arm64/include/asm/memory.h    |  2 +-
 arch/arm64/include/asm/mte-kasan.h |  5 ++-
 arch/arm64/include/asm/mte.h       | 30 ++++++++++++-
 arch/arm64/kernel/entry-common.c   |  6 +++
 arch/arm64/kernel/mte.c            | 70 ++++++++++++++++++++++++++++--
 arch/arm64/lib/mte.S               | 15 -------
 include/linux/kasan.h              |  1 +
 include/linux/kasan_def.h          | 25 +++++++++++
 mm/kasan/hw_tags.c                 | 24 ++--------
 mm/kasan/kasan.h                   |  2 +-
 10 files changed, 137 insertions(+), 43 deletions(-)
 create mode 100644 include/linux/kasan_def.h

-- 
2.30.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/4] kasan, arm64: Add KASAN light mode
  2021-01-07 17:29 [PATCH v2 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
@ 2021-01-07 17:29 ` Vincenzo Frascino
  2021-01-13 17:16   ` Catalin Marinas
  2021-01-07 17:29 ` [PATCH v2 2/4] arm64: mte: Add asynchronous mode support Vincenzo Frascino
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Vincenzo Frascino @ 2021-01-07 17:29 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov,
	Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

Architectures supported by KASAN HW can provide a light mode of
execution. On an MTE enabled arm64 hw for example this can be identified
with the asynch mode of execution. If an async exception occurs, the
arm64 core updates a register which is asynchronously detected the next
time in which the kernel is accessed.

KASAN requires a specific mode of execution to make use of this hw feature.

Add KASAN HW light execution mode.

Note: This patch adds the KASAN_ARG_MODE_LIGHT config option and the
"light" kernel command line option to enable the described feature.
This patch introduces the kasan_def.h header to make easier to propagate
the relevant enumerations to the architectural code.

Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/include/asm/memory.h    |  2 +-
 arch/arm64/include/asm/mte-kasan.h |  5 +++--
 arch/arm64/kernel/mte.c            |  2 +-
 include/linux/kasan.h              |  1 +
 include/linux/kasan_def.h          | 25 +++++++++++++++++++++++++
 mm/kasan/hw_tags.c                 | 24 ++++--------------------
 mm/kasan/kasan.h                   |  2 +-
 7 files changed, 36 insertions(+), 25 deletions(-)
 create mode 100644 include/linux/kasan_def.h

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 18fce223b67b..3a7c5beb7096 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -231,7 +231,7 @@ static inline const void *__tag_set(const void *addr, u8 tag)
 }
 
 #ifdef CONFIG_KASAN_HW_TAGS
-#define arch_enable_tagging()			mte_enable_kernel()
+#define arch_enable_tagging(mode)		mte_enable_kernel(mode)
 #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)
diff --git a/arch/arm64/include/asm/mte-kasan.h b/arch/arm64/include/asm/mte-kasan.h
index 26349a4b5e2e..79e612c31186 100644
--- a/arch/arm64/include/asm/mte-kasan.h
+++ b/arch/arm64/include/asm/mte-kasan.h
@@ -9,6 +9,7 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/kasan_def.h>
 #include <linux/types.h>
 
 /*
@@ -29,7 +30,7 @@ u8 mte_get_mem_tag(void *addr);
 u8 mte_get_random_tag(void);
 void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag);
 
-void mte_enable_kernel(void);
+void mte_enable_kernel(enum kasan_arg_mode mode);
 void mte_init_tags(u64 max_tag);
 
 #else /* CONFIG_ARM64_MTE */
@@ -52,7 +53,7 @@ static inline void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag)
 	return addr;
 }
 
-static inline void mte_enable_kernel(void)
+static inline void mte_enable_kernel(enum kasan_arg_mode mode)
 {
 }
 
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index dc9ada64feed..24a273d47df1 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -151,7 +151,7 @@ void mte_init_tags(u64 max_tag)
 	write_sysreg_s(SYS_GCR_EL1_RRND | gcr_kernel_excl, SYS_GCR_EL1);
 }
 
-void mte_enable_kernel(void)
+void mte_enable_kernel(enum kasan_arg_mode mode)
 {
 	/* Enable MTE Sync Mode for EL1. */
 	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 5e0655fb2a6f..026031444217 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -2,6 +2,7 @@
 #ifndef _LINUX_KASAN_H
 #define _LINUX_KASAN_H
 
+#include <linux/kasan_def.h>
 #include <linux/static_key.h>
 #include <linux/types.h>
 
diff --git a/include/linux/kasan_def.h b/include/linux/kasan_def.h
new file mode 100644
index 000000000000..e774b0122980
--- /dev/null
+++ b/include/linux/kasan_def.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_KASAN_DEF_H
+#define _LINUX_KASAN_DEF_H
+
+enum kasan_arg_mode {
+	KASAN_ARG_MODE_DEFAULT,
+	KASAN_ARG_MODE_OFF,
+	KASAN_ARG_MODE_LIGHT,
+	KASAN_ARG_MODE_PROD,
+	KASAN_ARG_MODE_FULL,
+};
+
+enum kasan_arg_stacktrace {
+	KASAN_ARG_STACKTRACE_DEFAULT,
+	KASAN_ARG_STACKTRACE_OFF,
+	KASAN_ARG_STACKTRACE_ON,
+};
+
+enum kasan_arg_fault {
+	KASAN_ARG_FAULT_DEFAULT,
+	KASAN_ARG_FAULT_REPORT,
+	KASAN_ARG_FAULT_PANIC,
+};
+
+#endif /* _LINUX_KASAN_DEF_H */
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index 55bd6f09c70f..d5c6ad8b2c44 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -19,25 +19,6 @@
 
 #include "kasan.h"
 
-enum kasan_arg_mode {
-	KASAN_ARG_MODE_DEFAULT,
-	KASAN_ARG_MODE_OFF,
-	KASAN_ARG_MODE_PROD,
-	KASAN_ARG_MODE_FULL,
-};
-
-enum kasan_arg_stacktrace {
-	KASAN_ARG_STACKTRACE_DEFAULT,
-	KASAN_ARG_STACKTRACE_OFF,
-	KASAN_ARG_STACKTRACE_ON,
-};
-
-enum kasan_arg_fault {
-	KASAN_ARG_FAULT_DEFAULT,
-	KASAN_ARG_FAULT_REPORT,
-	KASAN_ARG_FAULT_PANIC,
-};
-
 static enum kasan_arg_mode kasan_arg_mode __ro_after_init;
 static enum kasan_arg_stacktrace kasan_arg_stacktrace __ro_after_init;
 static enum kasan_arg_fault kasan_arg_fault __ro_after_init;
@@ -60,6 +41,8 @@ static int __init early_kasan_mode(char *arg)
 
 	if (!strcmp(arg, "off"))
 		kasan_arg_mode = KASAN_ARG_MODE_OFF;
+	else if (!strcmp(arg, "light"))
+		kasan_arg_mode = KASAN_ARG_MODE_LIGHT;
 	else if (!strcmp(arg, "prod"))
 		kasan_arg_mode = KASAN_ARG_MODE_PROD;
 	else if (!strcmp(arg, "full"))
@@ -118,7 +101,7 @@ void kasan_init_hw_tags_cpu(void)
 		return;
 
 	hw_init_tags(KASAN_TAG_MAX);
-	hw_enable_tagging();
+	hw_enable_tagging(kasan_arg_mode);
 }
 
 /* kasan_init_hw_tags() is called once on boot CPU. */
@@ -145,6 +128,7 @@ void __init kasan_init_hw_tags(void)
 	case KASAN_ARG_MODE_OFF:
 		/* If KASAN is disabled, do nothing. */
 		return;
+	case KASAN_ARG_MODE_LIGHT:
 	case KASAN_ARG_MODE_PROD:
 		static_branch_enable(&kasan_flag_enabled);
 		break;
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index cc4d9e1d49b1..78c09279327e 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -284,7 +284,7 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)
 #define arch_set_mem_tag_range(addr, size, tag) ((void *)(addr))
 #endif
 
-#define hw_enable_tagging()			arch_enable_tagging()
+#define hw_enable_tagging(mode)			arch_enable_tagging(mode)
 #define hw_init_tags(max_tag)			arch_init_tags(max_tag)
 #define hw_get_random_tag()			arch_get_random_tag()
 #define hw_get_mem_tag(addr)			arch_get_mem_tag(addr)
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/4] arm64: mte: Add asynchronous mode support
  2021-01-07 17:29 [PATCH v2 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
  2021-01-07 17:29 ` [PATCH v2 1/4] kasan, arm64: Add KASAN light mode Vincenzo Frascino
@ 2021-01-07 17:29 ` Vincenzo Frascino
  2021-01-13 17:22   ` Catalin Marinas
  2021-01-07 17:29 ` [PATCH v2 3/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
  2021-01-07 17:29 ` [PATCH v2 4/4] arm64: mte: Optimize mte_assign_mem_tag_range() Vincenzo Frascino
  3 siblings, 1 reply; 13+ messages in thread
From: Vincenzo Frascino @ 2021-01-07 17:29 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov,
	Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

MTE provides an asynchronous mode for detecting tag exceptions. In
particular instead of triggering a fault the arm64 core updates a
register which is checked by the kernel at the first entry after the tag
exception has occurred.

Add support for MTE asynchronous mode.

The exception handling mechanism will be added with a future patch.

Note: KASAN HW activates async mode via kasan.mode kernel parameter.
The default mode is set to synchronous.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/kernel/mte.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 24a273d47df1..5d992e16b420 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -153,8 +153,35 @@ void mte_init_tags(u64 max_tag)
 
 void mte_enable_kernel(enum kasan_arg_mode mode)
 {
-	/* Enable MTE Sync Mode for EL1. */
-	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
+	const char *m;
+
+	/* Preset parameter values based on the mode. */
+	switch (mode) {
+	case KASAN_ARG_MODE_OFF:
+		return;
+	case KASAN_ARG_MODE_LIGHT:
+		/* Enable MTE Async Mode for EL1. */
+		sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_ASYNC);
+		m = "asynchronous";
+		break;
+	case KASAN_ARG_MODE_DEFAULT:
+	case KASAN_ARG_MODE_PROD:
+	case KASAN_ARG_MODE_FULL:
+		/* Enable MTE Sync Mode for EL1. */
+		sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
+		m = "synchronous";
+		break;
+	default:
+		/*
+		 * kasan mode should be always set hence we should
+		 * not reach this condition.
+		 */
+		WARN_ON_ONCE(1);
+		return;
+	}
+
+	pr_info_once("MTE: enabled in %s mode at EL1\n", m);
+
 	isb();
 }
 
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/4] arm64: mte: Enable async tag check fault
  2021-01-07 17:29 [PATCH v2 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
  2021-01-07 17:29 ` [PATCH v2 1/4] kasan, arm64: Add KASAN light mode Vincenzo Frascino
  2021-01-07 17:29 ` [PATCH v2 2/4] arm64: mte: Add asynchronous mode support Vincenzo Frascino
@ 2021-01-07 17:29 ` Vincenzo Frascino
  2021-01-13 18:11   ` Catalin Marinas
  2021-01-07 17:29 ` [PATCH v2 4/4] arm64: mte: Optimize mte_assign_mem_tag_range() Vincenzo Frascino
  3 siblings, 1 reply; 13+ messages in thread
From: Vincenzo Frascino @ 2021-01-07 17:29 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov,
	Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

MTE provides a mode that asynchronously updates the TFSR_EL1 register
when a tag check exception is detected.

To take advantage of this mode the kernel has to verify the status of
the register at:
  1. Context switching
  2. Return to user/EL0 (Not required in entry from EL0 since the kernel
  did not run)
  3. Kernel entry from EL1
  4. Kernel exit to EL1

If the register is non-zero a trace is reported.

Add the required features for EL1 detection and reporting.

Note: ITFSB bit is set in the SCTLR_EL1 register hence it guaranties that
the indirect writes to TFSR_EL1 are synchronized at exception entry to
EL1. On the context switch path the synchronization is guarantied by the
dsb() in __switch_to().

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/include/asm/mte.h     |  4 ++++
 arch/arm64/kernel/entry-common.c |  6 ++++++
 arch/arm64/kernel/mte.c          | 37 ++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index d02aff9f493d..a60d3718baae 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -39,6 +39,7 @@ void mte_free_tag_storage(char *storage);
 /* track which pages have valid allocation tags */
 #define PG_mte_tagged	PG_arch_2
 
+void mte_check_tfsr_el1(void);
 void mte_sync_tags(pte_t *ptep, pte_t pte);
 void mte_copy_page_tags(void *kto, const void *kfrom);
 void flush_mte_state(void);
@@ -56,6 +57,9 @@ void mte_assign_mem_tag_range(void *addr, size_t size);
 /* unused if !CONFIG_ARM64_MTE, silence the compiler */
 #define PG_mte_tagged	0
 
+static inline void mte_check_tfsr_el1(void)
+{
+}
 static inline void mte_sync_tags(pte_t *ptep, pte_t pte)
 {
 }
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 5346953e4382..74b020ce72d7 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -37,6 +37,8 @@ static void noinstr enter_from_kernel_mode(struct pt_regs *regs)
 	lockdep_hardirqs_off(CALLER_ADDR0);
 	rcu_irq_enter_check_tick();
 	trace_hardirqs_off_finish();
+
+	mte_check_tfsr_el1();
 }
 
 /*
@@ -47,6 +49,8 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs)
 {
 	lockdep_assert_irqs_disabled();
 
+	mte_check_tfsr_el1();
+
 	if (interrupts_enabled(regs)) {
 		if (regs->exit_rcu) {
 			trace_hardirqs_on_prepare();
@@ -243,6 +247,8 @@ asmlinkage void noinstr enter_from_user_mode(void)
 
 asmlinkage void noinstr exit_to_user_mode(void)
 {
+	mte_check_tfsr_el1();
+
 	trace_hardirqs_on_prepare();
 	lockdep_hardirqs_on_prepare(CALLER_ADDR0);
 	user_enter_irqoff();
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 5d992e16b420..26030f0b79fe 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -185,6 +185,34 @@ void mte_enable_kernel(enum kasan_arg_mode mode)
 	isb();
 }
 
+void mte_check_tfsr_el1(void)
+{
+	u64 tfsr_el1;
+
+	if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS))
+		return;
+
+	if (!system_supports_mte())
+		return;
+
+	tfsr_el1 = read_sysreg_s(SYS_TFSR_EL1);
+
+	/*
+	 * The kernel should never hit the condition TF0 == 1
+	 * at this point because for the futex code we set
+	 * PSTATE.TCO.
+	 */
+	WARN_ON(tfsr_el1 & SYS_TFSR_EL1_TF0);
+
+	if (tfsr_el1 & SYS_TFSR_EL1_TF1) {
+		write_sysreg_s(0, SYS_TFSR_EL1);
+		isb();
+
+		pr_err("MTE: Asynchronous tag exception detected!");
+	}
+}
+NOKPROBE_SYMBOL(mte_check_tfsr_el1);
+
 static void update_sctlr_el1_tcf0(u64 tcf0)
 {
 	/* ISB required for the kernel uaccess routines */
@@ -250,6 +278,15 @@ void mte_thread_switch(struct task_struct *next)
 	/* avoid expensive SCTLR_EL1 accesses if no change */
 	if (current->thread.sctlr_tcf0 != next->thread.sctlr_tcf0)
 		update_sctlr_el1_tcf0(next->thread.sctlr_tcf0);
+
+	/*
+	 * Check if an async tag exception occurred at EL1.
+	 *
+	 * Note: On the context switch patch we rely on the dsb() present
+	 * in __switch_to() to guaranty that the indirect writes to TFSR_EL1
+	 * are synchronized before this point.
+	 */
+	mte_check_tfsr_el1();
 }
 
 void mte_suspend_exit(void)
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/4] arm64: mte: Optimize mte_assign_mem_tag_range()
  2021-01-07 17:29 [PATCH v2 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
                   ` (2 preceding siblings ...)
  2021-01-07 17:29 ` [PATCH v2 3/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
@ 2021-01-07 17:29 ` Vincenzo Frascino
  3 siblings, 0 replies; 13+ messages in thread
From: Vincenzo Frascino @ 2021-01-07 17:29 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov,
	Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

mte_assign_mem_tag_range() is called on production KASAN HW hot
paths. It makes sense to optimize it in an attempt to reduce the
overhead.

Optimize mte_assign_mem_tag_range() based on the indications provided at
[1].

[1] https://lore.kernel.org/r/CAAeHK+wCO+J7D1_T89DG+jJrPLk3X9RsGFKxJGd0ZcUFjQT-9Q@mail.gmail.com/

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/include/asm/mte.h | 26 +++++++++++++++++++++++++-
 arch/arm64/lib/mte.S         | 15 ---------------
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index a60d3718baae..c341ce6b9779 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -50,7 +50,31 @@ long get_mte_ctrl(struct task_struct *task);
 int mte_ptrace_copy_tags(struct task_struct *child, long request,
 			 unsigned long addr, unsigned long data);
 
-void mte_assign_mem_tag_range(void *addr, size_t size);
+static inline void mte_assign_mem_tag_range(void *addr, size_t size)
+{
+	u64 _addr = (u64)addr;
+	u64 _end = _addr + size;
+
+	/*
+	 * This function must be invoked from an MTE enabled context.
+	 *
+	 * Note: The address must be non-NULL and MTE_GRANULE_SIZE aligned and
+	 * size must be non-zero and MTE_GRANULE_SIZE aligned.
+	 */
+	do {
+		/*
+		 * 'asm volatile' is required to prevent the compiler to move
+		 * the statement outside of the loop.
+		 */
+		asm volatile(__MTE_PREAMBLE "stg %0, [%0]"
+			     :
+			     : "r" (_addr)
+			     : "memory");
+
+		_addr += MTE_GRANULE_SIZE;
+	} while (_addr < _end);
+}
+
 
 #else /* CONFIG_ARM64_MTE */
 
diff --git a/arch/arm64/lib/mte.S b/arch/arm64/lib/mte.S
index 9e1a12e10053..a0a650451510 100644
--- a/arch/arm64/lib/mte.S
+++ b/arch/arm64/lib/mte.S
@@ -150,18 +150,3 @@ SYM_FUNC_START(mte_restore_page_tags)
 	ret
 SYM_FUNC_END(mte_restore_page_tags)
 
-/*
- * Assign allocation tags for a region of memory based on the pointer tag
- *   x0 - source pointer
- *   x1 - size
- *
- * Note: The address must be non-NULL and MTE_GRANULE_SIZE aligned and
- * size must be non-zero and MTE_GRANULE_SIZE aligned.
- */
-SYM_FUNC_START(mte_assign_mem_tag_range)
-1:	stg	x0, [x0]
-	add	x0, x0, #MTE_GRANULE_SIZE
-	subs	x1, x1, #MTE_GRANULE_SIZE
-	b.gt	1b
-	ret
-SYM_FUNC_END(mte_assign_mem_tag_range)
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] kasan, arm64: Add KASAN light mode
  2021-01-07 17:29 ` [PATCH v2 1/4] kasan, arm64: Add KASAN light mode Vincenzo Frascino
@ 2021-01-13 17:16   ` Catalin Marinas
  2021-01-14  9:40     ` Vincenzo Frascino
  0 siblings, 1 reply; 13+ messages in thread
From: Catalin Marinas @ 2021-01-13 17:16 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: linux-arm-kernel, linux-kernel, kasan-dev, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

On Thu, Jan 07, 2021 at 05:29:05PM +0000, Vincenzo Frascino wrote:
> Architectures supported by KASAN HW can provide a light mode of
> execution. On an MTE enabled arm64 hw for example this can be identified
> with the asynch mode of execution. If an async exception occurs, the
> arm64 core updates a register which is asynchronously detected the next
> time in which the kernel is accessed.

What do you mean by "the kernel is accessed"? Also, there is no
"exception" as such, only a bit in a register updated asynchronously. So
the last sentence could be something like:

  In this mode, if a tag check fault occurs, the TFSR_EL1 register is
  updated asynchronously. The kernel checks the corresponding bits
  periodically.

(or you can be more precise on when the kernel checks for such faults)

> KASAN requires a specific mode of execution to make use of this hw feature.
> 
> Add KASAN HW light execution mode.

Shall we call it "fast"? ;)

> --- /dev/null
> +++ b/include/linux/kasan_def.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_KASAN_DEF_H
> +#define _LINUX_KASAN_DEF_H
> +
> +enum kasan_arg_mode {
> +	KASAN_ARG_MODE_DEFAULT,
> +	KASAN_ARG_MODE_OFF,
> +	KASAN_ARG_MODE_LIGHT,
> +	KASAN_ARG_MODE_PROD,
> +	KASAN_ARG_MODE_FULL,
> +};
> +
> +enum kasan_arg_stacktrace {
> +	KASAN_ARG_STACKTRACE_DEFAULT,
> +	KASAN_ARG_STACKTRACE_OFF,
> +	KASAN_ARG_STACKTRACE_ON,
> +};
> +
> +enum kasan_arg_fault {
> +	KASAN_ARG_FAULT_DEFAULT,
> +	KASAN_ARG_FAULT_REPORT,
> +	KASAN_ARG_FAULT_PANIC,
> +};
> +
> +#endif /* _LINUX_KASAN_DEF_H */

I thought we agreed not to expose the KASAN internal but come up with
another abstraction. Maybe this was after you posted these patches.

-- 
Catalin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/4] arm64: mte: Add asynchronous mode support
  2021-01-07 17:29 ` [PATCH v2 2/4] arm64: mte: Add asynchronous mode support Vincenzo Frascino
@ 2021-01-13 17:22   ` Catalin Marinas
  2021-01-14  9:43     ` Vincenzo Frascino
  0 siblings, 1 reply; 13+ messages in thread
From: Catalin Marinas @ 2021-01-13 17:22 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: linux-arm-kernel, linux-kernel, kasan-dev, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

On Thu, Jan 07, 2021 at 05:29:06PM +0000, Vincenzo Frascino wrote:
> MTE provides an asynchronous mode for detecting tag exceptions. In
> particular instead of triggering a fault the arm64 core updates a
> register which is checked by the kernel at the first entry after the tag
> exception has occurred.

Just rephrase the "tag exception" here as there's no exception taken.
Also we don't check this only when the kernel is first entered after a
tag check fault, as per patch 3.

> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -153,8 +153,35 @@ void mte_init_tags(u64 max_tag)
>  
>  void mte_enable_kernel(enum kasan_arg_mode mode)
>  {
> -	/* Enable MTE Sync Mode for EL1. */
> -	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
> +	const char *m;
> +
> +	/* Preset parameter values based on the mode. */
> +	switch (mode) {
> +	case KASAN_ARG_MODE_OFF:
> +		return;
> +	case KASAN_ARG_MODE_LIGHT:
> +		/* Enable MTE Async Mode for EL1. */
> +		sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_ASYNC);
> +		m = "asynchronous";
> +		break;
> +	case KASAN_ARG_MODE_DEFAULT:
> +	case KASAN_ARG_MODE_PROD:
> +	case KASAN_ARG_MODE_FULL:
> +		/* Enable MTE Sync Mode for EL1. */
> +		sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
> +		m = "synchronous";
> +		break;
> +	default:
> +		/*
> +		 * kasan mode should be always set hence we should
> +		 * not reach this condition.
> +		 */
> +		WARN_ON_ONCE(1);
> +		return;
> +	}

I guess the switch statement here will be re-written as we want kasan to
drive the actual sync/async modes as it sees fit rather than MTE
guessing what PROD/FULL/LIGHT means.

-- 
Catalin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/4] arm64: mte: Enable async tag check fault
  2021-01-07 17:29 ` [PATCH v2 3/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
@ 2021-01-13 18:11   ` Catalin Marinas
  2021-01-14 10:24     ` Vincenzo Frascino
  0 siblings, 1 reply; 13+ messages in thread
From: Catalin Marinas @ 2021-01-13 18:11 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: linux-arm-kernel, linux-kernel, kasan-dev, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

On Thu, Jan 07, 2021 at 05:29:07PM +0000, Vincenzo Frascino wrote:
> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> index d02aff9f493d..a60d3718baae 100644
> --- a/arch/arm64/include/asm/mte.h
> +++ b/arch/arm64/include/asm/mte.h
> @@ -39,6 +39,7 @@ void mte_free_tag_storage(char *storage);
>  /* track which pages have valid allocation tags */
>  #define PG_mte_tagged	PG_arch_2
>  
> +void mte_check_tfsr_el1(void);
>  void mte_sync_tags(pte_t *ptep, pte_t pte);
>  void mte_copy_page_tags(void *kto, const void *kfrom);
>  void flush_mte_state(void);
> @@ -56,6 +57,9 @@ void mte_assign_mem_tag_range(void *addr, size_t size);
>  /* unused if !CONFIG_ARM64_MTE, silence the compiler */
>  #define PG_mte_tagged	0
>  
> +static inline void mte_check_tfsr_el1(void)
> +{
> +}

I think we should enable this dummy function when !CONFIG_KASAN_HW_TAGS.
It saves us an unnecessary function call in a few places.

>  static inline void mte_sync_tags(pte_t *ptep, pte_t pte)
>  {
>  }
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index 5346953e4382..74b020ce72d7 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -37,6 +37,8 @@ static void noinstr enter_from_kernel_mode(struct pt_regs *regs)
>  	lockdep_hardirqs_off(CALLER_ADDR0);
>  	rcu_irq_enter_check_tick();
>  	trace_hardirqs_off_finish();
> +
> +	mte_check_tfsr_el1();
>  }
>  
>  /*
> @@ -47,6 +49,8 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs)
>  {
>  	lockdep_assert_irqs_disabled();
>  
> +	mte_check_tfsr_el1();
> +
>  	if (interrupts_enabled(regs)) {
>  		if (regs->exit_rcu) {
>  			trace_hardirqs_on_prepare();
> @@ -243,6 +247,8 @@ asmlinkage void noinstr enter_from_user_mode(void)
>  
>  asmlinkage void noinstr exit_to_user_mode(void)
>  {
> +	mte_check_tfsr_el1();

While for kernel entry the asynchronous faults are sync'ed automatically
with TFSR_EL1, we don't have this for exit, so we'd need an explicit
DSB. But rather than placing it here, it's better if we add a bool sync
argument to mte_check_tfsr_el1() which issues a dsb() before checking
the register. I think that's the only place where such argument would be
true (for now).

> +
>  	trace_hardirqs_on_prepare();
>  	lockdep_hardirqs_on_prepare(CALLER_ADDR0);
>  	user_enter_irqoff();
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index 5d992e16b420..26030f0b79fe 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -185,6 +185,34 @@ void mte_enable_kernel(enum kasan_arg_mode mode)
>  	isb();
>  }
>  
> +void mte_check_tfsr_el1(void)
> +{
> +	u64 tfsr_el1;
> +
> +	if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS))
> +		return;

If we define the static inline when !CONFIG_KASAN_HW_TAGS, we could add
the #ifdef here around the whole function.

> +	if (!system_supports_mte())
> +		return;
> +
> +	tfsr_el1 = read_sysreg_s(SYS_TFSR_EL1);
> +
> +	/*
> +	 * The kernel should never hit the condition TF0 == 1
> +	 * at this point because for the futex code we set
> +	 * PSTATE.TCO.
> +	 */
> +	WARN_ON(tfsr_el1 & SYS_TFSR_EL1_TF0);
> +
> +	if (tfsr_el1 & SYS_TFSR_EL1_TF1) {
> +		write_sysreg_s(0, SYS_TFSR_EL1);
> +		isb();
> +
> +		pr_err("MTE: Asynchronous tag exception detected!");
> +	}
> +}
> +NOKPROBE_SYMBOL(mte_check_tfsr_el1);

Do we need this to be NOKPROBE_SYMBOL? It's not that low level.

> +
>  static void update_sctlr_el1_tcf0(u64 tcf0)
>  {
>  	/* ISB required for the kernel uaccess routines */
> @@ -250,6 +278,15 @@ void mte_thread_switch(struct task_struct *next)
>  	/* avoid expensive SCTLR_EL1 accesses if no change */
>  	if (current->thread.sctlr_tcf0 != next->thread.sctlr_tcf0)
>  		update_sctlr_el1_tcf0(next->thread.sctlr_tcf0);
> +
> +	/*
> +	 * Check if an async tag exception occurred at EL1.
> +	 *
> +	 * Note: On the context switch patch we rely on the dsb() present

s/patch/path/

> +	 * in __switch_to() to guaranty that the indirect writes to TFSR_EL1

s/guaranty/guarantee/ (well, still valid though I think rarely used).

> +	 * are synchronized before this point.
> +	 */
> +	mte_check_tfsr_el1();
>  }
>  
>  void mte_suspend_exit(void)
> -- 
> 2.30.0

-- 
Catalin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] kasan, arm64: Add KASAN light mode
  2021-01-13 17:16   ` Catalin Marinas
@ 2021-01-14  9:40     ` Vincenzo Frascino
  0 siblings, 0 replies; 13+ messages in thread
From: Vincenzo Frascino @ 2021-01-14  9:40 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-arm-kernel, linux-kernel, kasan-dev, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

On 1/13/21 5:16 PM, Catalin Marinas wrote:
> On Thu, Jan 07, 2021 at 05:29:05PM +0000, Vincenzo Frascino wrote:
>> Architectures supported by KASAN HW can provide a light mode of
>> execution. On an MTE enabled arm64 hw for example this can be identified
>> with the asynch mode of execution. If an async exception occurs, the
>> arm64 core updates a register which is asynchronously detected the next
>> time in which the kernel is accessed.
> 
> What do you mean by "the kernel is accessed"? Also, there is no
> "exception" as such, only a bit in a register updated asynchronously. So
> the last sentence could be something like:
> 
>   In this mode, if a tag check fault occurs, the TFSR_EL1 register is
>   updated asynchronously. The kernel checks the corresponding bits
>   periodically.
> 
> (or you can be more precise on when the kernel checks for such faults)
>

Yes, I agree, I will change it accordingly. What I wrote has a similar meaning
but your exposition is more clear.

>> KASAN requires a specific mode of execution to make use of this hw feature.
>>
>> Add KASAN HW light execution mode.
> 
> Shall we call it "fast"? ;)
> 
>> --- /dev/null
>> +++ b/include/linux/kasan_def.h
>> @@ -0,0 +1,25 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _LINUX_KASAN_DEF_H
>> +#define _LINUX_KASAN_DEF_H
>> +
>> +enum kasan_arg_mode {
>> +	KASAN_ARG_MODE_DEFAULT,
>> +	KASAN_ARG_MODE_OFF,
>> +	KASAN_ARG_MODE_LIGHT,
>> +	KASAN_ARG_MODE_PROD,
>> +	KASAN_ARG_MODE_FULL,
>> +};
>> +
>> +enum kasan_arg_stacktrace {
>> +	KASAN_ARG_STACKTRACE_DEFAULT,
>> +	KASAN_ARG_STACKTRACE_OFF,
>> +	KASAN_ARG_STACKTRACE_ON,
>> +};
>> +
>> +enum kasan_arg_fault {
>> +	KASAN_ARG_FAULT_DEFAULT,
>> +	KASAN_ARG_FAULT_REPORT,
>> +	KASAN_ARG_FAULT_PANIC,
>> +};
>> +
>> +#endif /* _LINUX_KASAN_DEF_H */
> 
> I thought we agreed not to expose the KASAN internal but come up with
> another abstraction. Maybe this was after you posted these patches.
> 

Yes, indeed we agreed and I am going to change it in v3. The agreement
temporally came after I posted v2 hence it is not reflected here.

-- 
Regards,
Vincenzo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/4] arm64: mte: Add asynchronous mode support
  2021-01-13 17:22   ` Catalin Marinas
@ 2021-01-14  9:43     ` Vincenzo Frascino
  0 siblings, 0 replies; 13+ messages in thread
From: Vincenzo Frascino @ 2021-01-14  9:43 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-arm-kernel, linux-kernel, kasan-dev, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov



On 1/13/21 5:22 PM, Catalin Marinas wrote:
> On Thu, Jan 07, 2021 at 05:29:06PM +0000, Vincenzo Frascino wrote:
>> MTE provides an asynchronous mode for detecting tag exceptions. In
>> particular instead of triggering a fault the arm64 core updates a
>> register which is checked by the kernel at the first entry after the tag
>> exception has occurred.
> 
> Just rephrase the "tag exception" here as there's no exception taken.
> Also we don't check this only when the kernel is first entered after a
> tag check fault, as per patch 3.
>

Ok, I will clarify it in v3.

>> --- a/arch/arm64/kernel/mte.c
>> +++ b/arch/arm64/kernel/mte.c
>> @@ -153,8 +153,35 @@ void mte_init_tags(u64 max_tag)
>>  
>>  void mte_enable_kernel(enum kasan_arg_mode mode)
>>  {
>> -	/* Enable MTE Sync Mode for EL1. */
>> -	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
>> +	const char *m;
>> +
>> +	/* Preset parameter values based on the mode. */
>> +	switch (mode) {
>> +	case KASAN_ARG_MODE_OFF:
>> +		return;
>> +	case KASAN_ARG_MODE_LIGHT:
>> +		/* Enable MTE Async Mode for EL1. */
>> +		sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_ASYNC);
>> +		m = "asynchronous";
>> +		break;
>> +	case KASAN_ARG_MODE_DEFAULT:
>> +	case KASAN_ARG_MODE_PROD:
>> +	case KASAN_ARG_MODE_FULL:
>> +		/* Enable MTE Sync Mode for EL1. */
>> +		sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
>> +		m = "synchronous";
>> +		break;
>> +	default:
>> +		/*
>> +		 * kasan mode should be always set hence we should
>> +		 * not reach this condition.
>> +		 */
>> +		WARN_ON_ONCE(1);
>> +		return;
>> +	}
> 
> I guess the switch statement here will be re-written as we want kasan to
> drive the actual sync/async modes as it sees fit rather than MTE
> guessing what PROD/FULL/LIGHT means.
> 

Yes, this is correct, it will present only sync/async mode.

-- 
Regards,
Vincenzo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/4] arm64: mte: Enable async tag check fault
  2021-01-13 18:11   ` Catalin Marinas
@ 2021-01-14 10:24     ` Vincenzo Frascino
  2021-01-14 14:25       ` Catalin Marinas
  0 siblings, 1 reply; 13+ messages in thread
From: Vincenzo Frascino @ 2021-01-14 10:24 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-arm-kernel, linux-kernel, kasan-dev, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov


On 1/13/21 6:11 PM, Catalin Marinas wrote:
> On Thu, Jan 07, 2021 at 05:29:07PM +0000, Vincenzo Frascino wrote:
>> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
>> index d02aff9f493d..a60d3718baae 100644
>> --- a/arch/arm64/include/asm/mte.h
>> +++ b/arch/arm64/include/asm/mte.h
>> @@ -39,6 +39,7 @@ void mte_free_tag_storage(char *storage);
>>  /* track which pages have valid allocation tags */
>>  #define PG_mte_tagged	PG_arch_2
>>  
>> +void mte_check_tfsr_el1(void);
>>  void mte_sync_tags(pte_t *ptep, pte_t pte);
>>  void mte_copy_page_tags(void *kto, const void *kfrom);
>>  void flush_mte_state(void);
>> @@ -56,6 +57,9 @@ void mte_assign_mem_tag_range(void *addr, size_t size);
>>  /* unused if !CONFIG_ARM64_MTE, silence the compiler */
>>  #define PG_mte_tagged	0
>>  
>> +static inline void mte_check_tfsr_el1(void)
>> +{
>> +}
> 
> I think we should enable this dummy function when !CONFIG_KASAN_HW_TAGS.
> It saves us an unnecessary function call in a few places.
> 

Ok, I will add it in v3.

>>  static inline void mte_sync_tags(pte_t *ptep, pte_t pte)
>>  {
>>  }
>> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
>> index 5346953e4382..74b020ce72d7 100644
>> --- a/arch/arm64/kernel/entry-common.c
>> +++ b/arch/arm64/kernel/entry-common.c
>> @@ -37,6 +37,8 @@ static void noinstr enter_from_kernel_mode(struct pt_regs *regs)
>>  	lockdep_hardirqs_off(CALLER_ADDR0);
>>  	rcu_irq_enter_check_tick();
>>  	trace_hardirqs_off_finish();
>> +
>> +	mte_check_tfsr_el1();
>>  }
>>  
>>  /*
>> @@ -47,6 +49,8 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs)
>>  {
>>  	lockdep_assert_irqs_disabled();
>>  
>> +	mte_check_tfsr_el1();
>> +
>>  	if (interrupts_enabled(regs)) {
>>  		if (regs->exit_rcu) {
>>  			trace_hardirqs_on_prepare();
>> @@ -243,6 +247,8 @@ asmlinkage void noinstr enter_from_user_mode(void)
>>  
>>  asmlinkage void noinstr exit_to_user_mode(void)
>>  {
>> +	mte_check_tfsr_el1();
> 
> While for kernel entry the asynchronous faults are sync'ed automatically
> with TFSR_EL1, we don't have this for exit, so we'd need an explicit
> DSB. But rather than placing it here, it's better if we add a bool sync
> argument to mte_check_tfsr_el1() which issues a dsb() before checking
> the register. I think that's the only place where such argument would be
> true (for now).
> 

Good point, I will add the dsb() in mte_check_tfsr_el1() but instead of a bool
parameter I will add something more explicit.

>> +
>>  	trace_hardirqs_on_prepare();
>>  	lockdep_hardirqs_on_prepare(CALLER_ADDR0);
>>  	user_enter_irqoff();
>> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
>> index 5d992e16b420..26030f0b79fe 100644
>> --- a/arch/arm64/kernel/mte.c
>> +++ b/arch/arm64/kernel/mte.c
>> @@ -185,6 +185,34 @@ void mte_enable_kernel(enum kasan_arg_mode mode)
>>  	isb();
>>  }
>>  
>> +void mte_check_tfsr_el1(void)
>> +{
>> +	u64 tfsr_el1;
>> +
>> +	if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS))
>> +		return;
> 
> If we define the static inline when !CONFIG_KASAN_HW_TAGS, we could add
> the #ifdef here around the whole function.
>

Ok. I will add it in v3.

>> +	if (!system_supports_mte())
>> +		return;
>> +
>> +	tfsr_el1 = read_sysreg_s(SYS_TFSR_EL1);
>> +
>> +	/*
>> +	 * The kernel should never hit the condition TF0 == 1
>> +	 * at this point because for the futex code we set
>> +	 * PSTATE.TCO.
>> +	 */
>> +	WARN_ON(tfsr_el1 & SYS_TFSR_EL1_TF0);
>> +
>> +	if (tfsr_el1 & SYS_TFSR_EL1_TF1) {
>> +		write_sysreg_s(0, SYS_TFSR_EL1);
>> +		isb();
>> +
>> +		pr_err("MTE: Asynchronous tag exception detected!");
>> +	}
>> +}
>> +NOKPROBE_SYMBOL(mte_check_tfsr_el1);
> 
> Do we need this to be NOKPROBE_SYMBOL? It's not that low level.
>
It is an inheritance from when I had this code called very early. I will remove
it in the next version.

>> +
>>  static void update_sctlr_el1_tcf0(u64 tcf0)
>>  {
>>  	/* ISB required for the kernel uaccess routines */
>> @@ -250,6 +278,15 @@ void mte_thread_switch(struct task_struct *next)
>>  	/* avoid expensive SCTLR_EL1 accesses if no change */
>>  	if (current->thread.sctlr_tcf0 != next->thread.sctlr_tcf0)
>>  		update_sctlr_el1_tcf0(next->thread.sctlr_tcf0);
>> +
>> +	/*
>> +	 * Check if an async tag exception occurred at EL1.
>> +	 *
>> +	 * Note: On the context switch patch we rely on the dsb() present
> 
> s/patch/path/
> 
>> +	 * in __switch_to() to guaranty that the indirect writes to TFSR_EL1
> 
> s/guaranty/guarantee/ (well, still valid though I think rarely used).
> 
>> +	 * are synchronized before this point.
>> +	 */
>> +	mte_check_tfsr_el1();
>>  }
>>  
>>  void mte_suspend_exit(void)
>> -- 
>> 2.30.0
> 

-- 
Regards,
Vincenzo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/4] arm64: mte: Enable async tag check fault
  2021-01-14 10:24     ` Vincenzo Frascino
@ 2021-01-14 14:25       ` Catalin Marinas
  2021-01-14 14:57         ` Vincenzo Frascino
  0 siblings, 1 reply; 13+ messages in thread
From: Catalin Marinas @ 2021-01-14 14:25 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: linux-arm-kernel, linux-kernel, kasan-dev, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

On Thu, Jan 14, 2021 at 10:24:25AM +0000, Vincenzo Frascino wrote:
> On 1/13/21 6:11 PM, Catalin Marinas wrote:
> > On Thu, Jan 07, 2021 at 05:29:07PM +0000, Vincenzo Frascino wrote:
> >>  static inline void mte_sync_tags(pte_t *ptep, pte_t pte)
> >>  {
> >>  }
> >> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> >> index 5346953e4382..74b020ce72d7 100644
> >> --- a/arch/arm64/kernel/entry-common.c
> >> +++ b/arch/arm64/kernel/entry-common.c
> >> @@ -37,6 +37,8 @@ static void noinstr enter_from_kernel_mode(struct pt_regs *regs)
> >>  	lockdep_hardirqs_off(CALLER_ADDR0);
> >>  	rcu_irq_enter_check_tick();
> >>  	trace_hardirqs_off_finish();
> >> +
> >> +	mte_check_tfsr_el1();
> >>  }
> >>  
> >>  /*
> >> @@ -47,6 +49,8 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs)
> >>  {
> >>  	lockdep_assert_irqs_disabled();
> >>  
> >> +	mte_check_tfsr_el1();
> >> +
> >>  	if (interrupts_enabled(regs)) {
> >>  		if (regs->exit_rcu) {
> >>  			trace_hardirqs_on_prepare();
> >> @@ -243,6 +247,8 @@ asmlinkage void noinstr enter_from_user_mode(void)
> >>  
> >>  asmlinkage void noinstr exit_to_user_mode(void)
> >>  {
> >> +	mte_check_tfsr_el1();
> > 
> > While for kernel entry the asynchronous faults are sync'ed automatically
> > with TFSR_EL1, we don't have this for exit, so we'd need an explicit
> > DSB. But rather than placing it here, it's better if we add a bool sync
> > argument to mte_check_tfsr_el1() which issues a dsb() before checking
> > the register. I think that's the only place where such argument would be
> > true (for now).
> 
> Good point, I will add the dsb() in mte_check_tfsr_el1() but instead of a bool
> parameter I will add something more explicit.

Or rename the function to mte_check_tfsr_el1_no_sync() and have a static
inline mte_check_tfsr_el1() which issues a dsb() before calling the
*no_sync variant.

Adding an enum instead here is not worth it (if that's what you meant by
not using a bool).

-- 
Catalin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/4] arm64: mte: Enable async tag check fault
  2021-01-14 14:25       ` Catalin Marinas
@ 2021-01-14 14:57         ` Vincenzo Frascino
  0 siblings, 0 replies; 13+ messages in thread
From: Vincenzo Frascino @ 2021-01-14 14:57 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-arm-kernel, linux-kernel, kasan-dev, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov, Andrey Konovalov



On 1/14/21 2:25 PM, Catalin Marinas wrote:
> On Thu, Jan 14, 2021 at 10:24:25AM +0000, Vincenzo Frascino wrote:
>> On 1/13/21 6:11 PM, Catalin Marinas wrote:
>>> On Thu, Jan 07, 2021 at 05:29:07PM +0000, Vincenzo Frascino wrote:
>>>>  static inline void mte_sync_tags(pte_t *ptep, pte_t pte)
>>>>  {
>>>>  }
>>>> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
>>>> index 5346953e4382..74b020ce72d7 100644
>>>> --- a/arch/arm64/kernel/entry-common.c
>>>> +++ b/arch/arm64/kernel/entry-common.c
>>>> @@ -37,6 +37,8 @@ static void noinstr enter_from_kernel_mode(struct pt_regs *regs)
>>>>  	lockdep_hardirqs_off(CALLER_ADDR0);
>>>>  	rcu_irq_enter_check_tick();
>>>>  	trace_hardirqs_off_finish();
>>>> +
>>>> +	mte_check_tfsr_el1();
>>>>  }
>>>>  
>>>>  /*
>>>> @@ -47,6 +49,8 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs)
>>>>  {
>>>>  	lockdep_assert_irqs_disabled();
>>>>  
>>>> +	mte_check_tfsr_el1();
>>>> +
>>>>  	if (interrupts_enabled(regs)) {
>>>>  		if (regs->exit_rcu) {
>>>>  			trace_hardirqs_on_prepare();
>>>> @@ -243,6 +247,8 @@ asmlinkage void noinstr enter_from_user_mode(void)
>>>>  
>>>>  asmlinkage void noinstr exit_to_user_mode(void)
>>>>  {
>>>> +	mte_check_tfsr_el1();
>>>
>>> While for kernel entry the asynchronous faults are sync'ed automatically
>>> with TFSR_EL1, we don't have this for exit, so we'd need an explicit
>>> DSB. But rather than placing it here, it's better if we add a bool sync
>>> argument to mte_check_tfsr_el1() which issues a dsb() before checking
>>> the register. I think that's the only place where such argument would be
>>> true (for now).
>>
>> Good point, I will add the dsb() in mte_check_tfsr_el1() but instead of a bool
>> parameter I will add something more explicit.
> 
> Or rename the function to mte_check_tfsr_el1_no_sync() and have a static
> inline mte_check_tfsr_el1() which issues a dsb() before calling the
> *no_sync variant.
> 
> Adding an enum instead here is not worth it (if that's what you meant by
> not using a bool).
> 

I like this option more, thanks for pointing it out.

-- 
Regards,
Vincenzo

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-01-14 14:54 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-07 17:29 [PATCH v2 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
2021-01-07 17:29 ` [PATCH v2 1/4] kasan, arm64: Add KASAN light mode Vincenzo Frascino
2021-01-13 17:16   ` Catalin Marinas
2021-01-14  9:40     ` Vincenzo Frascino
2021-01-07 17:29 ` [PATCH v2 2/4] arm64: mte: Add asynchronous mode support Vincenzo Frascino
2021-01-13 17:22   ` Catalin Marinas
2021-01-14  9:43     ` Vincenzo Frascino
2021-01-07 17:29 ` [PATCH v2 3/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
2021-01-13 18:11   ` Catalin Marinas
2021-01-14 10:24     ` Vincenzo Frascino
2021-01-14 14:25       ` Catalin Marinas
2021-01-14 14:57         ` Vincenzo Frascino
2021-01-07 17:29 ` [PATCH v2 4/4] arm64: mte: Optimize mte_assign_mem_tag_range() Vincenzo Frascino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).