diff mbox series

[mm,v11,27/42] arm64: mte: Add in-kernel tag fault handler

Message ID ad31529b073e22840b7a2246172c2b67747ed7c4.1606161801.git.andreyknvl@google.com (mailing list archive)
State New, archived
Headers show
Series kasan: add hardware tag-based mode for arm64 | expand

Commit Message

Andrey Konovalov Nov. 23, 2020, 8:07 p.m. UTC 
From: Vincenzo Frascino <vincenzo.frascino@arm.com>

Add the implementation of the in-kernel fault handler.

When a tag fault happens on a kernel address:
* MTE is disabled on the current CPU,
* the execution continues.

When a tag fault happens on a user address:
* the kernel executes do_bad_area() and panics.

The tag fault handler for kernel addresses is currently empty and will be
filled in by a future commit.

Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Co-developed-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
Change-Id: I9b8aa79567f7c45f4d6a1290efcf34567e620717
---
 arch/arm64/include/asm/uaccess.h | 23 ++++++++++++++++
 arch/arm64/mm/fault.c            | 45 ++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

Comments

Catalin Marinas Dec. 3, 2020, 10:26 a.m. UTC | #1 
On Mon, Nov 23, 2020 at 09:07:51PM +0100, Andrey Konovalov wrote:
> diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
> index 385a189f7d39..d841a560fae7 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -200,13 +200,36 @@ do {									\
>  				CONFIG_ARM64_PAN));			\
>  } while (0)
>  
> +/*
> + * The Tag Check Flag (TCF) mode for MTE is per EL, hence TCF0
> + * affects EL0 and TCF affects EL1 irrespective of which TTBR is
> + * used.
> + * The kernel accesses TTBR0 usually with LDTR/STTR instructions
> + * when UAO is available, so these would act as EL0 accesses using
> + * TCF0.
> + * However futex.h code uses exclusives which would be executed as
> + * EL1, this can potentially cause a tag check fault even if the
> + * user disables TCF0.
> + *
> + * To address the problem we set the PSTATE.TCO bit in uaccess_enable()
> + * and reset it in uaccess_disable().
> + *
> + * The Tag check override (TCO) bit disables temporarily the tag checking
> + * preventing the issue.
> + */
>  static inline void uaccess_disable(void)
>  {
> +	asm volatile(ALTERNATIVE("nop", SET_PSTATE_TCO(0),
> +				 ARM64_MTE, CONFIG_KASAN_HW_TAGS));
> +
>  	__uaccess_disable(ARM64_HAS_PAN);
>  }
>  
>  static inline void uaccess_enable(void)
>  {
> +	asm volatile(ALTERNATIVE("nop", SET_PSTATE_TCO(1),
> +				 ARM64_MTE, CONFIG_KASAN_HW_TAGS));
> +
>  	__uaccess_enable(ARM64_HAS_PAN);
>  }

I think that's insufficient if CONFIG_ARM64_PAN is disabled. In the !PAN
case, the get/put_user() accessors use standard LDR/STR instructions
which would follow the TCF rather than TCF0 mode checking. However, they
don't use the above uaccess_disable/enable() functions.

The current user space support is affected as well but luckily we just
skip tag checking on the uaccess routines if !PAN since the kernel TCF
is 0. With the in-kernel MTE, TCF may be more strict than TCF0.

My suggestion is to simply make CONFIG_ARM64_MTE depend on (or select)
PAN. Architecturally this should work since PAN is required for ARMv8.1,
so present with any MTE implementation. This patch is on top of -next,
though it has a Fixes tag in 5.10:

--------------------------8<---------------------------
From ecc819804c1fb1ad498d7ced07e01e3b3e055a3f Mon Sep 17 00:00:00 2001
From: Catalin Marinas <catalin.marinas@arm.com>
Date: Thu, 3 Dec 2020 10:15:39 +0000
Subject: [PATCH] arm64: mte: Ensure CONFIG_ARM64_PAN is enabled with MTE

