[PATCH 0/4] arm64: ARMv8.5-A: MTE: Add async mode support

127 views
Skip to first unread message

Vincenzo Frascino

unread,
Jan 6, 2021, 6:56:40 AM1/6/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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...@arm.com>
Cc: Will Deacon <will....@arm.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Marco Elver <el...@google.com>
Cc: Evgenii Stepanov <eug...@google.com>
Cc: Branislav Rankov <Branisla...@arm.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@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

Vincenzo Frascino

unread,
Jan 6, 2021, 6:56:41 AM1/6/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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 <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@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

Vincenzo Frascino

unread,
Jan 6, 2021, 6:56:43 AM1/6/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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...@arm.com>
Cc: Will Deacon <will....@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@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

Vincenzo Frascino

unread,
Jan 6, 2021, 6:56:45 AM1/6/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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...@arm.com>
Cc: Will Deacon <will....@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@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

Vincenzo Frascino

unread,
Jan 6, 2021, 6:56:46 AM1/6/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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+jJr...@mail.gmail.com/

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <will....@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@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

kernel test robot

unread,
Jan 6, 2021, 10:03:01 AM1/6/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, kbuil...@lists.01.org, Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver
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 <l...@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/kbuil...@lists.01.org
.config.gz

Vincenzo Frascino

unread,
Jan 6, 2021, 11:31:53 AM1/6/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov, Will Deacon
Will is not in arm anymore :( Sorry Will... I will fix this in v2.
Regards,
Vincenzo

Will Deacon

unread,
Jan 6, 2021, 11:42:32 AM1/6/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
If only you worked for payroll ;)

Will

Andrey Konovalov

unread,
Jan 7, 2021, 11:29:51 AM1/7/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
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!

Vincenzo Frascino

unread,
Jan 7, 2021, 12:25:48 PM1/7/21
to Andrey Konovalov, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
Hi Andrey,
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

Vincenzo Frascino

unread,
Jan 7, 2021, 12:30:23 PM1/7/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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 <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@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/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,
+};
2.30.0

Vincenzo Frascino

unread,
Jan 7, 2021, 12:30:23 PM1/7/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
Changes:
--------
v2:
- Fixed a compilation issue reported by krobot.
- General cleanup.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Marco Elver <el...@google.com>
Cc: Evgenii Stepanov <eug...@google.com>
Cc: Branislav Rankov <Branisla...@arm.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@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

Vincenzo Frascino

unread,
Jan 7, 2021, 12:30:25 PM1/7/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo...@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
2.30.0

Vincenzo Frascino

unread,
Jan 7, 2021, 12:30:27 PM1/7/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo...@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/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

Vincenzo Frascino

unread,
Jan 7, 2021, 12:30:32 PM1/7/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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+jJr...@mail.gmail.com/

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo...@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.
+ *
2.30.0

Andrey Konovalov

unread,
Jan 7, 2021, 2:18:25 PM1/7/21
to Vincenzo Frascino, 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
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
}

Vincenzo Frascino

unread,
Jan 8, 2021, 5:44:37 AM1/8/21
to Andrey Konovalov, 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

Andrey Konovalov

unread,
Jan 8, 2021, 8:37:00 AM1/8/21
to Vincenzo Frascino, 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...@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.

Vincenzo Frascino

unread,
Jan 8, 2021, 12:22:33 PM1/8/21
to Andrey Konovalov, 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

Catalin Marinas

unread,
Jan 13, 2021, 12:16:09 PM1/13/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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"? ;)
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

Catalin Marinas

unread,
Jan 13, 2021, 12:22:34 PM1/13/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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.
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

Catalin Marinas

unread,
Jan 13, 2021, 1:11:27 PM1/13/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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

Vincenzo Frascino

unread,
Jan 14, 2021, 4:37:20 AM1/14/21
to Catalin Marinas, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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.
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

Vincenzo Frascino

unread,
Jan 14, 2021, 4:39:37 AM1/14/21
to Catalin Marinas, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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.
Yes, this is correct, it will present only sync/async mode.

--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 14, 2021, 5:20:42 AM1/14/21
to Catalin Marinas, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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.
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

Catalin Marinas

unread,
Jan 14, 2021, 9:25:19 AM1/14/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
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

Vincenzo Frascino

unread,
Jan 14, 2021, 9:53:21 AM1/14/21
to Catalin Marinas, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
I like this option more, thanks for pointing it out.

--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 15, 2021, 7:00:54 AM1/15/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
The series is based on linux 5.11-rc3.

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:
--------
v3:
- Exposed kasan_hw_tags_mode to convert the internal
KASAN represenetation.
- Added dsb() for kernel exit paths in arm64.
- Addressed review comments.
v2:
- Fixed a compilation issue reported by krobot.
- General cleanup.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Marco Elver <el...@google.com>
Cc: Evgenii Stepanov <eug...@google.com>
Cc: Branislav Rankov <Branisla...@arm.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@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 | 47 +++++++++++++++++++++-
arch/arm64/kernel/entry-common.c | 11 ++++++
arch/arm64/kernel/mte.c | 63 ++++++++++++++++++++++++++++--
arch/arm64/lib/mte.S | 15 -------
include/linux/kasan.h | 1 +
include/linux/kasan_def.h | 10 +++++
mm/kasan/hw_tags.c | 19 ++++++++-
mm/kasan/kasan.h | 2 +-
10 files changed, 151 insertions(+), 24 deletions(-)

Vincenzo Frascino

unread,
Jan 15, 2021, 7:00:56 AM1/15/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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.
In this mode, if a tag check fault occurs, the TFSR_EL1 register is
updated asynchronously. The kernel checks the corresponding bits
periodically.

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 <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@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 | 10 ++++++++++
mm/kasan/hw_tags.c | 19 ++++++++++++++++++-
mm/kasan/kasan.h | 2 +-
7 files changed, 35 insertions(+), 6 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..5402f4c8e88d 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_hw_tags_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_hw_tags_mode mode)
{
}

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index dc9ada64feed..53a6d734e29b 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_hw_tags_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..0a55400809c9
--- /dev/null
+++ b/include/linux/kasan_def.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_KASAN_DEF_H
+#define _LINUX_KASAN_DEF_H
+
+enum kasan_hw_tags_mode {
+ KASAN_HW_TAGS_SYNC,
+ KASAN_HW_TAGS_ASYNC,
+};
+
+#endif /* _LINUX_KASAN_DEF_H */
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index 55bd6f09c70f..6c3b0742f639 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -22,6 +22,7 @@
enum kasan_arg_mode {
KASAN_ARG_MODE_DEFAULT,
KASAN_ARG_MODE_OFF,
+ KASAN_ARG_MODE_LIGHT,
KASAN_ARG_MODE_PROD,
KASAN_ARG_MODE_FULL,
};
@@ -60,6 +61,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"))
@@ -105,9 +108,21 @@ static int __init early_kasan_fault(char *arg)
}
early_param("kasan.fault", early_kasan_fault);

+static inline int hw_init_mode(enum kasan_arg_mode mode)
+{
+ switch (mode) {
+ case KASAN_ARG_MODE_LIGHT:
+ return KASAN_HW_TAGS_ASYNC;
+ default:
+ return KASAN_HW_TAGS_SYNC;
+ }
+}
+
/* kasan_init_hw_tags_cpu() is called for each CPU. */
void kasan_init_hw_tags_cpu(void)
{
+ enum kasan_hw_tags_mode hw_mode;
+
/*
* There's no need to check that the hardware is MTE-capable here,
* as this function is only called for MTE-capable hardware.
@@ -118,7 +133,8 @@ void kasan_init_hw_tags_cpu(void)
return;

hw_init_tags(KASAN_TAG_MAX);
- hw_enable_tagging();
+ hw_mode = hw_init_mode(kasan_arg_mode);
+ hw_enable_tagging(hw_mode);
}

/* kasan_init_hw_tags() is called once on boot CPU. */
@@ -145,6 +161,7 @@ void __init kasan_init_hw_tags(void)

Vincenzo Frascino

unread,
Jan 15, 2021, 7:00:58 AM1/15/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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 after the asynchronous tag
check fault 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.
The code that verifies the status of TFSR_EL1 will be added with a
future patch.

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

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 53a6d734e29b..df7a1ae26d7c 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -153,8 +153,30 @@ void mte_init_tags(u64 max_tag)

void mte_enable_kernel(enum kasan_hw_tags_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_HW_TAGS_ASYNC:
+ /* Enable MTE Async Mode for EL1. */
+ sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_ASYNC);
+ m = "asynchronous";
+ break;
+ case KASAN_HW_TAGS_SYNC:
+ 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

Vincenzo Frascino

unread,
Jan 15, 2021, 7:00:59 AM1/15/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>
---
arch/arm64/include/asm/mte.h | 21 +++++++++++++++++++
arch/arm64/kernel/entry-common.c | 11 ++++++++++
arch/arm64/kernel/mte.c | 35 ++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+)

diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index d02aff9f493d..1a715963d909 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -92,5 +92,26 @@ static inline void mte_assign_mem_tag_range(void *addr, size_t size)

#endif /* CONFIG_ARM64_MTE */

+#ifdef CONFIG_KASAN_HW_TAGS
+void mte_check_tfsr_el1_no_sync(void);
+static inline void mte_check_tfsr_el1(void)
+{
+ mte_check_tfsr_el1_no_sync();
+ /*
+ * The asynchronous faults are synch'ed automatically with
+ * TFSR_EL1 on kernel entry but for exit an explicit dsb()
+ * is required.
+ */
+ dsb(ish);
+}
+#else
+static inline void mte_check_tfsr_el1_no_sync(void)
+{
+}
+static inline void mte_check_tfsr_el1(void)
+{
+}
+#endif /* CONFIG_KASAN_HW_TAGS */
+
#endif /* __ASSEMBLY__ */
#endif /* __ASM_MTE_H */
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 5346953e4382..c6dfe8a525b0 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_no_sync();
}

/*
@@ -47,6 +49,13 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs)
{
lockdep_assert_irqs_disabled();

+ /*
+ * The dsb() in mte_check_tfsr_el1() is required to relate
+ * the asynchronous tag check fault to the context in which
+ * it happens.
+ */
+ mte_check_tfsr_el1();
+
if (interrupts_enabled(regs)) {
if (regs->exit_rcu) {
trace_hardirqs_on_prepare();
@@ -243,6 +252,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 df7a1ae26d7c..6cb92e9d6ad1 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -180,6 +180,32 @@ void mte_enable_kernel(enum kasan_hw_tags_mode mode)
isb();
}

+#ifdef CONFIG_KASAN_HW_TAGS
+void mte_check_tfsr_el1_no_sync(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!");
+ }
+}
+#endif
+
static void update_sctlr_el1_tcf0(u64 tcf0)
{
/* ISB required for the kernel uaccess routines */
@@ -245,6 +271,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 path we rely on the dsb() present
+ * in __switch_to() to guarantee that the indirect writes to TFSR_EL1
+ * are synchronized before this point.
+ */
+ mte_check_tfsr_el1_no_sync();
}

void mte_suspend_exit(void)
--
2.30.0

Vincenzo Frascino

unread,
Jan 15, 2021, 7:01:02 AM1/15/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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+jJr...@mail.gmail.com/

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo...@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 1a715963d909..9730f2b07b79 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -49,7 +49,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.
+ *

Mark Rutland

