Message ID | 20200421142603.3894-11-catalin.marinas@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | arm64: Memory Tagging Extension user-space support | expand |
On Tue, Apr 21, 2020 at 03:25:50PM +0100, Catalin Marinas wrote: > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > index ddcde093c433..3650a0a77ed0 100644 > --- a/arch/arm64/kernel/entry.S > +++ b/arch/arm64/kernel/entry.S > @@ -145,6 +145,31 @@ alternative_cb_end > #endif > .endm > > + /* Check for MTE asynchronous tag check faults */ > + .macro check_mte_async_tcf, flgs, tmp > +#ifdef CONFIG_ARM64_MTE > +alternative_if_not ARM64_MTE > + b 1f > +alternative_else_nop_endif > + mrs_s \tmp, SYS_TFSRE0_EL1 > + tbz \tmp, #SYS_TFSR_EL1_TF0_SHIFT, 1f > + /* Asynchronous TCF occurred for TTBR0 access, set the TI flag */ > + orr \flgs, \flgs, #_TIF_MTE_ASYNC_FAULT > + str \flgs, [tsk, #TSK_TI_FLAGS] > + msr_s SYS_TFSRE0_EL1, xzr > +1: > +#endif > + .endm > + > + /* Clear the MTE asynchronous tag check faults */ > + .macro clear_mte_async_tcf > +#ifdef CONFIG_ARM64_MTE > +alternative_if ARM64_MTE > + msr_s SYS_TFSRE0_EL1, xzr > +alternative_else_nop_endif This needs a 'dsb ish' prior to the msr as an indirect write (async tag check fault) to the TFSRE0_EL1 register is not ordered with a subsequent direct write (msr) to this register. The check_mte_async_tcf macro is fine as we execute it after taking an exception with SCTLR_EL1.ITFSB bit set (which triggers such synchronisation).
On Tue, Apr 21, 2020 at 03:25:50PM +0100, Catalin Marinas wrote: > From: Vincenzo Frascino <vincenzo.frascino@arm.com> > > The Memory Tagging Extension has two modes of notifying a tag check > fault at EL0, configurable through the SCTLR_EL1.TCF0 field: > > 1. Synchronous raising of a Data Abort exception with DFSC 17. > 2. Asynchronous setting of a cumulative bit in TFSRE0_EL1. > > Add the exception handler for the synchronous exception and handling of > the asynchronous TFSRE0_EL1.TF0 bit setting via a new TIF flag in > do_notify_resume(). > > On a tag check failure in user-space, whether synchronous or > asynchronous, a SIGSEGV will be raised on the faulting thread. Has there been any discussion on whether this should be SIGSEGV or SIGBUS? Probably neither is much more appropriate than the other. > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> > Co-developed-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> [...] > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c > index 339882db5a91..e377d77c065e 100644 > --- a/arch/arm64/kernel/signal.c > +++ b/arch/arm64/kernel/signal.c > @@ -732,6 +732,9 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > regs->regs[29] = (unsigned long)&user->next_frame->fp; > regs->pc = (unsigned long)ka->sa.sa_handler; > > + /* TCO (Tag Check Override) always cleared for signal handlers */ > + regs->pstate &= ~PSR_TCO_BIT; > + > if (ka->sa.sa_flags & SA_RESTORER) > sigtramp = ka->sa.sa_restorer; > else > @@ -923,6 +926,11 @@ asmlinkage void do_notify_resume(struct pt_regs *regs, > if (thread_flags & _TIF_UPROBE) > uprobe_notify_resume(regs); > > + if (thread_flags & _TIF_MTE_ASYNC_FAULT) { > + clear_thread_flag(TIF_MTE_ASYNC_FAULT); > + force_signal_inject(SIGSEGV, SEGV_MTEAERR, 0); > + } > + Should this definitely be a force_signal_inject()? SEGV_MTEAERR is not intrinsically fatal: it must be possible to run past the error, because that's the whole point -- chances are we already did. Compare this with MTESERR where running past the signal would lead to a spin. If MTEAERR is forced, a martian tag check failure might land in the middle of a "normal" SIGSEGV, when SIGSEGV would usually be blocked for good reasons, defeating the process' own handling mechanisms for no good reason: delivering the MTEAERR when SIGSEGV is next unblocked seems perfectly reasonable in such a case. Only braindead software would block or ignore things like SIGSEGV across exec, so software shouldn't end up ignoring these non-forced signals unless it does so on purpose. Alternatively, perhaps asynchronous errors should be delivered via a different signal. I don't have a good suggestion though. [...] Cheers ---Dave
On Mon, Apr 27, 2020 at 05:58:22PM +0100, Dave P Martin wrote: > On Tue, Apr 21, 2020 at 03:25:50PM +0100, Catalin Marinas wrote: > > From: Vincenzo Frascino <vincenzo.frascino@arm.com> > > > > The Memory Tagging Extension has two modes of notifying a tag check > > fault at EL0, configurable through the SCTLR_EL1.TCF0 field: > > > > 1. Synchronous raising of a Data Abort exception with DFSC 17. > > 2. Asynchronous setting of a cumulative bit in TFSRE0_EL1. > > > > Add the exception handler for the synchronous exception and handling of > > the asynchronous TFSRE0_EL1.TF0 bit setting via a new TIF flag in > > do_notify_resume(). > > > > On a tag check failure in user-space, whether synchronous or > > asynchronous, a SIGSEGV will be raised on the faulting thread. > > Has there been any discussion on whether this should be SIGSEGV or > SIGBUS? > > Probably neither is much more appropriate than the other. You could argue either way. I don't recall a firm conclusion on this, so I picked one that follows SPARC ADI. > > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c > > index 339882db5a91..e377d77c065e 100644 > > --- a/arch/arm64/kernel/signal.c > > +++ b/arch/arm64/kernel/signal.c > > @@ -732,6 +732,9 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > > regs->regs[29] = (unsigned long)&user->next_frame->fp; > > regs->pc = (unsigned long)ka->sa.sa_handler; > > > > + /* TCO (Tag Check Override) always cleared for signal handlers */ > > + regs->pstate &= ~PSR_TCO_BIT; > > + > > if (ka->sa.sa_flags & SA_RESTORER) > > sigtramp = ka->sa.sa_restorer; > > else > > @@ -923,6 +926,11 @@ asmlinkage void do_notify_resume(struct pt_regs *regs, > > if (thread_flags & _TIF_UPROBE) > > uprobe_notify_resume(regs); > > > > + if (thread_flags & _TIF_MTE_ASYNC_FAULT) { > > + clear_thread_flag(TIF_MTE_ASYNC_FAULT); > > + force_signal_inject(SIGSEGV, SEGV_MTEAERR, 0); > > + } > > + > > Should this definitely be a force_signal_inject()? > > SEGV_MTEAERR is not intrinsically fatal: it must be possible to run past > the error, because that's the whole point -- chances are we already did. > > Compare this with MTESERR where running past the signal would lead to a > spin. Good point. This can be a send_sig_fault() (I need to check the right API). Thanks.
On Tue, Apr 28, 2020 at 02:43:01PM +0100, Catalin Marinas wrote: > On Mon, Apr 27, 2020 at 05:58:22PM +0100, Dave P Martin wrote: > > On Tue, Apr 21, 2020 at 03:25:50PM +0100, Catalin Marinas wrote: > > > From: Vincenzo Frascino <vincenzo.frascino@arm.com> > > > > > > The Memory Tagging Extension has two modes of notifying a tag check > > > fault at EL0, configurable through the SCTLR_EL1.TCF0 field: > > > > > > 1. Synchronous raising of a Data Abort exception with DFSC 17. > > > 2. Asynchronous setting of a cumulative bit in TFSRE0_EL1. > > > > > > Add the exception handler for the synchronous exception and handling of > > > the asynchronous TFSRE0_EL1.TF0 bit setting via a new TIF flag in > > > do_notify_resume(). > > > > > > On a tag check failure in user-space, whether synchronous or > > > asynchronous, a SIGSEGV will be raised on the faulting thread. > > > > Has there been any discussion on whether this should be SIGSEGV or > > SIGBUS? > > > > Probably neither is much more appropriate than the other. > > You could argue either way. I don't recall a firm conclusion on this, so > I picked one that follows SPARC ADI. Agreed, that precedent is good enough for me. I hadn't refreshed my memory of how sparc was using these signals. > > > > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c > > > index 339882db5a91..e377d77c065e 100644 > > > --- a/arch/arm64/kernel/signal.c > > > +++ b/arch/arm64/kernel/signal.c > > > @@ -732,6 +732,9 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, > > > regs->regs[29] = (unsigned long)&user->next_frame->fp; > > > regs->pc = (unsigned long)ka->sa.sa_handler; > > > > > > + /* TCO (Tag Check Override) always cleared for signal handlers */ > > > + regs->pstate &= ~PSR_TCO_BIT; > > > + > > > if (ka->sa.sa_flags & SA_RESTORER) > > > sigtramp = ka->sa.sa_restorer; > > > else > > > @@ -923,6 +926,11 @@ asmlinkage void do_notify_resume(struct pt_regs *regs, > > > if (thread_flags & _TIF_UPROBE) > > > uprobe_notify_resume(regs); > > > > > > + if (thread_flags & _TIF_MTE_ASYNC_FAULT) { > > > + clear_thread_flag(TIF_MTE_ASYNC_FAULT); > > > + force_signal_inject(SIGSEGV, SEGV_MTEAERR, 0); > > > + } > > > + > > > > Should this definitely be a force_signal_inject()? > > > > SEGV_MTEAERR is not intrinsically fatal: it must be possible to run past > > the error, because that's the whole point -- chances are we already did. > > > > Compare this with MTESERR where running past the signal would lead to a > > spin. > > Good point. This can be a send_sig_fault() (I need to check the right > API). Sounds fair. Cheers ---Dave
diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h index 64e814273659..e9711ea51eb5 100644 --- a/arch/arm64/include/asm/mte.h +++ b/arch/arm64/include/asm/mte.h @@ -4,8 +4,18 @@ #ifndef __ASSEMBLY__ +#include <linux/sched.h> + /* Memory Tagging API */ int mte_memcmp_pages(const void *page1_addr, const void *page2_addr); +#ifdef CONFIG_ARM64_MTE +void flush_mte_state(void); +#else +static inline void flush_mte_state(void) +{ +} +#endif + #endif /* __ASSEMBLY__ */ #endif /* __ASM_MTE_H */ diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h index 512174a8e789..0c6e5523b932 100644 --- a/arch/arm64/include/asm/thread_info.h +++ b/arch/arm64/include/asm/thread_info.h @@ -63,6 +63,7 @@ void arch_release_task_struct(struct task_struct *tsk); #define TIF_FOREIGN_FPSTATE 3 /* CPU's FP state is not current's */ #define TIF_UPROBE 4 /* uprobe breakpoint or singlestep */ #define TIF_FSCHECK 5 /* Check FS is USER_DS on return */ +#define TIF_MTE_ASYNC_FAULT 6 /* MTE Asynchronous Tag Check Fault */ #define TIF_SYSCALL_TRACE 8 /* syscall trace active */ #define TIF_SYSCALL_AUDIT 9 /* syscall auditing */ #define TIF_SYSCALL_TRACEPOINT 10 /* syscall tracepoint for ftrace */ @@ -91,10 +92,11 @@ void arch_release_task_struct(struct task_struct *tsk); #define _TIF_FSCHECK (1 << TIF_FSCHECK) #define _TIF_32BIT (1 << TIF_32BIT) #define _TIF_SVE (1 << TIF_SVE) +#define _TIF_MTE_ASYNC_FAULT (1 << TIF_MTE_ASYNC_FAULT) #define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \ _TIF_NOTIFY_RESUME | _TIF_FOREIGN_FPSTATE | \ - _TIF_UPROBE | _TIF_FSCHECK) + _TIF_UPROBE | _TIF_FSCHECK | _TIF_MTE_ASYNC_FAULT) #define _TIF_SYSCALL_WORK (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \ _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP | \ diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 4e5b8ee31442..dbede7a4c5fb 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -63,6 +63,7 @@ obj-$(CONFIG_CRASH_CORE) += crash_core.o obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o obj-$(CONFIG_ARM64_SSBD) += ssbd.o obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o +obj-$(CONFIG_ARM64_MTE) += mte.o obj-y += vdso/ probes/ obj-$(CONFIG_COMPAT_VDSO) += vdso32/ diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index ddcde093c433..3650a0a77ed0 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -145,6 +145,31 @@ alternative_cb_end #endif .endm + /* Check for MTE asynchronous tag check faults */ + .macro check_mte_async_tcf, flgs, tmp +#ifdef CONFIG_ARM64_MTE +alternative_if_not ARM64_MTE + b 1f +alternative_else_nop_endif + mrs_s \tmp, SYS_TFSRE0_EL1 + tbz \tmp, #SYS_TFSR_EL1_TF0_SHIFT, 1f + /* Asynchronous TCF occurred for TTBR0 access, set the TI flag */ + orr \flgs, \flgs, #_TIF_MTE_ASYNC_FAULT + str \flgs, [tsk, #TSK_TI_FLAGS] + msr_s SYS_TFSRE0_EL1, xzr +1: +#endif + .endm + + /* Clear the MTE asynchronous tag check faults */ + .macro clear_mte_async_tcf +#ifdef CONFIG_ARM64_MTE +alternative_if ARM64_MTE + msr_s SYS_TFSRE0_EL1, xzr +alternative_else_nop_endif +#endif + .endm + .macro kernel_entry, el, regsize = 64 .if \regsize == 32 mov w0, w0 // zero upper 32 bits of x0 @@ -176,6 +201,8 @@ alternative_cb_end ldr x19, [tsk, #TSK_TI_FLAGS] disable_step_tsk x19, x20 + /* Check for asynchronous tag check faults in user space */ + check_mte_async_tcf x19, x22 apply_ssbd 1, x22, x23 ptrauth_keys_install_kernel tsk, 1, x20, x22, x23 @@ -244,6 +271,13 @@ alternative_if ARM64_HAS_IRQ_PRIO_MASKING str x20, [sp, #S_PMR_SAVE] alternative_else_nop_endif + /* Re-enable tag checking (TCO set on exception entry) */ +#ifdef CONFIG_ARM64_MTE +alternative_if ARM64_MTE + SET_PSTATE_TCO(0) +alternative_else_nop_endif +#endif + /* * Registers that may be useful after this macro is invoked: * @@ -744,6 +778,8 @@ work_pending: ret_to_user: disable_daif gic_prio_kentry_setup tmp=x3 + /* Ignore asynchronous tag check faults in the uaccess routines */ + clear_mte_async_tcf ldr x1, [tsk, #TSK_TI_FLAGS] and x2, x1, #_TIF_WORK_MASK cbnz x2, work_pending diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c new file mode 100644 index 000000000000..032016823957 --- /dev/null +++ b/arch/arm64/kernel/mte.c @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2020 ARM Ltd. + */ + +#include <linux/thread_info.h> + +#include <asm/cpufeature.h> +#include <asm/mte.h> +#include <asm/sysreg.h> + +void flush_mte_state(void) +{ + if (!system_supports_mte()) + return; + + /* clear any pending asynchronous tag fault */ + dsb(ish); + write_sysreg_s(0, SYS_TFSRE0_EL1); + clear_thread_flag(TIF_MTE_ASYNC_FAULT); +} diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 56be4cbf771f..740047c9cd13 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -50,6 +50,7 @@ #include <asm/exec.h> #include <asm/fpsimd.h> #include <asm/mmu_context.h> +#include <asm/mte.h> #include <asm/processor.h> #include <asm/pointer_auth.h> #include <asm/stacktrace.h> @@ -323,6 +324,7 @@ void flush_thread(void) tls_thread_flush(); flush_ptrace_hw_breakpoint(current); flush_tagged_addr_state(); + flush_mte_state(); } void release_thread(struct task_struct *dead_task) @@ -355,6 +357,9 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src) dst->thread.sve_state = NULL; clear_tsk_thread_flag(dst, TIF_SVE); + /* clear any pending asynchronous tag fault raised by the parent */ + clear_tsk_thread_flag(dst, TIF_MTE_ASYNC_FAULT); + return 0; } diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c index 339882db5a91..e377d77c065e 100644 --- a/arch/arm64/kernel/signal.c +++ b/arch/arm64/kernel/signal.c @@ -732,6 +732,9 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka, regs->regs[29] = (unsigned long)&user->next_frame->fp; regs->pc = (unsigned long)ka->sa.sa_handler; + /* TCO (Tag Check Override) always cleared for signal handlers */ + regs->pstate &= ~PSR_TCO_BIT; + if (ka->sa.sa_flags & SA_RESTORER) sigtramp = ka->sa.sa_restorer; else @@ -923,6 +926,11 @@ asmlinkage void do_notify_resume(struct pt_regs *regs, if (thread_flags & _TIF_UPROBE) uprobe_notify_resume(regs); + if (thread_flags & _TIF_MTE_ASYNC_FAULT) { + clear_thread_flag(TIF_MTE_ASYNC_FAULT); + force_signal_inject(SIGSEGV, SEGV_MTEAERR, 0); + } + if (thread_flags & _TIF_SIGPENDING) do_signal(regs); diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c index a12c0c88d345..db25f5d6a07c 100644 --- a/arch/arm64/kernel/syscall.c +++ b/arch/arm64/kernel/syscall.c @@ -102,6 +102,16 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr, local_daif_restore(DAIF_PROCCTX); user_exit(); + if (system_supports_mte() && (flags & _TIF_MTE_ASYNC_FAULT)) { + /* + * Process the asynchronous tag check fault before the actual + * syscall. do_notify_resume() will send a signal to userspace + * before the syscall is restarted. + */ + regs->regs[0] = -ERESTARTNOINTR; + return; + } + if (has_syscall_work(flags)) { /* set default errno for user-issued syscall(-1) */ if (scno == NO_SYSCALL) diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c index c9cedc0432d2..38b59cace3e3 100644 --- a/arch/arm64/mm/fault.c +++ b/arch/arm64/mm/fault.c @@ -650,6 +650,13 @@ static int do_sea(unsigned long addr, unsigned int esr, struct pt_regs *regs) return 0; } +static int do_tag_check_fault(unsigned long addr, unsigned int esr, + struct pt_regs *regs) +{ + do_bad_area(addr, esr, regs); + return 0; +} + static const struct fault_info fault_info[] = { { do_bad, SIGKILL, SI_KERNEL, "ttbr address size fault" }, { do_bad, SIGKILL, SI_KERNEL, "level 1 address size fault" }, @@ -668,7 +675,7 @@ static const struct fault_info fault_info[] = { { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 2 permission fault" }, { do_page_fault, SIGSEGV, SEGV_ACCERR, "level 3 permission fault" }, { do_sea, SIGBUS, BUS_OBJERR, "synchronous external abort" }, - { do_bad, SIGKILL, SI_KERNEL, "unknown 17" }, + { do_tag_check_fault, SIGSEGV, SEGV_MTESERR, "synchronous tag check fault" }, { do_bad, SIGKILL, SI_KERNEL, "unknown 18" }, { do_bad, SIGKILL, SI_KERNEL, "unknown 19" }, { do_sea, SIGKILL, SI_KERNEL, "level 0 (translation table walk)" },