linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support
@ 2021-01-06 11:55 Vincenzo Frascino
  2021-01-06 11:55 ` [PATCH 1/4] kasan, arm64: Add KASAN light mode Vincenzo Frascino
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Vincenzo Frascino @ 2021-01-06 11:55 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

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
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       | 27 +++++++++++-
 arch/arm64/kernel/entry-common.c   |  6 +++
 arch/arm64/kernel/mte.c            | 67 ++++++++++++++++++++++++++++--
 arch/arm64/lib/mte.S               | 15 -------
 include/linux/kasan.h              |  1 +
 include/linux/kasan_def.h          | 39 +++++++++++++++++
 mm/kasan/hw_tags.c                 | 24 ++---------
 mm/kasan/kasan.h                   |  2 +-
 10 files changed, 145 insertions(+), 43 deletions(-)
 create mode 100644 include/linux/kasan_def.h

-- 
2.29.2


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

* [PATCH 1/4] kasan, arm64: Add KASAN light mode
  2021-01-06 11:55 [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
@ 2021-01-06 11:55 ` Vincenzo Frascino
  2021-01-06 11:55 ` [PATCH 2/4] arm64: mte: Add asynchronous mode support Vincenzo Frascino
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Vincenzo Frascino @ 2021-01-06 11:55 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          | 39 ++++++++++++++++++++++++++++++
 mm/kasan/hw_tags.c                 | 24 +++---------------
 mm/kasan/kasan.h                   |  2 +-
 7 files changed, 50 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..5e2b3ea5472b
--- /dev/null
+++ b/include/linux/kasan_def.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_KASAN_DEF_H
+#define _LINUX_KASAN_DEF_H
+
+#ifdef CONFIG_KASAN
+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,
+};
+#else
+enum kasan_arg_mode {
+	KASAN_ARG_MODE_DEFAULT,
+};
+
+enum kasan_arg_stacktrace {
+	KASAN_ARG_STACKTRACE_DEFAULT,
+};
+
+enum kasan_arg_fault {
+	KASAN_ARG_FAULT_DEFAULT,
+};
+#endif
+
+#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.29.2


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

* [PATCH 2/4] arm64: mte: Add asynchronous mode support
  2021-01-06 11:55 [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
  2021-01-06 11:55 ` [PATCH 1/4] kasan, arm64: Add KASAN light mode Vincenzo Frascino