unread,
Jan 15, 2021, 10:08:24 AM1/15/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Marco Elver, Catalin Marinas, Branislav Rankov, Alexander Potapenko, Evgenii Stepanov, Andrey Konovalov, Andrey Ryabinin, Will Deacon, Dmitry Vyukov
On Fri, Jan 15, 2021 at 12:00:40PM +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.
> In this mode, if a tag check fault occurs, the TFSR_EL1 register is
> updated asynchronously. The kernel checks the corresponding bits
> periodically.

What's the expected usage of this relative to prod, given that this has
to be chosen at boot time? When/where is this expected to be used
relative to prod mode?

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

Rather than passing a mode in, I think it'd be better to have:

* arch_enable_tagging_prod()
* arch_enable_tagging_light()

... that we can map in the arch code to separate:

* mte_enable_kernel_sync()
* mte_enable_kernel_async()

... as by construction that avoids calls with an unhandled mode, and we
wouldn't need the mode enum kasan_hw_tags_mode...

> +static inline int hw_init_mode(enum kasan_arg_mode mode)
> +{
> + switch (mode) {
> + case KASAN_ARG_MODE_LIGHT:
> + return KASAN_HW_TAGS_ASYNC;
> + default:
> + return KASAN_HW_TAGS_SYNC;
> + }
> +}

... and we can just have a wrapper like this to call either of the two functions directly, i.e.

static inline void hw_enable_tagging_mode(enum kasan_arg_mode mode)
{
if (mode == KASAN_ARG_MODE_LIGHT)
arch_enable_tagging_mode_light();
else
arch_enable_tagging_mode_prod();
}

Thanks,
Mark.

Mark Rutland

unread,
Jan 15, 2021, 10:13:35 AM1/15/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
For clarity, we should have that ISB before the pr_info_once().

As with my comment on patch 1, I think with separate functions this
would be much clearer and simpler:

static inline void __mte_enable_kernel(const char *mode, unsigned long tcf)
{
sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, tcf);
isb();

pr_info_once("MTE: enabled in %s mode at EL1\n", mode);
}

void mte_enable_kernel_sync(void)
{
__mte_enable_kernel("synchronous", SCTLR_ELx_TCF_SYNC);
}

void mte_enable_kernel_async(void)
{
__mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
}

Thanks,
Mark.

Mark Rutland

unread,
Jan 15, 2021, 10:38:05 AM1/15/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
Nit: can we please use "sync" rather than "synch", to match what we do
elsewhere, e.g. mte_check_tfsr_el1_no_sync immediately above. The
inconsistency is unfortunate and distracting.

> + * TFSR_EL1 on kernel entry but for exit an explicit dsb()
> + * is required.
> + */
> + dsb(ish);
> +}

Did you mean to have the barrier /before/ checking the TFSR? I'm
confused as to why it's after the check if the point of it is to ensure
that TFSR has been updated.

I don't understand this difference between the entry/exit paths; are you
relying on a prior DSB in the entry path?

Is the DSB alone sufficient to update the TFSR (i.e. is an indirect
write ordered before a direct read)? ... or do you need a DSB + ISB
here?

It's probably worth a comment as to why the ISH domain is correct here
rather than NSH or SY. I'm not entirely certain if ISH is necessary or
sufficient, but it depends on the completion rules.

[...]

> >
> /*
> @@ -47,6 +49,13 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs)
> {
> lockdep_assert_irqs_disabled();
>
> + /*
> + * The dsb() in mte_check_tfsr_el1() is required to relate
> + * the asynchronous tag check fault to the context in which
> + * it happens.
> + */
> + mte_check_tfsr_el1();

I think this comment is misplaced, given that mte_check_tfsr_el1() isn't
even in the same file.

If you need to do different things upon entry/exit, I'd rather we had
separate functions, e.g.

* mte_check_tfsr_entry();
* mte_check_tfsr_exit();

... since then it's immediately obvious in context as to whether we're
using the right function, and then we can have a comment within each of
the functions explaining what we need to do in that specific case.
I thing it's worth spelling out what TF0 == 1 means, e.g.

/*
* The kernel should never trigger an asynchronous fault on a
* TTBR0 address, so we should never see TF0 set.
* For futexes we disable checks via PSTATE.TCO.
*/

... what about regular uaccess using LDTR/STTR? What happens for those?

> + WARN_ON(tfsr_el1 & SYS_TFSR_EL1_TF0);

It's probably worth giving this a message so that we can debug it more
easily, e.g.

WARN(tfsr_el1 & SYS_TFSR_EL1_TF0,
"Kernel async tag fault on TTBR0 address");

> + if (tfsr_el1 & SYS_TFSR_EL1_TF1) {

It might be worth wrapping this with an unlikely(), given we hope this
never happens.

Thanks,
Mark.

Mark Rutland

unread,
Jan 15, 2021, 10:45:31 AM1/15/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
On Fri, Jan 15, 2021 at 12:00:43PM +0000, Vincenzo Frascino wrote:
> 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].

... what exactly is the optimization?

I /think/ you're just trying to have it inlined, but you should mention
that explicitly.
Is there any chance that this can be used for the last bytes of the
virtual address space? This might need to change to `_addr == _end` if
that is possible, otherwise it'll terminate early in that case.

> +}

What does the code generation look like for this, relative to the
assembly version?

Thanks,
Mark.

Andrey Konovalov

unread,
Jan 15, 2021, 1:59:26 PM1/15/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
Hi Vincenzo,

I've just mailed the change to KASAN parameters [1] as discussed, so
we should use a standalone parameter here (kasan.trap?).

Thanks!

[1] https://lkml.org/lkml/2021/1/15/1242

Vincenzo Frascino

unread,
Jan 16, 2021, 8:37:08 AM1/16/21
to Andrey Konovalov, 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/15/21 6:59 PM, Andrey Konovalov wrote:
> On Fri, Jan 15, 2021 at 1:00 PM Vincenzo Frascino
> <vincenzo...@arm.com> wrote:
>>

[...]
>> @@ -60,6 +61,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;
>
> Hi Vincenzo,
>
> I've just mailed the change to KASAN parameters [1] as discussed, so
> we should use a standalone parameter here (kasan.trap?).
>
> Thanks!
>
> [1] https://lkml.org/lkml/2021/1/15/1242
>

Thanks for this. I will have a look into it today. In the meantime, could you
please elaborate a bit more on kasan.trap?

--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 16, 2021, 8:43:23 AM1/16/21
to Mark Rutland, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Marco Elver, Catalin Marinas, Branislav Rankov, Alexander Potapenko, Evgenii Stepanov, Andrey Konovalov, Andrey Ryabinin, Will Deacon, Dmitry Vyukov
Hi Mark,

On 1/15/21 3:08 PM, Mark Rutland wrote:
> On Fri, Jan 15, 2021 at 12:00:40PM +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.
>> In this mode, if a tag check fault occurs, the TFSR_EL1 register is
>> updated asynchronously. The kernel checks the corresponding bits
>> periodically.
>
> What's the expected usage of this relative to prod, given that this has
> to be chosen at boot time? When/where is this expected to be used
> relative to prod mode?
>

IIUC the light mode is meant for low spec devices. I let Andrey comment a bit
more on this topic.
Fine by me, this would remove the need of adding a new enumeration as well and
reflect on the arch code. I would keep "arch_enable_tagging_mode_sync" and
"arch_enable_tagging_mode_async" though to give a clear indication in the KASAN
code of the mode we are setting. I will adapt my code accordingly for v4.

> Thanks,
> Mark.
>

--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 16, 2021, 8:45:31 AM1/16/21
to Mark Rutland, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
Good point, I will fix it in v4.

> As with my comment on patch 1, I think with separate functions this
> would be much clearer and simpler:
>
> static inline void __mte_enable_kernel(const char *mode, unsigned long tcf)
> {
> sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, tcf);
> isb();
>
> pr_info_once("MTE: enabled in %s mode at EL1\n", mode);
> }
>
> void mte_enable_kernel_sync(void)
> {
> __mte_enable_kernel("synchronous", SCTLR_ELx_TCF_SYNC);
> }
>
> void mte_enable_kernel_async(void)
> {
> __mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
> }
>

Ok, seems cleaner like this, will adapt my code accordingly.

Andrey Konovalov

unread,
Jan 16, 2021, 9:00:02 AM1/16/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
On Sat, Jan 16, 2021 at 2:37 PM Vincenzo Frascino
<vincenzo...@arm.com> wrote:
>
> > [1] https://lkml.org/lkml/2021/1/15/1242
> >
>
> Thanks for this. I will have a look into it today. In the meantime, could you
> please elaborate a bit more on kasan.trap?

That's what I call the boot parameter that allows switching between
sync and async. We'll need one as we're dropping
kasan.mode=off/prod/light/full.

Feel free to name it differently. Perhaps, as kasan.mode is now
unused, we can use that for sync/async.

Vincenzo Frascino

unread,
Jan 16, 2021, 9:02:53 AM1/16/21
to Andrey Konovalov, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
I see, thanks for the explanation. "mode" or "trap" would work for me.

--
Regards,
Vincenzo

Andrey Konovalov

unread,
Jan 16, 2021, 9:09:43 AM1/16/21
to Vincenzo Frascino, Mark Rutland, Linux ARM, LKML, kasan-dev, Marco Elver, Catalin Marinas, Branislav Rankov, Alexander Potapenko, Evgenii Stepanov, Andrey Ryabinin, Will Deacon, Dmitry Vyukov
On Sat, Jan 16, 2021 at 2:43 PM Vincenzo Frascino
<vincenzo...@arm.com> wrote:
> On 1/15/21 3:08 PM, Mark Rutland wrote:
> > On Fri, Jan 15, 2021 at 12:00:40PM +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.
> >> In this mode, if a tag check fault occurs, the TFSR_EL1 register is
> >> updated asynchronously. The kernel checks the corresponding bits
> >> periodically.
> >
> > What's the expected usage of this relative to prod, given that this has
> > to be chosen at boot time? When/where is this expected to be used
> > relative to prod mode?

Hi Mark,

Sync + no panic (what is called prod right now) + logging is for the
initial MTE integration stage as causing panics is risky. There's no
way to know how often MTE-detected bugs will happen during normal
usage as the kernel is buggy.

Eventually, we're hoping to switch to sync + panic to allow MTE to act
as a security mitigation. For devices where the slowdown caused by
sync is untolerable, there'll be an option to use async, which is
significantly faster. The exact perf numbers are yet to be measured
properly, I'll share them with one of the future patches.

Thanks!

Vincenzo Frascino

unread,
Jan 16, 2021, 9:18:24 AM1/16/21
to Mark Rutland, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
Hi Mark,

On 1/15/21 3:45 PM, Mark Rutland wrote:
> On Fri, Jan 15, 2021 at 12:00:43PM +0000, Vincenzo Frascino wrote:
>> 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].
>
> ... what exactly is the optimization?
>
> I /think/ you're just trying to have it inlined, but you should mention
> that explicitly.
>

Good point, I will change it in the next version. I used "Optimize" as a
continuation of the topic in the previous thread but you are right it is not
immediately obvious.
Theoretically it is a possibility. I will change the condition and add a note
for that.

>> +}
>
> What does the code generation look like for this, relative to the
> assembly version?
>

The assembly looks like this:

