Re: [PATCH v3 26/39] arm64: mte: Add in-kernel tag fault handler
From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2020-09-25 10:49:41
Also in:
linux-mm, lkml
On Fri, Sep 25, 2020 at 12:50:33AM +0200, Andrey Konovalov wrote:
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index 991dd5f031e4..c7fff8daf2a7 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); }
This look fine.
quoted hunk ↗ jump to hunk
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index a3bd189602df..d110f382dacf 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,11 @@ 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) +{ +}
Do we need to introduce report_tag_fault() in this patch? It's fine but add a note in the commit log that it will be populated in a subsequent patch.
quoted hunk ↗ jump to hunk
+ static void __do_kernel_fault(unsigned long addr, unsigned int esr, struct pt_regs *regs) {@@ -641,10 +647,40 @@ 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) +{ + static bool reported = false; + + if (!READ_ONCE(reported)) { + report_tag_fault(addr, esr, regs); + WRITE_ONCE(reported, true); + }
I don't mind the READ_ONCE/WRITE_ONCE here but not sure what they help with. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel