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 |
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.
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.
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 --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; }