390: 8b000022 add x2, x1, x0
394: aa0003e1 mov x1, x0
398: d9200821 stg x1, [x1]
39c: 91004021 add x1, x1, #0x10
3a0: eb01005f cmp x2, x1
3a4: 54ffffa8 b.hi 398 <mte_set_mem_tag_range+0x48>

You can see the handcrafted one below.
--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 17, 2021, 7:23:24 AM1/17/21
to Mark Rutland, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
Hi Mark,

On 1/16/21 2:22 PM, Vincenzo Frascino wrote:
>> Is there any chance that this can be used for the last bytes of the
>> virtual address space? This might need to change to `_addr == _end` if
>> that is possible, otherwise it'll terminate early in that case.
>>
> Theoretically it is a possibility. I will change the condition and add a note
> for that.
>

I was thinking to the end of the virtual address space scenario and I forgot
that if I use a condition like `_addr == _end` the tagging operation overflows
to the first granule of the next allocation. This disrupts tagging accesses for
that memory area hence I think that `_addr < _end` is the way to go.

--
Regards,
Vincenzo

Mark Rutland

unread,
Jan 18, 2021, 5:24:55 AM1/18/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Marco Elver, Catalin Marinas, Branislav Rankov, Alexander Potapenko, Evgenii Stepanov, Andrey Konovalov, Andrey Ryabinin, Will Deacon, Dmitry Vyukov
Thanks, that sounds great!

I completely agree on keeping the '_sync' and '_aync' suffixes in the
the core code.

Mark.

Mark Rutland

unread,
Jan 18, 2021, 5:41:22 AM1/18/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
I think it implies `_addr != _end` is necessary. Otherwise, if `addr` is
PAGE_SIZE from the end of memory, and `size` is PAGE_SIZE, `_end` will
be 0, so using `_addr < _end` will mean the loop will terminate after a
single MTE tag granule rather than the whole page.

Generally, for some addr/increment/size combination (where all are
suitably aligned), you need a pattern like:

| do {
| thing(addr);
| addr += increment;
| } while (addr != end);

... or:

| for (addr = start; addr != end; addr += increment) {
| thing(addr);
| }

... to correctly handle working at the very end of the VA space.

We do similar for page tables, e.g. when we use pmd_addr_end().

Thanks,
Mark.

Vincenzo Frascino

unread,
Jan 18, 2021, 5:56:54 AM1/18/21
to Mark Rutland, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
Good point! I agree it wraps around otherwise. I will change it accordingly.

Thanks!

Catalin Marinas

unread,
Jan 18, 2021, 7:57:21 AM1/18/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
On Fri, Jan 15, 2021 at 12:00:42PM +0000, Vincenzo Frascino wrote:
> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> index d02aff9f493d..1a715963d909 100644
> --- a/arch/arm64/include/asm/mte.h
> +++ b/arch/arm64/include/asm/mte.h
> @@ -92,5 +92,26 @@ static inline void mte_assign_mem_tag_range(void *addr, size_t size)
>
> #endif /* CONFIG_ARM64_MTE */
>
> +#ifdef CONFIG_KASAN_HW_TAGS
> +void mte_check_tfsr_el1_no_sync(void);
> +static inline void mte_check_tfsr_el1(void)
> +{
> + mte_check_tfsr_el1_no_sync();
> + /*
> + * The asynchronous faults are synch'ed automatically with
> + * TFSR_EL1 on kernel entry but for exit an explicit dsb()
> + * is required.
> + */
> + dsb(ish);
> +}

Mark commented already, the barrier should be above
mte_check_tfsr_el1_no_sync(). Regarding the ISB, we are waiting for
confirmation from the architects.

> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index df7a1ae26d7c..6cb92e9d6ad1 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -180,6 +180,32 @@ void mte_enable_kernel(enum kasan_hw_tags_mode mode)
> isb();
> }
>
> +#ifdef CONFIG_KASAN_HW_TAGS
> +void mte_check_tfsr_el1_no_sync(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);

I'd change this to a WARN_ON_ONCE() in case we trip over this due to
model bugs etc. and it floods the log.

> + if (tfsr_el1 & SYS_TFSR_EL1_TF1) {
> + write_sysreg_s(0, SYS_TFSR_EL1);
> + isb();

While in general we use ISB after a sysreg update, I haven't convinced
myself it's needed here. There's no side-effect to updating this reg and
a subsequent TFSR access should see the new value. If a speculated load
is allowed to update this reg, we'd probably need an ISB+DSB (I don't
think it does, something to check with the architects).

> +
> + pr_err("MTE: Asynchronous tag exception detected!");

We discussed this already, I think we should replace this pr_err() with
a call to kasan_report(). In principle, kasan already knows the mode as
it asked for sync/async but we could make this explicit and expand the
kasan API to take some argument (or have separate function like
kasan_report_async()).

--
Catalin

Vincenzo Frascino

unread,
Jan 18, 2021, 8:33:50 AM1/18/21
to Catalin Marinas, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov


On 1/18/21 12:57 PM, Catalin Marinas wrote:
>> +#ifdef CONFIG_KASAN_HW_TAGS
>> +void mte_check_tfsr_el1_no_sync(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);
> I'd change this to a WARN_ON_ONCE() in case we trip over this due to
> model bugs etc. and it floods the log.
>

I will merge yours and Mark's comment using WARN_ONCE() here. Did not think of
potential bug in the model and you are completely right.

