diff mbox series

[v2,24/37] arm64: mte: Add in-kernel tag fault handler

Message ID 7866d9e6f11f12f1bad42c895bf4947addba71c2.1600204505.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 Sept. 15, 2020, 9:16 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:
* a warning is logged,
* 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.

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>
---
Change-Id: I9b8aa79567f7c45f4d6a1290efcf34567e620717
---
 arch/arm64/mm/fault.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

Comments

Catalin Marinas Sept. 17, 2020, 2:03 p.m. UTC | #1
On Tue, Sep 15, 2020 at 11:16:06PM +0200, Andrey Konovalov wrote:
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index a3bd189602df..cdc23662691c 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -294,6 +295,18 @@ 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)
> +{
> +	bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0;
> +
> +	pr_alert("Memory Tagging Extension Fault in %pS\n", (void *)regs->pc);
> +	pr_alert("  %s at address %lx\n", is_write ? "Write" : "Read", addr);
> +	pr_alert("  Pointer tag: [%02x], memory tag: [%02x]\n",
> +			mte_get_ptr_tag(addr),
> +			mte_get_mem_tag((void *)addr));
> +}
> +
>  static void __do_kernel_fault(unsigned long addr, unsigned int esr,
>  			      struct pt_regs *regs)
>  {
> @@ -641,10 +654,31 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
>  	return 0;
>  }
>  
> +static void do_tag_recovery(unsigned long addr, unsigned int esr,
> +			   struct pt_regs *regs)
> +{
> +	report_tag_fault(addr, esr, regs);

I'd only report this once since we expect it to be disabled lazily on
the other CPUs (i.e. just use a "static bool reported" to keep track).

> +
> +	/*
> +	 * Disable Memory Tagging Extension Tag Checking on the local CPU

Too verbose, just say MTE tag checking, people reading this code should
have learnt already what MTE stands for ;).

> +	 * 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 int do_tag_check_fault(unsigned long addr, unsigned int esr,
>  			      struct pt_regs *regs)
>  {
> -	do_bad_area(addr, esr, regs);
> +	/* The tag check fault (TCF) is per TTBR */
> +	if (is_ttbr0_addr(addr))
> +		do_bad_area(addr, esr, regs);
> +	else
> +		do_tag_recovery(addr, esr, regs);

This part looks fine now.
Vincenzo Frascino Sept. 17, 2020, 2:24 p.m. UTC | #2
On 9/17/20 3:03 PM, Catalin Marinas wrote:
> On Tue, Sep 15, 2020 at 11:16:06PM +0200, Andrey Konovalov wrote:
>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>> index a3bd189602df..cdc23662691c 100644
>> --- a/arch/arm64/mm/fault.c
>> +++ b/arch/arm64/mm/fault.c
>> @@ -294,6 +295,18 @@ 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)
>> +{
>> +	bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0;
>> +
>> +	pr_alert("Memory Tagging Extension Fault in %pS\n", (void *)regs->pc);
>> +	pr_alert("  %s at address %lx\n", is_write ? "Write" : "Read", addr);
>> +	pr_alert("  Pointer tag: [%02x], memory tag: [%02x]\n",
>> +			mte_get_ptr_tag(addr),
>> +			mte_get_mem_tag((void *)addr));
>> +}
>> +
>>  static void __do_kernel_fault(unsigned long addr, unsigned int esr,
>>  			      struct pt_regs *regs)
>>  {
>> @@ -641,10 +654,31 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
>>  	return 0;
>>  }
>>  
>> +static void do_tag_recovery(unsigned long addr, unsigned int esr,
>> +			   struct pt_regs *regs)
>> +{
>> +	report_tag_fault(addr, esr, regs);
> 
> I'd only report this once since we expect it to be disabled lazily on
> the other CPUs (i.e. just use a "static bool reported" to keep track).
>

Ok, I will fix it in the next version.

>> +
>> +	/*
>> +	 * Disable Memory Tagging Extension Tag Checking on the local CPU
> 
> Too verbose, just say MTE tag checking, people reading this code should
> have learnt already what MTE stands for ;).
> 

All right, will change it ;)

>> +	 * 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 int do_tag_check_fault(unsigned long addr, unsigned int esr,
>>  			      struct pt_regs *regs)
>>  {
>> -	do_bad_area(addr, esr, regs);
>> +	/* The tag check fault (TCF) is per TTBR */
>> +	if (is_ttbr0_addr(addr))
>> +		do_bad_area(addr, esr, regs);
>> +	else
>> +		do_tag_recovery(addr, esr, regs);
> 
> This part looks fine now.
> 

Ok, thanks.
Catalin Marinas Sept. 17, 2020, 2:59 p.m. UTC | #3
On Tue, Sep 15, 2020 at 11:16:06PM +0200, Andrey Konovalov wrote:
>  static int do_tag_check_fault(unsigned long addr, unsigned int esr,
>  			      struct pt_regs *regs)
>  {
> -	do_bad_area(addr, esr, regs);
> +	/* The tag check fault (TCF) is per TTBR */
> +	if (is_ttbr0_addr(addr))
> +		do_bad_area(addr, esr, regs);
> +	else
> +		do_tag_recovery(addr, esr, regs);
> +
>  	return 0;
>  }

I had forgotten the details here. The TCF mode is per EL, so TCF0
affects EL0, TCF affects EL1 irrespective of which TTBR is used. Now, we
know the kernel accesses TTBR0 usually with LDTR/STTR instructions if
UAO is available (soon to get rid of), so these would act as EL0
accesses using TCF0. However, we have the futex.h code which uses
exclusives and they'd be executed as EL1, so you can potentially get a
tag check fault for such uaccess even if the user disabled it in TCF0.

The solution here I think is for uaccess_enable() to set PSTATE.TCO,
restore it in uaccess_disable().

We get away with not toggling PSTATE.TCO in the user MTE patches since
the TCF is always 0 for the kernel.

The do_tag_check_fault() above is still correct, apart from the comment
which needs a better explanation on why we do a is_ttbr0_addr() check.
diff mbox series

Patch

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index a3bd189602df..cdc23662691c 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -33,6 +33,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>
@@ -294,6 +295,18 @@  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)
+{
+	bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) != 0;
+
+	pr_alert("Memory Tagging Extension Fault in %pS\n", (void *)regs->pc);
+	pr_alert("  %s at address %lx\n", is_write ? "Write" : "Read", addr);
+	pr_alert("  Pointer tag: [%02x], memory tag: [%02x]\n",
+			mte_get_ptr_tag(addr),
+			mte_get_mem_tag((void *)addr));
+}
+
 static void __do_kernel_fault(unsigned long addr, unsigned int esr,
 			      struct pt_regs *regs)
 {
@@ -641,10 +654,31 @@  static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs)
 	return 0;
 }
 
+static void do_tag_recovery(unsigned long addr, unsigned int esr,
+			   struct pt_regs *regs)
+{
+	report_tag_fault(addr, esr, regs);
+
+	/*
+	 * Disable Memory Tagging Extension 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 int do_tag_check_fault(unsigned long addr, unsigned int esr,
 			      struct pt_regs *regs)
 {
-	do_bad_area(addr, esr, regs);
+	/* The tag check fault (TCF) is per TTBR */
+	if (is_ttbr0_addr(addr))
+		do_bad_area(addr, esr, regs);
+	else
+		do_tag_recovery(addr, esr, regs);
+
 	return 0;
 }