@ 2021-01-06 11:55 ` Vincenzo Frascino
  2021-01-07 16:29   ` Andrey Konovalov
  2021-01-06 11:55 ` [PATCH 3/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Vincenzo Frascino @ 2021-01-06 11:55 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.deacon@arm.com>
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.29.2


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

* [PATCH 3/4] arm64: mte: Enable async tag check fault
  2021-01-06 11:55 [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
  2021-01-06 11:55 ` [PATCH 1/4] kasan, arm64: Add KASAN light mode Vincenzo Frascino
  2021-01-06 11:55 ` [PATCH 2/4] arm64: mte: Add asynchronous mode support Vincenzo Frascino
@ 2021-01-06 11:55 ` Vincenzo Frascino
  2021-01-06 15:01   ` kernel test robot
  2021-01-06 11:55 ` [PATCH 4/4] arm64: mte: Optimize mte_assign_mem_tag_range() Vincenzo Frascino
  2021-01-06 16:35 ` [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
  4 siblings, 1 reply; 14+ messages in thread
From: Vincenzo Frascino @ 2021-01-06 11:55 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.deacon@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/include/asm/mte.h     |  1 +
 arch/arm64/kernel/entry-common.c |  6 ++++++
 arch/arm64/kernel/mte.c          | 34 ++++++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+)

diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index d02aff9f493d..c757ff756e09 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -33,6 +33,7 @@ void mte_invalidate_tags(int type, pgoff_t offset);
 void mte_invalidate_tags_area(int type);
 void *mte_allocate_tag_storage(void);
 void mte_free_tag_storage(char *storage);
+void mte_check_tfsr_el1(void);
 
 #ifdef CONFIG_ARM64_MTE
 
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..7082fc287635 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -185,6 +185,31 @@ void mte_enable_kernel(enum kasan_arg_mode mode)
 	isb();
 }
 
+void mte_check_tfsr_el1(void)
+{
+	u64 tfsr_el1;
+
+	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 +275,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.29.2


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

* [PATCH 4/4] arm64: mte: Optimize mte_assign_mem_tag_range()
  2021-01-06 11:55 [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
                   ` (2 preceding siblings ...)
  2021-01-06 11:55 ` [PATCH 3/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
@ 2021-01-06 11:55 ` Vincenzo Frascino
  2021-01-06 16:35 ` [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
  4 siblings, 0 replies; 14+ messages in thread
From: Vincenzo Frascino @ 2021-01-06 11:55 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.deacon@arm.com>
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 c757ff756e09..ac134a74e1a1 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.29.2


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

* Re: [PATCH 3/4] arm64: mte: Enable async tag check fault
  2021-01-06 11:55 ` [PATCH 3/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
@ 2021-01-06 15:01   ` kernel test robot
  0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-01-06 15:01 UTC (permalink / raw)
  To: Vincenzo Frascino, linux-arm-kernel, linux-kernel, kasan-dev
  Cc: kbuild-all, Vincenzo Frascino, Catalin Marinas, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver

[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]

Hi Vincenzo,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.11-rc2 next-20210104]
[cannot apply to arm64/for-next/core soc/for-next arm/for-next xlnx/master kvmarm/next hnaz-linux-mm/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210106-200352
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git e71ba9452f0b5b2e8dc8aa5445198cd9214a6a62
config: arm64-randconfig-m031-20210106 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/f4149cbb992741e9fc73e8d0e787bd34eda7d4a4
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210106-200352
        git checkout f4149cbb992741e9fc73e8d0e787bd34eda7d4a4
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   aarch64-linux-ld: Unexpected GOT/PLT entries detected!
   aarch64-linux-ld: Unexpected run-time procedure linkages detected!
   aarch64-linux-ld: arch/arm64/kernel/entry-common.o: in function `enter_from_kernel_mode.isra.0':
>> entry-common.c:(.noinstr.text+0x4c): undefined reference to `mte_check_tfsr_el1'
   aarch64-linux-ld: arch/arm64/kernel/entry-common.o: in function `exit_to_kernel_mode':
   entry-common.c:(.noinstr.text+0x2b0): undefined reference to `mte_check_tfsr_el1'
   aarch64-linux-ld: arch/arm64/kernel/entry-common.o: in function `exit_to_user_mode':
   entry-common.c:(.noinstr.text+0x1730): undefined reference to `mte_check_tfsr_el1'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 43761 bytes --]

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

* Re: [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support
  2021-01-06 11:55 [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
                   ` (3 preceding siblings ...)
  2021-01-06 11:55 ` [PATCH 4/4] arm64: mte: Optimize mte_assign_mem_tag_range() Vincenzo Frascino
@ 2021-01-06 16:35 ` Vincenzo Frascino
  2021-01-06 16:42   ` Will Deacon
  4 siblings, 1 reply; 14+ messages in thread
From: Vincenzo Frascino @ 2021-01-06 16:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, kasan-dev
  Cc: Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin,
	Alexander Potapenko, Marco Elver, Evgenii Stepanov,
	Branislav Rankov, Andrey Konovalov, Will Deacon


On 1/6/21 11:55 AM, Vincenzo Frascino wrote:
> 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
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>

Will is not in arm anymore :( Sorry Will... I will fix this in v2.

> 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       | 27 +++++++++++-
>  arch/arm64/kernel/entry-common.c   |  6 +++
>  arch/arm64/kernel/mte.c            | 67 ++++++++++++++++++++++++++++--
>  arch/arm64/lib/mte.S               | 15 -------
>  include/linux/kasan.h              |  1 +
>  include/linux/kasan_def.h          | 39 +++++++++++++++++
>  mm/kasan/hw_tags.c                 | 24 ++---------
>  mm/kasan/kasan.h                   |  2 +-
>  10 files changed, 145 insertions(+), 43 deletions(-)
>  create mode 100644 include/linux/kasan_def.h
> 

-- 
Regards,
Vincenzo

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

* Re: [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support
  2021-01-06 16:35 ` [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
@ 2021-01-06 16:42   ` Will Deacon
  0 siblings, 0 replies; 14+ messages in thread
From: Will Deacon @ 2021-01-06 16:42 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: linux-arm-kernel, linux-kernel, kasan-dev, Catalin Marinas,
	Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko,
	Marco Elver, Evgenii Stepanov, Branislav Rankov,
	Andrey Konovalov

On Wed, Jan 06, 2021 at 04:35:29PM +0000, Vincenzo Frascino wrote:
> 
> On 1/6/21 11:55 AM, Vincenzo Frascino wrote:
> > 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
> > 
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> 
> Will is not in arm anymore :( Sorry Will... I will fix this in v2.

If only you worked for payroll ;)

Will

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

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

On Wed, Jan 6, 2021 at 12:56 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> 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.
>
> 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.deacon@arm.com>
> 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.29.2
>

Hi Vincenzo,

It would be cleaner to pass a bool to mte_enable_kernel() and have it
indicate sync/async mode. This way you don't have to pull all these
KASAN constants into the arm64 code.

Thanks!

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

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

Hi Andrey,

On 1/7/21 4:29 PM, Andrey Konovalov wrote:
> On Wed, Jan 6, 2021 at 12:56 PM Vincenzo Frascino
> <vincenzo.frascino@arm.com> 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.
>>
>> 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.deacon@arm.com>
>> 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.29.2
>>
> 
> Hi Vincenzo,
> 
> It would be cleaner to pass a bool to mte_enable_kernel() and have it
> indicate sync/async mode. This way you don't have to pull all these
> KASAN constants into the arm64 code.
>

Boolean arguments are generally bad for legibility, hence I tend to avoid them.
In this case exposing the constants does not seem a big issue especially because
the only user of this code is "KASAN_HW_TAGS" and definitely improves its
legibility hence I would prefer to keep it as is.
> Thanks!
> 

-- 
Regards,
Vincenzo

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

* Re: [PATCH 2/4] arm64: mte: Add asynchronous mode support
  2021-01-07 17:29     ` Vincenzo Frascino
@ 2021-01-07 19:18       ` Andrey Konovalov
  2021-01-08 10:48         ` Vincenzo Frascino
  0 siblings, 1 reply; 14+ messages in thread
From: Andrey Konovalov @ 2021-01-07 19:18 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov

On Thu, Jan 7, 2021 at 6:25 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Andrey,
>
> On 1/7/21 4:29 PM, Andrey Konovalov wrote:
> > On Wed, Jan 6, 2021 at 12:56 PM Vincenzo Frascino
> > <vincenzo.frascino@arm.com> 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.
> >>
> >> 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.deacon@arm.com>
> >> 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.29.2
> >>
> >
> > Hi Vincenzo,
> >
> > It would be cleaner to pass a bool to mte_enable_kernel() and have it
> > indicate sync/async mode. This way you don't have to pull all these
> > KASAN constants into the arm64 code.
> >
>
> Boolean arguments are generally bad for legibility, hence I tend to avoid them.
> In this case exposing the constants does not seem a big issue especially because
> the only user of this code is "KASAN_HW_TAGS" and definitely improves its
> legibility hence I would prefer to keep it as is.

I don't like that this spills KASAN internals to the arm64 code. Let's
add another enum with two values and pass it as an argument then.
Something like:

enum mte_mode {
  ARM_MTE_SYNC,
  ARM_MTE_ASYNC
}

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

* Re: [PATCH 2/4] arm64: mte: Add asynchronous mode support
  2021-01-07 19:18       ` Andrey Konovalov
@ 2021-01-08 10:48         ` Vincenzo Frascino
  2021-01-08 13:36           ` Andrey Konovalov
  0 siblings, 1 reply; 14+ messages in thread
From: Vincenzo Frascino @ 2021-01-08 10:48 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov

Hi Andrey,

On 1/7/21 7:18 PM, Andrey Konovalov wrote:
>> Boolean arguments are generally bad for legibility, hence I tend to avoid them.
>> In this case exposing the constants does not seem a big issue especially because
>> the only user of this code is "KASAN_HW_TAGS" and definitely improves its
>> legibility hence I would prefer to keep it as is.
>
> I don't like that this spills KASAN internals to the arm64 code.

Could you please elaborate a bit more on this?

If I understand it correctly these enumerations I exposed are the direct
representation of a kernel command line parameter which, according to me, should
not be considered an internal interface.
Seems that in general the kernel subsystems expose the interface for the
architectures to consume which is the same design pattern I followed in this case.

> Let's add another enum with two values and pass it as an argument then.
> Something like:
> 
> enum mte_mode {
>   ARM_MTE_SYNC,
>   ARM_MTE_ASYNC
> }

I had something similar at the beginning of the development but I ended up in a
situation in which the generic kasan code had to know about "enum mte_mode",
hence I preferred to keep kasan agnostic to the hw implementation details.

What do you think?

-- 
Regards,
Vincenzo

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

* Re: [PATCH 2/4] arm64: mte: Add asynchronous mode support
  2021-01-08 10:48         ` Vincenzo Frascino
@ 2021-01-08 13:36           ` Andrey Konovalov
  2021-01-08 17:26             ` Vincenzo Frascino
  0 siblings, 1 reply; 14+ messages in thread
From: Andrey Konovalov @ 2021-01-08 13:36 UTC (permalink / raw)
  To: Vincenzo Frascino
  Cc: Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon,
	Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver,
	Evgenii Stepanov, Branislav Rankov

On Fri, Jan 8, 2021 at 11:44 AM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Andrey,
>
> On 1/7/21 7:18 PM, Andrey Konovalov wrote:
> >> Boolean arguments are generally bad for legibility, hence I tend to avoid them.
> >> In this case exposing the constants does not seem a big issue especially because
> >> the only user of this code is "KASAN_HW_TAGS" and definitely improves its
> >> legibility hence I would prefer to keep it as is.
> >
> > I don't like that this spills KASAN internals to the arm64 code.
>
> Could you please elaborate a bit more on this?
>
> If I understand it correctly these enumerations I exposed are the direct
> representation of a kernel command line parameter which, according to me, should
> not be considered an internal interface.
> Seems that in general the kernel subsystems expose the interface for the
> architectures to consume which is the same design pattern I followed in this case.

It's fine from the point of view of kernel interfaces and such, but
not from a higher-level design perspective.

I think the best way to approach the KASAN-MTE architecture is: 1.
arm64 code provides API to enable, disable and otherwise work with
MTE, and 2. KASAN builds on top of this API to implement the logic of
the bug detector, including which APIs to use. Part #2 includes making
the decisions about which mode - sync or async - to use and when. And
that mode is chosen by KASAN code based on the command line configs.

With your current approach, the active decision about enabling
sync/async is made by the arm64 code, and that doesn't fit within this
architecture. But having a decisionless arm64 API to choose the MTE
mode and using it from KASAN code would fit.

> > Let's add another enum with two values and pass it as an argument then.
> > Something like:
> >
> > enum mte_mode {
> >   ARM_MTE_SYNC,
> >   ARM_MTE_ASYNC
> > }
>
> I had something similar at the beginning of the development but I ended up in a
> situation in which the generic kasan code had to know about "enum mte_mode",
> hence I preferred to keep kasan agnostic to the hw implementation details.
>
> What do you think?

Perhaps we could add a generic arch-agnostic enum to
include/linux/kasan.h and use it in both arm64 and KASAN code?

enum kasan_hw_tags_mode {
  KASAN_HW_TAGS_SYNC,
  KASAN_HW_TAGS_ASYNC
}

Assuming other architectures that support memory tagging will end up
with sync/async mode separation as well, this should work. But even if
that doesn't happen, this interface can be adjusted later.

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

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

Hi Andrey,

On 1/8/21 1:36 PM, Andrey Konovalov wrote:
> Perhaps we could add a generic arch-agnostic enum to
> include/linux/kasan.h and use it in both arm64 and KASAN code?
> 
> enum kasan_hw_tags_mode {
>   KASAN_HW_TAGS_SYNC,
>   KASAN_HW_TAGS_ASYNC
> }
> 
> Assuming other architectures that support memory tagging will end up
> with sync/async mode separation as well, this should work. But even if
> that doesn't happen, this interface can be adjusted later.

I am fine with this solution, I will add it in my v3.
As part of the enumeration I will add READ_SYNC mode as well, so we have all the
possible combinations.

-- 
Regards,
Vincenzo

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

end of thread, other threads:[~2021-01-08 17:23 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-06 11:55 [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
2021-01-06 11:55 ` [PATCH 1/4] kasan, arm64: Add KASAN light mode Vincenzo Frascino
2021-01-06 11:55 ` [PATCH 2/4] arm64: mte: Add asynchronous mode support Vincenzo Frascino
2021-01-07 16:29   ` Andrey Konovalov
2021-01-07 17:29     ` Vincenzo Frascino
2021-01-07 19:18       ` Andrey Konovalov
2021-01-08 10:48         ` Vincenzo Frascino
2021-01-08 13:36           ` Andrey Konovalov
2021-01-08 17:26             ` Vincenzo Frascino
2021-01-06 11:55 ` [PATCH 3/4] arm64: mte: Enable async tag check fault Vincenzo Frascino
2021-01-06 15:01   ` kernel test robot
2021-01-06 11:55 ` [PATCH 4/4] arm64: mte: Optimize mte_assign_mem_tag_range() Vincenzo Frascino
2021-01-06 16:35 ` [PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
2021-01-06 16:42   ` Will Deacon

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).