>> + if (tfsr_el1 & SYS_TFSR_EL1_TF1) {
>> + write_sysreg_s(0, SYS_TFSR_EL1);
>> + isb();
> While in general we use ISB after a sysreg update, I haven't convinced
> myself it's needed here. There's no side-effect to updating this reg and
> a subsequent TFSR access should see the new value.

Why there is no side-effect?

> If a speculated load is allowed to update this reg, we'd probably need an
> ISB+DSB (I don't think it does, something to check with the architects).
>

I will check this with the architects and let you know.

--
Regards,
Vincenzo

Mark Rutland

unread,
Jan 18, 2021, 9:14:37 AM1/18/21
to Vincenzo Frascino, Catalin Marinas, Branislav Rankov, Marco Elver, Andrey Konovalov, Evgenii Stepanov, linux-...@vger.kernel.org, kasa...@googlegroups.com, Alexander Potapenko, linux-ar...@lists.infradead.org, Andrey Ryabinin, Will Deacon, Dmitry Vyukov
On Mon, Jan 18, 2021 at 01:37:35PM +0000, Vincenzo Frascino wrote:
> On 1/18/21 12:57 PM, Catalin Marinas wrote:

> >> + if (tfsr_el1 & SYS_TFSR_EL1_TF1) {
> >> + write_sysreg_s(0, SYS_TFSR_EL1);
> >> + isb();
> > While in general we use ISB after a sysreg update, I haven't convinced
> > myself it's needed here. There's no side-effect to updating this reg and
> > a subsequent TFSR access should see the new value.
>
> Why there is no side-effect?

Catalin's saying that the value of TFSR_EL1 doesn't affect anything
other than a read of TFSR_EL1, i.e. there are no indirect reads of
TFSR_EL1 where the value has an effect, so there are no side-effects.

Looking at the ARM ARM, no synchronization is requires from a direct
write to an indirect write (per ARM DDI 0487F.c table D13-1), so I agree
that we don't need the ISB here so long as there are no indirect reads.

Are you aware of cases where the TFSR_EL1 value is read other than by an
MRS? e.g. are there any cases where checks are elided if TF1 is set? If
so, we may need the ISB to order the direct write against subsequent
indirect reads.

Thanks,
Mark.

Vincenzo Frascino

unread,
Jan 18, 2021, 9:45:07 AM1/18/21
to Mark Rutland, Catalin Marinas, Branislav Rankov, Marco Elver, Andrey Konovalov, Evgenii Stepanov, linux-...@vger.kernel.org, kasa...@googlegroups.com, Alexander Potapenko, linux-ar...@lists.infradead.org, Andrey Ryabinin, Will Deacon, Dmitry Vyukov
Hi Mark,
Thank you for the explanation. I am not aware of any case in which TFSR_EL1 is
read other then by an MRS. Based on the ARM DDI 0487F.c (J1-7626) TF0/TF1 are
always set to '1' without being accessed before. I will check with the
architects for further clarification and if this is correct I will remove the
isb() in the next version.

Vincenzo Frascino

unread,
Jan 18, 2021, 10:35:59 AM1/18/21
to Mark Rutland, Catalin Marinas, Branislav Rankov, Marco Elver, Andrey Konovalov, Evgenii Stepanov, linux-...@vger.kernel.org, kasa...@googlegroups.com, Alexander Potapenko, linux-ar...@lists.infradead.org, Andrey Ryabinin, Will Deacon, Dmitry Vyukov


On 1/18/21 2:48 PM, Vincenzo Frascino wrote:
>> Are you aware of cases where the TFSR_EL1 value is read other than by an
>> MRS? e.g. are there any cases where checks are elided if TF1 is set? If
>> so, we may need the ISB to order the direct write against subsequent
>> indirect reads.
>>
> Thank you for the explanation. I am not aware of any case in which TFSR_EL1 is
> read other then by an MRS. Based on the ARM DDI 0487F.c (J1-7626) TF0/TF1 are
> always set to '1' without being accessed before. I will check with the
> architects for further clarification and if this is correct I will remove the
> isb() in the next version.
>

I spoke to the architects and I confirm that the isb() can be removed.

--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 18, 2021, 10:37:11 AM1/18/21
to Catalin Marinas, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov

On 1/18/21 1:37 PM, Vincenzo Frascino wrote:
>> If a speculated load is allowed to update this reg, we'd probably need an
>> ISB+DSB (I don't think it does, something to check with the architects).
>>
> I will check this with the architects and let you know.

I spoke to the architects and no speculative load can update TFSR_EL1.

--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 18, 2021, 1:30:49 PM1/18/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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-rc3.

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:
--------
v4:
- Added support for kasan.mode (sync/async) kernel
command line parameter.
- Addressed review comments.
v3:
- Exposed kasan_hw_tags_mode to convert the internal
KASAN represenetation.
- Added dsb() for kernel exit paths in arm64.
- Addressed review comments.
v2:
- Fixed a compilation issue reported by krobot.
- General cleanup.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Marco Elver <el...@google.com>
Cc: Evgenii Stepanov <eug...@google.com>
Cc: Branislav Rankov <Branisla...@arm.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>

Vincenzo Frascino (5):
arm64: mte: Add asynchronous mode support
kasan: Add KASAN mode kernel parameter
kasan: Add report for async mode
arm64: mte: Enable async tag check fault
arm64: mte: Inline mte_assign_mem_tag_range()

Documentation/dev-tools/kasan.rst | 3 ++
arch/arm64/include/asm/memory.h | 3 +-
arch/arm64/include/asm/mte-kasan.h | 9 ++++-
arch/arm64/include/asm/mte.h | 58 ++++++++++++++++++++++++++-
arch/arm64/kernel/entry-common.c | 6 +++
arch/arm64/kernel/mte.c | 63 +++++++++++++++++++++++++++++-
arch/arm64/lib/mte.S | 15 -------
include/linux/kasan.h | 3 ++
mm/kasan/hw_tags.c | 31 ++++++++++++++-
mm/kasan/kasan.h | 3 +-
mm/kasan/report.c | 16 +++++++-
11 files changed, 185 insertions(+), 25 deletions(-)

--
2.30.0

Vincenzo Frascino

unread,
Jan 18, 2021, 1:30:50 PM1/18/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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 after the asynchronous tag
check fault 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.
The code that verifies the status of TFSR_EL1 will be added with a
future patch.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>
---
arch/arm64/include/asm/memory.h | 3 ++-
arch/arm64/include/asm/mte-kasan.h | 9 +++++++--
arch/arm64/kernel/mte.c | 16 ++++++++++++++--
3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 18fce223b67b..233d9feec45c 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -231,7 +231,8 @@ 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_sync() mte_enable_kernel_sync()
+#define arch_enable_tagging_async() mte_enable_kernel_async()
#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..9a5e30dbe12a 100644
--- a/arch/arm64/include/asm/mte-kasan.h
+++ b/arch/arm64/include/asm/mte-kasan.h
@@ -29,7 +29,8 @@ 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_sync(void);
+void mte_enable_kernel_async(void);
void mte_init_tags(u64 max_tag);

#else /* CONFIG_ARM64_MTE */
@@ -52,7 +53,11 @@ 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_sync(void)
+{
+}
+
+static inline void mte_enable_kernel_sync(void)
{
}

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index dc9ada64feed..78fc079a3b1e 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -151,11 +151,23 @@ 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)
+static inline void __mte_enable_kernel(const char *mode, unsigned long tcf)
{
/* Enable MTE Sync Mode for EL1. */
- sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
+ sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, tcf);
isb();
+
+ pr_info_once("MTE: enabled in %s mode at EL1\n", mode);
+}
+
+void mte_enable_kernel_sync(void)
+{
+ __mte_enable_kernel("synchronous", SCTLR_ELx_TCF_SYNC);
+}
+
+void mte_enable_kernel_async(void)
+{
+ __mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
}

static void update_sctlr_el1_tcf0(u64 tcf0)
--
2.30.0

Vincenzo Frascino

unread,
Jan 18, 2021, 1:30:52 PM1/18/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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 sync or async mode of
execution. On an MTE enabled arm64 hw for example this can be identified
with the synchronous or asynchronous tagging mode of execution.
In synchronous mode, an exception is triggered if a tag check fault occurs.
In asynchronous mode, if a tag check fault occurs, the TFSR_EL1 register is
updated asynchronously. The kernel checks the corresponding bits
periodically.

KASAN requires a specific kernel command line parameter to make use of this
hw features.

Add KASAN HW execution mode kernel command line parameter.

Note: This patch adds the kasan.mode kernel parameter and the
sync/async kernel command line options to enable the described features.

Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>
---
Documentation/dev-tools/kasan.rst | 3 +++
mm/kasan/hw_tags.c | 31 ++++++++++++++++++++++++++++++-
mm/kasan/kasan.h | 3 ++-
3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index 1651d961f06a..60ad73c2a33c 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -162,6 +162,9 @@ particular KASAN features.

- ``kasan=off`` or ``=on`` controls whether KASAN is enabled (default: ``on``).

+- ``kasan.mode=sync`` or ``=async`` controls whether KASAN is configured in
+ synchronous or asynchronous mode of execution (default: ``sync``).
+
- ``kasan.stacktrace=off`` or ``=on`` disables or enables alloc and free stack
traces collection (default: ``on`` for ``CONFIG_DEBUG_KERNEL=y``, otherwise
``off``).
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index e529428e7a11..344aeec05d43 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -25,6 +25,11 @@ enum kasan_arg {
KASAN_ARG_ON,
};

+enum kasan_arg_mode {
+ KASAN_ARG_MODE_SYNC,
+ KASAN_ARG_MODE_ASYNC,
+};
+
enum kasan_arg_stacktrace {
KASAN_ARG_STACKTRACE_DEFAULT,
KASAN_ARG_STACKTRACE_OFF,
@@ -38,6 +43,7 @@ enum kasan_arg_fault {
};

static enum kasan_arg kasan_arg __ro_after_init;
+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;

@@ -68,6 +74,21 @@ static int __init early_kasan_flag(char *arg)
}
early_param("kasan", early_kasan_flag);

+/* kasan.mode=sync/async */
+static int __init early_kasan_mode(char *arg)
+{
+ /* If arg is not set the default mode is sync */
+ if ((!arg) || !strcmp(arg, "sync"))
+ kasan_arg_mode = KASAN_ARG_MODE_SYNC;
+ else if (!strcmp(arg, "async"))
+ kasan_arg_mode = KASAN_ARG_MODE_ASYNC;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+early_param("kasan.mode", early_kasan_mode);
+
/* kasan.stacktrace=off/on */
static int __init early_kasan_flag_stacktrace(char *arg)
{
@@ -102,6 +123,14 @@ static int __init early_kasan_fault(char *arg)
}
early_param("kasan.fault", early_kasan_fault);

+static inline void hw_enable_tagging_mode(void)
+{
+ if (kasan_arg_mode == KASAN_ARG_MODE_ASYNC)
+ hw_enable_tagging_async();
+ else
+ hw_enable_tagging_sync();
+}
+
/* kasan_init_hw_tags_cpu() is called for each CPU. */
void kasan_init_hw_tags_cpu(void)
{
@@ -115,7 +144,7 @@ void kasan_init_hw_tags_cpu(void)
return;

hw_init_tags(KASAN_TAG_MAX);
- hw_enable_tagging();
+ hw_enable_tagging_mode();
}

/* kasan_init_hw_tags() is called once on boot CPU. */
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index cc4d9e1d49b1..7db7bd42fe97 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -284,7 +284,8 @@ 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_sync() arch_enable_tagging_sync()
+#define hw_enable_tagging_async() arch_enable_tagging_async()
#define hw_init_tags(max_tag) arch_init_tags(max_tag)
#define hw_get_random_tag() arch_get_random_tag()
#define hw_get_mem_tag(addr) arch_get_mem_tag(addr)
--
2.30.0

Vincenzo Frascino

unread,
Jan 18, 2021, 1:30:55 PM1/18/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
KASAN provides an asynchronous mode of execution.

Add reporting functionality for this mode.

Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>
---
include/linux/kasan.h | 3 +++
mm/kasan/report.c | 16 ++++++++++++++--
2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index fe1ae73ff8b5..8f43836ccdac 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -336,6 +336,9 @@ static inline void *kasan_reset_tag(const void *addr)
bool kasan_report(unsigned long addr, size_t size,
bool is_write, unsigned long ip);

+bool kasan_report_async(unsigned long addr, size_t size,
+ bool is_write, unsigned long ip);
+
#else /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */

static inline void *kasan_reset_tag(const void *addr)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index c0fb21797550..946016ead6a9 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -388,11 +388,11 @@ static void __kasan_report(unsigned long addr, size_t size, bool is_write,
start_report(&flags);

print_error_description(&info);
- if (addr_has_metadata(untagged_addr))
+ if (addr_has_metadata(untagged_addr) && (untagged_addr != 0))
print_tags(get_tag(tagged_addr), info.first_bad_addr);
pr_err("\n");

- if (addr_has_metadata(untagged_addr)) {
+ if (addr_has_metadata(untagged_addr) && (untagged_addr != 0)) {
print_address_description(untagged_addr, get_tag(tagged_addr));
pr_err("\n");
print_memory_metadata(info.first_bad_addr);
@@ -419,6 +419,18 @@ bool kasan_report(unsigned long addr, size_t size, bool is_write,
return ret;
}

+bool kasan_report_async(unsigned long addr, size_t size,
+ bool is_write, unsigned long ip)
+{
+ pr_info("==================================================================\n");
+ pr_info("KASAN: set in asynchronous mode\n");
+ pr_info("KASAN: some information might not be accurate\n");
+ pr_info("KASAN: fault address is ignored\n");
+ pr_info("KASAN: write/read distinction is ignored\n");
+
+ return kasan_report(addr, size, is_write, ip);
+}
+
#ifdef CONFIG_KASAN_INLINE
/*
* With CONFIG_KASAN_INLINE, accesses to bogus pointers (outside the high
--
2.30.0

Vincenzo Frascino

unread,
Jan 18, 2021, 1:30:56 PM1/18/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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().
The dsb(nsh) in mte_check_tfsr_exit() is provisional pending
confirmation by the architects.

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

diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index d02aff9f493d..237bb2f7309d 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -92,5 +92,37 @@ static inline void mte_assign_mem_tag_range(void *addr, size_t size)

#endif /* CONFIG_ARM64_MTE */

+#ifdef CONFIG_KASAN_HW_TAGS
+void mte_check_tfsr_el1(void);
+
+static inline void mte_check_tfsr_entry(void)
+{
+ mte_check_tfsr_el1();
+}
+
+static inline void mte_check_tfsr_exit(void)
+{
+ /*
+ * The asynchronous faults are sync'ed automatically with
+ * TFSR_EL1 on kernel entry but for exit an explicit dsb()
+ * is required.
+ */
+ dsb(nsh);
+ isb();
+
+ mte_check_tfsr_el1();
+}
+#else
+static inline void mte_check_tfsr_el1(void)
+{
+}
+static inline void mte_check_tfsr_entry(void)
+{
+}
+static inline void mte_check_tfsr_exit(void)
+{
+}
+#endif /* CONFIG_KASAN_HW_TAGS */
+
#endif /* __ASSEMBLY__ */
#endif /* __ASM_MTE_H */
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 5346953e4382..31666511ba67 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_entry();
}

/*
@@ -47,6 +49,8 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs)
{
lockdep_assert_irqs_disabled();

+ mte_check_tfsr_exit();
+
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_exit();
+
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 78fc079a3b1e..0a9cc82a5301 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -170,6 +170,44 @@ void mte_enable_kernel_async(void)
__mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
}

+#ifdef CONFIG_KASAN_HW_TAGS
+static inline void mte_report_async(void)
+{
+ u64 pc = (u64)__builtin_return_address(0);
+
+ kasan_report_async(0, 0, false, pc);
+}
+
+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 trigger an asynchronous fault on a
+ * TTBR0 address, so we should never see TF0 set.
+ * For futexes we disable checks via PSTATE.TCO.
+ */
+ WARN_ONCE(tfsr_el1 & SYS_TFSR_EL1_TF0,
+ "Kernel async tag fault on TTBR0 address");
+
+ if (unlikely(tfsr_el1 & SYS_TFSR_EL1_TF1)) {
+ /*
+ * Note: isb() is not required after this direct write
+ * because there is no indirect read subsequent to it
+ * (per ARM DDI 0487F.c table D13-1).
+ */
+ write_sysreg_s(0, SYS_TFSR_EL1);
+
+ mte_report_async();
+ }
+}
+#endif
+
static void update_sctlr_el1_tcf0(u64 tcf0)
{
/* ISB required for the kernel uaccess routines */
@@ -235,6 +273,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 path we rely on the dsb() present
+ * in __switch_to() to guarantee that the indirect writes to TFSR_EL1
+ * are synchronized before this point.
+ */
+ mte_check_tfsr_el1();
}

void mte_suspend_exit(void)
--
2.30.0

Vincenzo Frascino

unread,
Jan 18, 2021, 1:30:58 PM1/18/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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 inline it in an attempt to reduce the
overhead.

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

[1] https://lore.kernel.org/r/CAAeHK+wCO+J7D1_T89DG+jJr...@mail.gmail.com/

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo...@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 237bb2f7309d..1a6fd53f82c3 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -49,7 +49,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);
+}

Catalin Marinas

unread,
Jan 19, 2021, 7:57:38 AM1/19/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
On Mon, Jan 18, 2021 at 06:30:29PM +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 after the asynchronous tag
> check fault 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.
> The code that verifies the status of TFSR_EL1 will be added with a
> future patch.
>
> Cc: Catalin Marinas <catalin...@arm.com>
> Cc: Will Deacon <wi...@kernel.org>
> Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>

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

Catalin Marinas

unread,
Jan 19, 2021, 8:04:46 AM1/19/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
On Mon, Jan 18, 2021 at 06:30:31PM +0000, Vincenzo Frascino wrote:
> KASAN provides an asynchronous mode of execution.
>
> Add reporting functionality for this mode.
>
> Cc: Dmitry Vyukov <dvy...@google.com>
> Cc: Andrey Ryabinin <arya...@virtuozzo.com>
> Cc: Alexander Potapenko <gli...@google.com>
> Cc: Andrey Konovalov <andre...@google.com>
> Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>
> ---
> include/linux/kasan.h | 3 +++
> mm/kasan/report.c | 16 ++++++++++++++--
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index fe1ae73ff8b5..8f43836ccdac 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -336,6 +336,9 @@ static inline void *kasan_reset_tag(const void *addr)
> bool kasan_report(unsigned long addr, size_t size,
> bool is_write, unsigned long ip);
>
> +bool kasan_report_async(unsigned long addr, size_t size,
> + bool is_write, unsigned long ip);

We have no address, no size and no is_write information. Do we have a
reason to pass all these arguments here? Not sure what SPARC ADI does
but they may not have all this information either. We can pass ip as the
point where we checked the TFSR reg but that's about it.
So just call kasan_report (0, 0, 0, ip) here.

--
Catalin

Vincenzo Frascino

unread,
Jan 19, 2021, 9:19:17 AM1/19/21
to Catalin Marinas, Andrey Konovalov, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
I kept the interface generic for future development and mainly to start a
discussion. I do not have a strong opinion either way. If Andrey agrees as well
I am happy to change it to what you are suggesting in v5.
Fine by me.

--
Regards,
Vincenzo

Catalin Marinas

unread,
Jan 19, 2021, 9:34:19 AM1/19/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
On Mon, Jan 18, 2021 at 06:30:32PM +0000, Vincenzo Frascino wrote:
> static void update_sctlr_el1_tcf0(u64 tcf0)
> {
> /* ISB required for the kernel uaccess routines */
> @@ -235,6 +273,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 path we rely on the dsb() present
> + * in __switch_to() to guarantee that the indirect writes to TFSR_EL1
> + * are synchronized before this point.
> + */
> + mte_check_tfsr_el1();
> }

We need an isb() before mte_check_tfsr_el1() here as well, we only have
a dsb() in __switch_to(). We do have an isb() in update_sctlr_el1_tcf0()
but only if the check passed. Now, it's worth benchmarking how expensive
update_sctlr_el1_tcf0() is (i.e. an SCTLR_EL1 access + isb with
something like hackbench) and we could probably remove the check
altogether. In the meantime, you can add an isb() on the "else" path of
the above check.

--
Catalin

Vincenzo Frascino

unread,
Jan 19, 2021, 9:42:08 AM1/19/21
to Catalin Marinas, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
Good catch, I saw the isb() in update_sctlr_el1_tcf0() and for some reasons that
it is not escaping me I thought it was sufficient, but clearly it is not.

I am happy to benchmark what you are suggesting and provide some data after this
series is merged (if it works for you) so that we can decide. In the meantime as
you suggested I will fix the "else" for v5.

--
Regards,
Vincenzo

Catalin Marinas

unread,
Jan 19, 2021, 9:45:05 AM1/19/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
While I'm ok with moving this function to C, I don't think it solves the
inlining in the kasan code. The only interface we have to kasan is via
mte_{set,get}_mem_tag_range(), so the above function doesn't need to
live in a header.

If you do want inlining all the way to the kasan code, we should
probably move the mte_{set,get}_mem_tag_range() functions to the header
as well (and ideally backed by some numbers to show that it matters).

Moving it to mte.c also gives us more control on how it's called (we
have the WARN_ONs in place in the callers).

--
Catalin

Mark Rutland

unread,
Jan 19, 2021, 9:46:32 AM1/19/21
to Vincenzo Frascino, Catalin Marinas, Andrey Konovalov, Branislav Rankov, Marco Elver, Evgenii Stepanov, linux-...@vger.kernel.org, kasa...@googlegroups.com, Alexander Potapenko, linux-ar...@lists.infradead.org, Andrey Ryabinin, Will Deacon, Dmitry Vyukov
On Tue, Jan 19, 2021 at 02:23:03PM +0000, Vincenzo Frascino wrote:
> On 1/19/21 1:04 PM, Catalin Marinas wrote:
> > On Mon, Jan 18, 2021 at 06:30:31PM +0000, Vincenzo Frascino wrote:

> >> +bool kasan_report_async(unsigned long addr, size_t size,
> >> + bool is_write, unsigned long ip);
> >
> > We have no address, no size and no is_write information. Do we have a
> > reason to pass all these arguments here? Not sure what SPARC ADI does
> > but they may not have all this information either. We can pass ip as the
> > point where we checked the TFSR reg but that's about it.
>
> I kept the interface generic for future development and mainly to start a
> discussion. I do not have a strong opinion either way. If Andrey agrees as well
> I am happy to change it to what you are suggesting in v5.

For now, I think it's preferable that this only has parameters that we
can actually provide. That way it's clearer what's going on in both
callers and callees, and we can always rework the prototype later or add
separate variants of the function that can take additional parameters.

I don't think we even need to use __kasan_report() -- more on that
below.

[...]
Given there's no information available, I think it's simpler and
preferable to handle the logging separately, as is done for
kasan_report_invalid_free(). For example, we could do something roughly
like:

void kasan_report_async(void)
{
unsigned long flags;

start_report(&flags);
pr_err("BUG: KASAN: Tag mismatch detected asynchronously\n");
pr_err("KASAN: no fault information available\n");
dump_stack();
end_report(&flags);
}

... which is easier to consume, since there's no misleading output,
avoids complicating the synchronous reporting path, and we could
consider adding information that's only of use for debugging
asynchronous faults here.

Since the callside is logged in the backtrace, we don't even need the
synthetic IP parameter.

Thanks,
Mark.

Vincenzo Frascino

unread,
Jan 19, 2021, 10:01:59 AM1/19/21
to Mark Rutland, Catalin Marinas, Andrey Konovalov, Branislav Rankov, Marco Elver, Evgenii Stepanov, linux-...@vger.kernel.org, kasa...@googlegroups.com, Alexander Potapenko, linux-ar...@lists.infradead.org, Andrey Ryabinin, Will Deacon, Dmitry Vyukov
Agree, especially because I tend to not like to rely on compiler builtins and
what you proposed solves the problem ;)