The uaccess routines like get/put_user() rely on the user TCF0 mode
setting for tag checking. However, if CONFIG_ARM64_PAN is disabled,
these routines would use the standard LDR/STR instructions and therefore
the kernel TCF mode. In 5.10, the kernel TCF==0, so no tag checking, but
this will change with the in-kernel MTE support.

Make ARM64_MTE depend on ARM64_PAN.

Fixes: 89b94df9dfb1 ("arm64: mte: Kconfig entry")
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 844d62df776c..f9eed3a5917e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1673,6 +1673,8 @@ config ARM64_MTE
 	default y
 	depends on ARM64_AS_HAS_MTE && ARM64_TAGGED_ADDR_ABI
 	depends on AS_HAS_ARMV8_5
+	# Required for tag checking in the uaccess routines
+	depends on ARM64_PAN
 	select ARCH_USES_HIGH_VMA_FLAGS
 	help
 	  Memory Tagging (part of the ARMv8.5 Extensions) provides
Vincenzo Frascino Dec. 3, 2020, 10:39 a.m. UTC | #2 
On 12/3/20 10:26 AM, Catalin Marinas wrote:
>>  static inline void uaccess_enable(void)
>>  {
>> +	asm volatile(ALTERNATIVE("nop", SET_PSTATE_TCO(1),
>> +				 ARM64_MTE, CONFIG_KASAN_HW_TAGS));
>> +
>>  	__uaccess_enable(ARM64_HAS_PAN);
>>  }
> 
> I think that's insufficient if CONFIG_ARM64_PAN is disabled. In the !PAN
> case, the get/put_user() accessors use standard LDR/STR instructions
> which would follow the TCF rather than TCF0 mode checking. However, they
> don't use the above uaccess_disable/enable() functions.
> 
> The current user space support is affected as well but luckily we just
> skip tag checking on the uaccess routines if !PAN since the kernel TCF
> is 0. With the in-kernel MTE, TCF may be more strict than TCF0.
> 
> My suggestion is to simply make CONFIG_ARM64_MTE depend on (or select)
> PAN. Architecturally this should work since PAN is required for ARMv8.1,
> so present with any MTE implementation. This patch is on top of -next,
> though it has a Fixes tag in 5.10:
> 

Agreed, since PAN is required for ARMv8.1 we should not find any implementation
of MTE that lacks PAN.

> --------------------------8<---------------------------
> From ecc819804c1fb1ad498d7ced07e01e3b3e055a3f Mon Sep 17 00:00:00 2001
> From: Catalin Marinas <catalin.marinas@arm.com>
> Date: Thu, 3 Dec 2020 10:15:39 +0000
> Subject: [PATCH] arm64: mte: Ensure CONFIG_ARM64_PAN is enabled with MTE
> 
> The uaccess routines like get/put_user() rely on the user TCF0 mode
> setting for tag checking. However, if CONFIG_ARM64_PAN is disabled,
> these routines would use the standard LDR/STR instructions and therefore
> the kernel TCF mode. In 5.10, the kernel TCF==0, so no tag checking, but
> this will change with the in-kernel MTE support.
> 
> Make ARM64_MTE depend on ARM64_PAN.
> 
> Fixes: 89b94df9dfb1 ("arm64: mte: Kconfig entry")
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>

Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> ---
>  arch/arm64/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> =======================================
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 844d62df776c..f9eed3a5917e 100644
> 
> --- a/arch/arm64/Kconfig
> 
> +++ b/arch/arm64/Kconfig
> 
> @@ -1673,6 +1673,8 @@
> 
>  config ARM64_MTE
> 
> » default·y
> » depends·on·ARM64_AS_HAS_MTE·&&·ARM64_TAGGED_ADDR_ABI
> » depends·on·AS_HAS_ARMV8_5
> +» #·Required·for·tag·checking·in·the·uaccess·routines
> +» depends·on·ARM64_PAN
> » select·ARCH_USES_HIGH_VMA_FLAGS
> » help
> » ··Memory·Tagging·(part·of·the·ARMv8.5·Extensions)·provides
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 385a189f7d39..d841a560fae7 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -200,13 +200,36 @@  do {									\
 				CONFIG_ARM64_PAN));			\
 } while (0)
 
+/*
+ * The Tag Check Flag (TCF) mode for MTE is per EL, hence TCF0
+ * affects EL0 and TCF affects EL1 irrespective of which TTBR is
+ * used.
+ * The kernel accesses TTBR0 usually with LDTR/STTR instructions
+ * when UAO is available, so these would act as EL0 accesses using
+ * TCF0.
+ * However futex.h code uses exclusives which would be executed as
+ * EL1, this can potentially cause a tag check fault even if the
+ * user disables TCF0.
+ *
+ * To address the problem we set the PSTATE.TCO bit in uaccess_enable()
+ * and reset it in uaccess_disable().
+ *
+ * The Tag check override (TCO) bit disables temporarily the tag checking
+ * preventing the issue.
+ */
 static inline void uaccess_disable(void)
 {
+	asm volatile(ALTERNATIVE("nop", SET_PSTATE_TCO(0),
+				 ARM64_MTE, CONFIG_KASAN_HW_TAGS));
+
 	__uaccess_disable(ARM64_HAS_PAN);
 }
 
 static inline void uaccess_enable(void)
 {
+	asm volatile(ALTERNATIVE("nop", SET_PSTATE_TCO(1),
+				 ARM64_MTE, CONFIG_KASAN_HW_TAGS));
+
 	__uaccess_enable(ARM64_HAS_PAN);
 }
 
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 183d1e6dd9e0..1e4b9353c68a 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -34,6 +34,7 @@ 
 #include <asm/debug-monitors.h>
 #include <asm/esr.h>
 #include <asm/kprobes.h>
+#include <asm/mte.h>
 #include <asm/processor.h>
 #include <asm/sysreg.h>
 #include <asm/system_misc.h>
@@ -297,6 +298,44 @@  static void die_kernel_fault(const char *msg, unsigned long addr,
 	do_exit(SIGKILL);
 }
 
+static void report_tag_fault(unsigned long addr, unsigned int esr,
+			     struct pt_regs *regs)
+{
+}
+
+static void do_tag_recovery(unsigned long addr, unsigned int esr,
+			   struct pt_regs *regs)
+{
+	static bool reported;
+
+	if (!READ_ONCE(reported)) {
+		report_tag_fault(addr, esr, regs);
+		WRITE_ONCE(reported, true);
+	}
+
+	/*
+	 * Disable MTE Tag Checking on the local CPU for the current EL.
+	 * It will be done lazily on the other CPUs when they will hit a
+	 * tag fault.
+	 */
+	sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_NONE);
+	isb();
+}
+
+static bool is_el1_mte_sync_tag_check_fault(unsigned int esr)
+{
+	unsigned int ec = ESR_ELx_EC(esr);
+	unsigned int fsc = esr & ESR_ELx_FSC;
+
+	if (ec != ESR_ELx_EC_DABT_CUR)
+		return false;
+
+	if (fsc == ESR_ELx_FSC_MTE)
+		return true;
+
+	return false;
+}
+
 static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 			      struct pt_regs *regs)
 {
@@ -313,6 +352,12 @@  static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 	    "Ignoring spurious kernel translation fault at virtual address %016lx\n", addr))
 		return;
 
+	if (is_el1_mte_sync_tag_check_fault(esr)) {
+		do_tag_recovery(addr, esr, regs);
+
+		return;
+	}
+
 	if (is_el1_permission_fault(addr, esr, regs)) {
 		if (esr & ESR_ELx_WNR)
 			msg = "write to read-only memory";