I will refactor my code once Andrey had a chance to take a look as well.

Vincenzo Frascino

unread,
Jan 19, 2021, 10:45:11 AM1/19/21
to Catalin Marinas, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
Hi Catalin,
Based on the thread [1] this patch contains only an intermediate step to allow
KASAN to call directly mte_assign_mem_tag_range() in future. At that point I
think that mte_set_mem_tag_range() can be removed.

If you agree, I would live the things like this to give to Andrey a chance to
execute on the original plan with a separate series.

I agree though that this change alone does not bring huge benefits but
regressions neither.

If you want I can add something to the commit message in the next version to
make this more explicit.

Let me know how do you want me to proceed.

--
Regards,
Vincenzo

Andrey Konovalov

unread,
Jan 19, 2021, 1:10:11 PM1/19/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
Hi Vincenzo,

This change has multiple conflicts with the KASAN testing patches that
are currently in the mm tree. If Andrew decides to send all of them
during RC, then this should be good to go through arm64. Otherwise, I
guess this will need to go through mm as well. So you probably need to
rebase this on top of those patches in any case.

Thanks!

Andrey Konovalov

unread,
Jan 19, 2021, 1:10:23 PM1/19/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
On Mon, Jan 18, 2021 at 7:30 PM Vincenzo Frascino
<vincenzo...@arm.com> wrote:
>
Reviewed-by: Andrey Konovalov <andre...@google.com>

Andrey Konovalov

unread,
Jan 19, 2021, 1:10:32 PM1/19/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
On Mon, Jan 18, 2021 at 7:30 PM Vincenzo Frascino
<vincenzo...@arm.com> wrote:
> --- a/Documentation/dev-tools/kasan.rst
> +++ b/Documentation/dev-tools/kasan.rst
> @@ -162,6 +162,9 @@ particular KASAN features.
>
> - ``kasan=off`` or ``=on`` controls whether KASAN is enabled (default: ``on``).
>
> +- ``kasan.mode=sync`` or ``=async`` controls whether KASAN is configured in
> + synchronous or asynchronous mode of execution (default: ``sync``).

This needs to be expanded with a short explanation of the difference.

> +static inline void hw_enable_tagging_mode(void)
> +{
> + if (kasan_arg_mode == KASAN_ARG_MODE_ASYNC)
> + hw_enable_tagging_async();
> + else
> + hw_enable_tagging_sync();
> +}

It's OK to open-code this in kasan_init_hw_tags_cpu(), no need for an
additional function.

> --- a/mm/kasan/kasan.h
> +++ b/mm/kasan/kasan.h
> @@ -284,7 +284,8 @@ 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_sync() arch_enable_tagging_sync()
> +#define hw_enable_tagging_async() arch_enable_tagging_async()

This is one of the places that conflicts with the testing patches.
You'll need to: add an else case definition of
hw_enable_tagging_sync(); change lib/test_kasan.c to use
hw_enable_tagging_sync().

I'll later add a patch on top that forbids running the tests with the
async mode.

Andrey Konovalov

unread,
Jan 19, 2021, 1:12:24 PM1/19/21
to Mark Rutland, Vincenzo Frascino, Catalin Marinas, Branislav Rankov, Marco Elver, Evgenii Stepanov, LKML, kasan-dev, Alexander Potapenko, Linux ARM, Andrey Ryabinin, Will Deacon, Dmitry Vyukov
On Tue, Jan 19, 2021 at 3:46 PM Mark Rutland <mark.r...@arm.com> wrote:
>
> Given there's no information available, I think it's simpler and
> preferable to handle the logging separately, as is done for
> kasan_report_invalid_free(). For example, we could do something roughly
> like:
>
> void kasan_report_async(void)
> {
> unsigned long flags;
>
> start_report(&flags);
> pr_err("BUG: KASAN: Tag mismatch detected asynchronously\n");

"BUG: KASAN: invalid-access"

It also might make sense to pass the ip, even though it's not exactly
related to the access:

pr_err("BUG: KASAN: invalid-access in %pS\n", (void *)ip);

Up to you.

> pr_err("KASAN: no fault information available\n");

pr_err("Asynchronous mode enabled: no access details available\n");

> dump_stack();
> end_report(&flags);
> }

This approach with a dedicated function is better. Thanks, Mark!

Please put it next to kasan_report_invalid_free().

Andrey Konovalov

unread,
Jan 19, 2021, 1:12:54 PM1/19/21
to Vincenzo Frascino, Catalin Marinas, Linux ARM, LKML, kasan-dev, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
I think we should drop this patch from this series as it's unrelated.

I will pick it up into my future optimization series. Then it will be
easier to discuss it in the context. The important part that I needed
is an inlinable C implementation of mte_assign_mem_tag_range(), which
I now have with this patch.

Thanks, Vincenzo!

Catalin Marinas

unread,
Jan 19, 2021, 2:00:44 PM1/19/21
to Andrey Konovalov, Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
That's fine by me but we may want to add some forced-alignment on the
addr and size as the loop here depends on them being aligned, otherwise
it gets stuck. The mte_set_mem_tag_range() at least had a WARN_ON in
place. Here we could do:

addr &= MTE_GRANULE_MASK;
size = ALIGN(size, MTE_GRANULE_SIZE);

(or maybe trim "size" with MTE_GRANULE_MASK)

That's unless the call places are well known and guarantee this
alignment (only had a very brief look).

--
Catalin

Andrey Konovalov

unread,
Jan 19, 2021, 2:34:56 PM1/19/21
to Catalin Marinas, Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
No problem. I'll either add the ALIGN or change the call site to
ensure alignment.

Vincenzo Frascino

unread,
Jan 20, 2021, 9:41:20 AM1/20/21
to Andrey Konovalov, 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/19/21 6:10 PM, Andrey Konovalov wrote:
> On Mon, Jan 18, 2021 at 7:30 PM Vincenzo Frascino
> <vincenzo...@arm.com> wrote:
>> --- a/Documentation/dev-tools/kasan.rst
>> +++ b/Documentation/dev-tools/kasan.rst
>> @@ -162,6 +162,9 @@ particular KASAN features.
>>
>> - ``kasan=off`` or ``=on`` controls whether KASAN is enabled (default: ``on``).
>>
>> +- ``kasan.mode=sync`` or ``=async`` controls whether KASAN is configured in
>> + synchronous or asynchronous mode of execution (default: ``sync``).
> This needs to be expanded with a short explanation of the difference.
>

Ok, I will extend it in v5.

>> +static inline void hw_enable_tagging_mode(void)
>> +{
>> + if (kasan_arg_mode == KASAN_ARG_MODE_ASYNC)
>> + hw_enable_tagging_async();
>> + else
>> + hw_enable_tagging_sync();
>> +}
> It's OK to open-code this in kasan_init_hw_tags_cpu(), no need for an
> additional function.
>

I added the new function to keep the code cleaner, but I do not have strong
opinion hence it is fine by me to have open-code here.

--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 20, 2021, 9:42:57 AM1/20/21
to Andrey Konovalov, Mark Rutland, Catalin Marinas, Branislav Rankov, Marco Elver, Evgenii Stepanov, LKML, kasan-dev, Alexander Potapenko, Linux ARM, Andrey Ryabinin, Will Deacon, Dmitry Vyukov

On 1/19/21 6:12 PM, Andrey Konovalov wrote:
> On Tue, Jan 19, 2021 at 3:46 PM Mark Rutland <mark.r...@arm.com> wrote:
>>
>> Given there's no information available, I think it's simpler and
>> preferable to handle the logging separately, as is done for
>> kasan_report_invalid_free(). For example, we could do something roughly
>> like:
>>
>> void kasan_report_async(void)
>> {
>> unsigned long flags;
>>
>> start_report(&flags);
>> pr_err("BUG: KASAN: Tag mismatch detected asynchronously\n");
>
> "BUG: KASAN: invalid-access"
>

Ok, I will do in v5. It looks more uniform with what we have for the sync exception.

> It also might make sense to pass the ip, even though it's not exactly
> related to the access:
>

I would like to avoid to add a builtin for something that has not a real meaning
as you are correctly pointing out.

> pr_err("BUG: KASAN: invalid-access in %pS\n", (void *)ip);
>
> Up to you.
>
>> pr_err("KASAN: no fault information available\n");
>
> pr_err("Asynchronous mode enabled: no access details available\n");
>
>> dump_stack();
>> end_report(&flags);
>> }
>
> This approach with a dedicated function is better. Thanks, Mark!
>
> Please put it next to kasan_report_invalid_free().
>

Will do in v5.

Thanks!

--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 21, 2021, 6:31:45 AM1/21/21
to Andrey Konovalov, 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/19/21 6:09 PM, Andrey Konovalov wrote:
> Hi Vincenzo,
>
> This change has multiple conflicts with the KASAN testing patches that
> are currently in the mm tree. If Andrew decides to send all of them
> during RC, then this should be good to go through arm64. Otherwise, I
> guess this will need to go through mm as well. So you probably need to
> rebase this on top of those patches in any case.
>

Could you please let me know on which tree do you want me to rebase my patches?
I almost completed the requested changes.

Thank you!

> Thanks!

--
Regards,
Vincenzo

Andrey Konovalov

unread,
Jan 21, 2021, 7:25:54 AM1/21/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
linux-next/akpm should work. Thanks!

Vincenzo Frascino

unread,
Jan 21, 2021, 11:39:53 AM1/21/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
The series is based on linux-next/akpm.

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

Changes:
--------
v5:
- Rebase the series on linux-next/akpm.
- Forbid execution for KASAN KUNIT tests when async
mode is enabled.
- Dropped patch to inline mte_assign_mem_tag_range().
- Address review comments.
v4:
- Added support for kasan.mode (sync/async) kernel
command line parameter.
- Addressed review comments.
v3:
- Exposed kasan_hw_tags_mode to convert the internal
KASAN represenetation.
- Added dsb() for kernel exit paths in arm64.
- Addressed review comments.
v2:
- Fixed a compilation issue reported by krobot.
- General cleanup.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Marco Elver <el...@google.com>
Cc: Evgenii Stepanov <eug...@google.com>
Cc: Branislav Rankov <Branisla...@arm.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>

Vincenzo Frascino (6):
arm64: mte: Add asynchronous mode support
kasan: Add KASAN mode kernel parameter
kasan: Add report for async mode
arm64: mte: Enable async tag check fault
arm64: mte: Expose execution mode
kasan: Forbid kunit tests when async mode is enabled

Documentation/dev-tools/kasan.rst | 7 +++
arch/arm64/include/asm/memory.h | 4 +-
arch/arm64/include/asm/mte-kasan.h | 15 ++++++-
arch/arm64/include/asm/mte.h | 32 ++++++++++++++
arch/arm64/kernel/entry-common.c | 6 +++
arch/arm64/kernel/mte.c | 68 +++++++++++++++++++++++++++++-
include/linux/kasan.h | 2 +
lib/test_kasan.c | 7 ++-
mm/kasan/hw_tags.c | 27 +++++++++++-
mm/kasan/kasan.h | 8 +++-
mm/kasan/report.c | 11 +++++
11 files changed, 178 insertions(+), 9 deletions(-)

--
2.30.0

Vincenzo Frascino

unread,
Jan 21, 2021, 11:39:57 AM1/21/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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 after the asynchronous tag
check fault 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.
The code that verifies the status of TFSR_EL1 will be added with a
future patch.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Reviewed-by: Catalin Marinas <catalin...@arm.com>
Reviewed-by: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>
---
arch/arm64/include/asm/memory.h | 3 ++-
arch/arm64/include/asm/mte-kasan.h | 9 +++++++--
arch/arm64/kernel/mte.c | 16 ++++++++++++++--
3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index cedfc9e97bcc..df96b9c10b81 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -231,7 +231,8 @@ 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_sync() mte_enable_kernel_sync()
+#define arch_enable_tagging_async() mte_enable_kernel_async()
#define arch_set_tagging_report_once(state) mte_set_report_once(state)
#define arch_init_tags(max_tag) mte_init_tags(max_tag)
#define arch_get_random_tag() mte_get_random_tag()
diff --git a/arch/arm64/include/asm/mte-kasan.h b/arch/arm64/include/asm/mte-kasan.h
index 3748d5bb88c0..76b6a5988ce5 100644
--- a/arch/arm64/include/asm/mte-kasan.h
+++ b/arch/arm64/include/asm/mte-kasan.h
@@ -29,7 +29,8 @@ 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_sync(void);
+void mte_enable_kernel_async(void);
void mte_init_tags(u64 max_tag);

void mte_set_report_once(bool state);
@@ -55,7 +56,11 @@ 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_sync(void)
+{
+}
+
+static inline void mte_enable_kernel_sync(void)
{
}

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index c63b3d7a3cd9..92078e1eb627 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -153,11 +153,23 @@ 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)
+static inline void __mte_enable_kernel(const char *mode, unsigned long tcf)
{
/* Enable MTE Sync Mode for EL1. */
- sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
+ sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, tcf);
isb();
+
+ pr_info_once("MTE: enabled in %s mode at EL1\n", mode);
+}
+
+void mte_enable_kernel_sync(void)
+{
+ __mte_enable_kernel("synchronous", SCTLR_ELx_TCF_SYNC);
+}
+
+void mte_enable_kernel_async(void)
+{
+ __mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
}

void mte_set_report_once(bool state)
--
2.30.0

Vincenzo Frascino

unread,
Jan 21, 2021, 11:39:57 AM1/21/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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_TAGS can provide a sync or async mode
of execution. On an MTE enabled arm64 hw for example this can be identified
with the synchronous or asynchronous tagging mode of execution.
In synchronous mode, an exception is triggered if a tag check fault occurs.
In asynchronous mode, if a tag check fault occurs, the TFSR_EL1 register is
updated asynchronously. The kernel checks the corresponding bits
periodically.

KASAN requires a specific kernel command line parameter to make use of this
hw features.

Add KASAN HW execution mode kernel command line parameter.

Note: This patch adds the kasan.mode kernel parameter and the
sync/async kernel command line options to enable the described features.

Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>
---
Documentation/dev-tools/kasan.rst | 7 +++++++
lib/test_kasan.c | 2 +-
mm/kasan/hw_tags.c | 27 ++++++++++++++++++++++++++-
mm/kasan/kasan.h | 6 ++++--
4 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index e022b7506e37..7e4a6e0c9f57 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -161,6 +161,13 @@ particular KASAN features.

- ``kasan=off`` or ``=on`` controls whether KASAN is enabled (default: ``on``).

+- ``kasan.mode=sync`` or ``=async`` controls whether KASAN is configured in
+ synchronous or asynchronous mode of execution (default: ``sync``).
+ ``synchronous mode``: an exception is triggered if a tag check fault occurs.
+ ``asynchronous mode``: if a tag check fault occurs, the information is stored
+ asynchronously in hardware (e.g. in the TFSR_EL1 register for arm64). The kernel
+ checks the hardware location and reports an error if the fault is detected.
+
- ``kasan.stacktrace=off`` or ``=on`` disables or enables alloc and free stack
traces collection (default: ``on`` for ``CONFIG_DEBUG_KERNEL=y``, otherwise
``off``).
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index d16ec9e66806..7285dcf9fcc1 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -97,7 +97,7 @@ static void kasan_test_exit(struct kunit *test)
READ_ONCE(fail_data.report_found)); \
if (IS_ENABLED(CONFIG_KASAN_HW_TAGS)) { \
if (READ_ONCE(fail_data.report_found)) \
- hw_enable_tagging(); \
+ hw_enable_tagging_sync(); \
migrate_enable(); \
} \
} while (0)
diff --git a/mm/kasan/hw_tags.c b/mm/kasan/hw_tags.c
index e529428e7a11..224a2187839c 100644
--- a/mm/kasan/hw_tags.c
+++ b/mm/kasan/hw_tags.c
@@ -25,6 +25,11 @@ enum kasan_arg {
KASAN_ARG_ON,
};

+enum kasan_arg_mode {
+ KASAN_ARG_MODE_SYNC,
+ KASAN_ARG_MODE_ASYNC,
+};
+
enum kasan_arg_stacktrace {
KASAN_ARG_STACKTRACE_DEFAULT,
KASAN_ARG_STACKTRACE_OFF,
@@ -38,6 +43,7 @@ enum kasan_arg_fault {
};

static enum kasan_arg kasan_arg __ro_after_init;
+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;

@@ -68,6 +74,21 @@ static int __init early_kasan_flag(char *arg)
}
early_param("kasan", early_kasan_flag);

+/* kasan.mode=sync/async */
+static int __init early_kasan_mode(char *arg)
+{
+ /* If arg is not set the default mode is sync */
+ if ((!arg) || !strcmp(arg, "sync"))
+ kasan_arg_mode = KASAN_ARG_MODE_SYNC;
+ else if (!strcmp(arg, "async"))
+ kasan_arg_mode = KASAN_ARG_MODE_ASYNC;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+early_param("kasan.mode", early_kasan_mode);
+
/* kasan.stacktrace=off/on */
static int __init early_kasan_flag_stacktrace(char *arg)
{
@@ -115,7 +136,11 @@ void kasan_init_hw_tags_cpu(void)
return;

hw_init_tags(KASAN_TAG_MAX);
- hw_enable_tagging();
+
+ if (kasan_arg_mode == KASAN_ARG_MODE_ASYNC)
+ hw_enable_tagging_async();
+ else
+ hw_enable_tagging_sync();
}

/* kasan_init_hw_tags() is called once on boot CPU. */
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 07ef7fc742ad..3923d9744105 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -294,7 +294,8 @@ 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_sync() arch_enable_tagging_sync()
+#define hw_enable_tagging_async() arch_enable_tagging_async()
#define hw_init_tags(max_tag) arch_init_tags(max_tag)
#define hw_set_tagging_report_once(state) arch_set_tagging_report_once(state)
#define hw_get_random_tag() arch_get_random_tag()
@@ -303,7 +304,8 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)

#else /* CONFIG_KASAN_HW_TAGS */

-#define hw_enable_tagging()
+#define hw_enable_tagging_sync()
+#define hw_enable_tagging_async()
#define hw_set_tagging_report_once(state)

#endif /* CONFIG_KASAN_HW_TAGS */
--
2.30.0

Vincenzo Frascino

unread,
Jan 21, 2021, 11:39:59 AM1/21/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
KASAN provides an asynchronous mode of execution.

Add reporting functionality for this mode.

Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>
---
include/linux/kasan.h | 2 ++
mm/kasan/report.c | 11 +++++++++++
2 files changed, 13 insertions(+)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index bb862d1f0e15..b0a1d9dfa85c 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -351,6 +351,8 @@ static inline void *kasan_reset_tag(const void *addr)
bool kasan_report(unsigned long addr, size_t size,
bool is_write, unsigned long ip);

+void kasan_report_async(void);
+
#else /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */

static inline void *kasan_reset_tag(const void *addr)
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 234f35a84f19..2fd6845a95e9 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -358,6 +358,17 @@ void kasan_report_invalid_free(void *object, unsigned long ip)
end_report(&flags);
}

+void kasan_report_async(void)
+{
+ unsigned long flags;
+
+ start_report(&flags);
+ pr_err("BUG: KASAN: invalid-access\n");
+ pr_err("Asynchronous mode enabled: no access details available\n");
+ dump_stack();
+ end_report(&flags);
+}
+
static void __kasan_report(unsigned long addr, size_t size, bool is_write,
unsigned long ip)
{
--
2.30.0

Vincenzo Frascino

unread,
Jan 21, 2021, 11:40:01 AM1/21/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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().
The dsb(nsh) in mte_check_tfsr_exit() is provisional pending
confirmation by the architects.

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

diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
index d02aff9f493d..237bb2f7309d 100644
--- a/arch/arm64/include/asm/mte.h
+++ b/arch/arm64/include/asm/mte.h
@@ -92,5 +92,37 @@ static inline void mte_assign_mem_tag_range(void *addr, size_t size)

#endif /* CONFIG_ARM64_MTE */

+#ifdef CONFIG_KASAN_HW_TAGS
+void mte_check_tfsr_el1(void);
+
+static inline void mte_check_tfsr_entry(void)
+{
+ mte_check_tfsr_el1();
+}
+
+static inline void mte_check_tfsr_exit(void)
+{
+ /*
+ * The asynchronous faults are sync'ed automatically with
+ * TFSR_EL1 on kernel entry but for exit an explicit dsb()
+ * is required.
+ */
+ dsb(nsh);
+ isb();
+
+ mte_check_tfsr_el1();
+}
+#else
+static inline void mte_check_tfsr_el1(void)
+{
+}
+static inline void mte_check_tfsr_entry(void)
+{
+}
+static inline void mte_check_tfsr_exit(void)
+{
+}
+#endif /* CONFIG_KASAN_HW_TAGS */
+
#endif /* __ASSEMBLY__ */
#endif /* __ASM_MTE_H */
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 5346953e4382..31666511ba67 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_entry();
}

/*
@@ -47,6 +49,8 @@ static void noinstr exit_to_kernel_mode(struct pt_regs *regs)
{
lockdep_assert_irqs_disabled();

+ mte_check_tfsr_exit();
+
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_exit();
+
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 92078e1eb627..7763ac1f2917 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -182,6 +182,37 @@ bool mte_report_once(void)
return READ_ONCE(report_fault_once);
}

+#ifdef CONFIG_KASAN_HW_TAGS
+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 trigger an asynchronous fault on a
+ * TTBR0 address, so we should never see TF0 set.
+ * For futexes we disable checks via PSTATE.TCO.
+ */
+ WARN_ONCE(tfsr_el1 & SYS_TFSR_EL1_TF0,
+ "Kernel async tag fault on TTBR0 address");
+
+ if (unlikely(tfsr_el1 & SYS_TFSR_EL1_TF1)) {
+ /*
+ * Note: isb() is not required after this direct write
+ * because there is no indirect read subsequent to it
+ * (per ARM DDI 0487F.c table D13-1).
+ */
+ write_sysreg_s(0, SYS_TFSR_EL1);
+
+ kasan_report_async();
+ }
+}
+#endif
+
static void update_sctlr_el1_tcf0(u64 tcf0)
{
/* ISB required for the kernel uaccess routines */
@@ -247,6 +278,19 @@ 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);
+ else
+ isb();
+
+ /*
+ * Check if an async tag exception occurred at EL1.
+ *
+ * Note: On the context switch path we rely on the dsb() present
+ * in __switch_to() to guarantee that the indirect writes to TFSR_EL1
+ * are synchronized before this point.
+ * isb() above is required for the same reason.
+ *
+ */
+ mte_check_tfsr_el1();
}

void mte_suspend_exit(void)
--
2.30.0

Vincenzo Frascino

unread,
Jan 21, 2021, 11:40:02 AM1/21/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
MTE enabled arm64 HW can be configured in synchronous or asynchronous
tagging mode of execution.
In synchronous mode, an exception is triggered if a tag check fault
occurs.
In asynchronous mode, if a tag check fault occurs, the TFSR_EL1 register
is updated asynchronously. The kernel checks the corresponding bits
periodically.

Introduce an API that exposes the mode of execution to the kernel.

Note: This API will be used by KASAN KUNIT tests to forbid the execution
when async mode is enable.

Cc: Catalin Marinas <catalin...@arm.com>
Cc: Will Deacon <wi...@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>
---
arch/arm64/include/asm/memory.h | 1 +
arch/arm64/include/asm/mte-kasan.h | 6 ++++++
arch/arm64/kernel/mte.c | 8 ++++++++
3 files changed, 15 insertions(+)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index df96b9c10b81..1d4eef519fa6 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -233,6 +233,7 @@ static inline const void *__tag_set(const void *addr, u8 tag)
#ifdef CONFIG_KASAN_HW_TAGS
#define arch_enable_tagging_sync() mte_enable_kernel_sync()
#define arch_enable_tagging_async() mte_enable_kernel_async()
+#define arch_is_mode_sync() mte_is_mode_sync()
#define arch_set_tagging_report_once(state) mte_set_report_once(state)
#define arch_init_tags(max_tag) mte_init_tags(max_tag)
#define arch_get_random_tag() mte_get_random_tag()
diff --git a/arch/arm64/include/asm/mte-kasan.h b/arch/arm64/include/asm/mte-kasan.h
index 76b6a5988ce5..c216160e805c 100644
--- a/arch/arm64/include/asm/mte-kasan.h
+++ b/arch/arm64/include/asm/mte-kasan.h
@@ -31,6 +31,7 @@ void *mte_set_mem_tag_range(void *addr, size_t size, u8 tag);

void mte_enable_kernel_sync(void);
void mte_enable_kernel_async(void);
+bool mte_is_mode_sync(void);
void mte_init_tags(u64 max_tag);

void mte_set_report_once(bool state);
@@ -64,6 +65,11 @@ static inline void mte_enable_kernel_sync(void)
{
}

+static inline bool mte_is_mode_sync(void)
+{
+ return false;
+}
+
static inline void mte_init_tags(u64 max_tag)
{
}
diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 7763ac1f2917..1cc3fc173b97 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c
@@ -26,6 +26,7 @@
u64 gcr_kernel_excl __ro_after_init;

static bool report_fault_once = true;
+static bool __mte_mode_sync = true;

static void mte_sync_page_tags(struct page *page, pte_t *ptep, bool check_swap)
{
@@ -169,9 +170,16 @@ void mte_enable_kernel_sync(void)

void mte_enable_kernel_async(void)
{
+ __mte_mode_sync = false;
+
__mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC);
}

+bool mte_is_mode_sync(void)
+{
+ return __mte_mode_sync;
+}
+
void mte_set_report_once(bool state)
{
WRITE_ONCE(report_fault_once, state);
--
2.30.0

Vincenzo Frascino

unread,
Jan 21, 2021, 11:40:05 AM1/21/21
to linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, 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_TAGS can provide a sync or async
mode of execution. KASAN KUNIT tests can be executed only when sync
mode is enabled.

Forbid the execution of the KASAN KUNIT tests when async mode is
enabled.

Cc: Dmitry Vyukov <dvy...@google.com>
Cc: Andrey Ryabinin <arya...@virtuozzo.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@google.com>
Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>
---
lib/test_kasan.c | 5 +++++
mm/kasan/kasan.h | 2 ++
2 files changed, 7 insertions(+)

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 7285dcf9fcc1..1306f707b4fe 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -52,6 +52,11 @@ static int kasan_test_init(struct kunit *test)
return -1;
}

+ if (!hw_is_mode_sync()) {
+ kunit_err(test, "can't run KASAN tests in async mode");
+ return -1;
+ }
+
multishot = kasan_save_enable_multi_shot();
hw_set_tagging_report_once(false);
return 0;
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 3923d9744105..3464113042ab 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -296,6 +296,7 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)

#define hw_enable_tagging_sync() arch_enable_tagging_sync()
#define hw_enable_tagging_async() arch_enable_tagging_async()
+#define hw_is_mode_sync() arch_is_mode_sync()
#define hw_init_tags(max_tag) arch_init_tags(max_tag)
#define hw_set_tagging_report_once(state) arch_set_tagging_report_once(state)
#define hw_get_random_tag() arch_get_random_tag()
@@ -306,6 +307,7 @@ static inline const void *arch_kasan_set_tag(const void *addr, u8 tag)

#define hw_enable_tagging_sync()
#define hw_enable_tagging_async()
+#define hw_is_mode_sync()

Vincenzo Frascino

unread,
Jan 21, 2021, 11:41:44 AM1/21/21
to Andrey Konovalov, 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/19/21 6:10 PM, Andrey Konovalov wrote:
> I'll later add a patch on top that forbids running the tests with the
> async mode.

Sorry, I misread this part, I thought you wanted me to do this. Anyway I added
the check to my last series.

Please have a look.

--
Regards,
Vincenzo

Andrey Konovalov

unread,
Jan 21, 2021, 12:34:13 PM1/21/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
Synchronous mode: a bad access is detected immediately when a tag
check fault occurs.

(No need for `` here, "synchronous mode" is not an inline snippet.)

> + ``asynchronous mode``: if a tag check fault occurs, the information is stored
> + asynchronously in hardware (e.g. in the TFSR_EL1 register for arm64). The kernel
> + checks the hardware location and reports an error if the fault is detected.

Asynchronous mode: a bad access detection is delayed. When a tag check
fault occurs, the information is stored in hardware (in the TFSR_EL1
register for arm64). The kernel periodically checks the hardware and
only reports tag faults during these checks.
For other modes I explicitly added a _DEFAULT option first. It makes
sense to do this here as well for consistency.
Let's add a comment:

/* Enable async mode only when explicitly requested through the command line. */

Andrey Konovalov

unread,
Jan 21, 2021, 12:36:32 PM1/21/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
On Thu, Jan 21, 2021 at 5:39 PM Vincenzo Frascino
<vincenzo...@arm.com> wrote:
>
Reviewed-by: Andrey Konovalov <andre...@google.com>

FTR: this will conflict with the Alex's patch:

https://lore.kernel.org/linux-api/20210121131915....@google.com/T/#m8872c56af85babfc08784e2b2fcd5cc1c0c73859

Andrey Konovalov

unread,
Jan 21, 2021, 12:38:18 PM1/21/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
Do we need a static bool reported like in do_tag_recovery() here?

Andrey Konovalov

unread,
Jan 21, 2021, 12:40:48 PM1/21/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
On Thu, Jan 21, 2021 at 5:40 PM Vincenzo Frascino
<vincenzo...@arm.com> wrote:
>
I'd rather implement this check at the KASAN level, than in arm64
code. Just the way kasan_stack_collection_enabled() is implemented.

Feel free to drop this change and the previous patch, I'll implement
this myself later.

Andrey Konovalov

unread,
Jan 21, 2021, 12:41:20 PM1/21/21
to Vincenzo Frascino, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov
On Thu, Jan 21, 2021 at 5:40 PM Vincenzo Frascino
<vincenzo...@arm.com> wrote:
>
(See my comment on patch #6.)

kernel test robot

unread,
Jan 21, 2021, 9:46:09 PM1/21/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, kbuil...@lists.01.org, clang-bu...@googlegroups.com, Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver
Hi Vincenzo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on next-20210121]
[cannot apply to arm64/for-next/core arm/for-next soc/for-next xlnx/master kvmarm/next linus/master hnaz-linux-mm/master v5.11-rc4 v5.11-rc3 v5.11-rc2 v5.11-rc4]
[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/20210122-004631
base: bc085f8fc88fc16796c9f2364e2bfb3fef305cad
config: riscv-randconfig-r003-20210122 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bd3a387ee76f58caa0d7901f3f84e9bb3d006f27)
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
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/5d51fa880ab55b639b377b24bfe0b8ef6560c14c
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/20210122-004631
git checkout 5d51fa880ab55b639b377b24bfe0b8ef6560c14c
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv

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

All warnings (new ones prefixed by >>):

>> mm/kasan/report.c:361:6: warning: no previous prototype for function 'kasan_report_async' [-Wmissing-prototypes]
void kasan_report_async(void)
^
mm/kasan/report.c:361:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void kasan_report_async(void)
^
static
1 warning generated.


vim +/kasan_report_async +361 mm/kasan/report.c

360
> 361 void kasan_report_async(void)
362 {
363 unsigned long flags;
364
365 start_report(&flags);
366 pr_err("BUG: KASAN: invalid-access\n");
367 pr_err("Asynchronous mode enabled: no access details available\n");
368 dump_stack();
369 end_report(&flags);
370 }
371

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

kernel test robot

unread,
Jan 21, 2021, 9:47:05 PM1/21/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, kbuil...@lists.01.org, Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver
Hi Vincenzo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on next-20210121]
[cannot apply to arm64/for-next/core arm/for-next soc/for-next xlnx/master kvmarm/next linus/master hnaz-linux-mm/master v5.11-rc4 v5.11-rc3 v5.11-rc2 v5.11-rc4]
[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/20210122-004631
base: bc085f8fc88fc16796c9f2364e2bfb3fef305cad
config: x86_64-randconfig-s022-20210122 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-208-g46a52ca4-dirty
# https://github.com/0day-ci/linux/commit/5d51fa880ab55b639b377b24bfe0b8ef6560c14c
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/20210122-004631
git checkout 5d51fa880ab55b639b377b24bfe0b8ef6560c14c
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64

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

All warnings (new ones prefixed by >>):

>> mm/kasan/report.c:361:6: warning: no previous prototype for 'kasan_report_async' [-Wmissing-prototypes]
361 | void kasan_report_async(void)
| ^~~~~~~~~~~~~~~~~~
.config.gz

kernel test robot

unread,
Jan 21, 2021, 11:05:12 PM1/21/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, kbuil...@lists.01.org, clang-bu...@googlegroups.com, Vincenzo Frascino, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver
Hi Vincenzo,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20210121]
[cannot apply to arm64/for-next/core arm/for-next soc/for-next xlnx/master kvmarm/next linus/master hnaz-linux-mm/master v5.11-rc4 v5.11-rc3 v5.11-rc2 v5.11-rc4]
[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/20210122-004631
base: bc085f8fc88fc16796c9f2364e2bfb3fef305cad
config: riscv-randconfig-r003-20210122 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bd3a387ee76f58caa0d7901f3f84e9bb3d006f27)
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
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/0ac23ec8a70b5fd68efdec0a6a501bdccddf4d5e
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/20210122-004631
git checkout 0ac23ec8a70b5fd68efdec0a6a501bdccddf4d5e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv

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

All errors (new ones prefixed by >>):

>> lib/test_kasan.c:55:24: error: expected expression
if (!hw_is_mode_sync()) {
^
1 error generated.


vim +55 lib/test_kasan.c

41
42 /*
43 * Temporarily enable multi-shot mode. Otherwise, KASAN would only report the
44 * first detected bug and panic the kernel if panic_on_warn is enabled. For
45 * hardware tag-based KASAN also allow tag checking to be reenabled for each
46 * test, see the comment for KUNIT_EXPECT_KASAN_FAIL().
47 */
48 static int kasan_test_init(struct kunit *test)
49 {
50 if (!kasan_enabled()) {
51 kunit_err(test, "can't run KASAN tests with KASAN disabled");
52 return -1;
53 }
54
> 55 if (!hw_is_mode_sync()) {
56 kunit_err(test, "can't run KASAN tests in async mode");
57 return -1;
58 }
59
60 multishot = kasan_save_enable_multi_shot();
61 hw_set_tagging_report_once(false);
62 return 0;
63 }
64
.config.gz

Vincenzo Frascino

unread,
Jan 22, 2021, 6:20:40 AM1/22/21
to Andrey Konovalov, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov


On 1/21/21 5:38 PM, Andrey Konovalov wrote:
>> + if (unlikely(tfsr_el1 & SYS_TFSR_EL1_TF1)) {
>> + /*
>> + * Note: isb() is not required after this direct write
>> + * because there is no indirect read subsequent to it
>> + * (per ARM DDI 0487F.c table D13-1).
>> + */
>> + write_sysreg_s(0, SYS_TFSR_EL1);
>> +
>> + kasan_report_async();
> Do we need a static bool reported like in do_tag_recovery() here?
>

I would say not because async mode does not get disabled after the first fault.

--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 22, 2021, 6:22:01 AM1/22/21
to Andrey Konovalov, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov


On 1/21/21 5:34 PM, Andrey Konovalov wrote:
>> +- ``kasan.mode=sync`` or ``=async`` controls whether KASAN is configured in
>> + synchronous or asynchronous mode of execution (default: ``sync``).
>> + ``synchronous mode``: an exception is triggered if a tag check fault occurs.
> Synchronous mode: a bad access is detected immediately when a tag
> check fault occurs.
>
> (No need for `` here, "synchronous mode" is not an inline snippet.)
>

Ok will do in v5.

>> + ``asynchronous mode``: if a tag check fault occurs, the information is stored
>> + asynchronously in hardware (e.g. in the TFSR_EL1 register for arm64). The kernel
>> + checks the hardware location and reports an error if the fault is detected.
> Asynchronous mode: a bad access detection is delayed. When a tag check
> fault occurs, the information is stored in hardware (in the TFSR_EL1
> register for arm64). The kernel periodically checks the hardware and
> only reports tag faults during these checks.
>

Will do in v5.
Will do in v5.
Will do in v5.

--
Regards,
Vincenzo

Vincenzo Frascino

unread,
Jan 22, 2021, 6:23:04 AM1/22/21
to Andrey Konovalov, Linux ARM, LKML, kasan-dev, Catalin Marinas, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov

On 1/21/21 5:40 PM, Andrey Konovalov wrote:
>> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
>> index 7285dcf9fcc1..1306f707b4fe 100644
>> --- a/lib/test_kasan.c
>> +++ b/lib/test_kasan.c
>> @@ -52,6 +52,11 @@ static int kasan_test_init(struct kunit *test)
>> return -1;
>> }
>>
>> + if (!hw_is_mode_sync()) {
>> + kunit_err(test, "can't run KASAN tests in async mode");
>> + return -1;
>> + }
> I'd rather implement this check at the KASAN level, than in arm64
> code. Just the way kasan_stack_collection_enabled() is implemented.
>
> Feel free to drop this change and the previous patch, I'll implement
> this myself later.
>

Fine by me, will drop 5 and 6 in v5.

--
Regards,
Vincenzo

Catalin Marinas

unread,
Jan 22, 2021, 6:58:24 AM1/22/21
to Vincenzo Frascino, linux-ar...@lists.infradead.org, linux-...@vger.kernel.org, kasa...@googlegroups.com, Will Deacon, Dmitry Vyukov, Andrey Ryabinin, Alexander Potapenko, Marco Elver, Evgenii Stepanov, Branislav Rankov, Andrey Konovalov
On Thu, Jan 21, 2021 at 04:39:41PM +0000, Vincenzo Frascino wrote:
> 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().
> The dsb(nsh) in mte_check_tfsr_exit() is provisional pending
> confirmation by the architects.
>
> Cc: Catalin Marinas <catalin...@arm.com>
> Cc: Will Deacon <wi...@kernel.org>
> Signed-off-by: Vincenzo Frascino <vincenzo...@arm.com>

Reviewed-by: Catalin Marinas <catalin...@arm.com>
It is loading more messages.
0 new messages