When entering the kernel after an async tag fault due to a syscall, rather
than for another reason (e.g. preemption), we don't want to service the
syscall as it may mask the tag fault. Rewind the PC to the svc instruction
in order to give a userspace signal handler an opportunity to handle the
fault and resume, and skip all other syscall processing.
Signed-off-by: Peter Collingbourne <pcc@xxxxxxxxxx>
---
On Tue, Dec 17, 2019 at 10:01 AM Catalin Marinas <catalin.marinas@xxxxxxx> wrote:
>
> On Fri, Dec 13, 2019 at 05:43:15PM -0800, Peter Collingbourne wrote:
> > On Wed, Dec 11, 2019 at 10:44 AM Catalin Marinas
> > <catalin.marinas@xxxxxxx> wrote:
> > > diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> > > index dd2cdc0d5be2..41fae64af82a 100644
> > > --- a/arch/arm64/kernel/signal.c
> > > +++ b/arch/arm64/kernel/signal.c
> > > @@ -730,6 +730,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
> > > @@ -921,6 +924,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);
> >
> > In the case where the kernel is entered due to a syscall, this will
> > inject a signal, but only after servicing the syscall. This means
> > that, for example, if the syscall is exit(), the async tag check
> > failure will be silently ignored. I can reproduce the problem with the
> > program below:
> [...]
> > This patch fixes the problem for me:
> >
> > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > index 9a9d98a443fc..d0c8918dee00 100644
> > --- a/arch/arm64/kernel/syscall.c
> > +++ b/arch/arm64/kernel/syscall.c
> > @@ -94,6 +94,8 @@ static void el0_svc_common(struct pt_regs *regs, int
> > scno, int sc_nr,
> > const syscall_fn_t syscall_table[])
> > {
> > unsigned long flags = current_thread_info()->flags;
> > + if (flags & _TIF_MTE_ASYNC_FAULT)
> > + return;
>
> It needs a bit of thinking. This one wouldn't work if you want to handle
> the signal and resume since it would skip the SVC instruction. We'd need
> at least to do a regs->pc -= 4 and probably move it further down in this
> function.
Okay, how does this look?
Peter
arch/arm64/kernel/syscall.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
index 9a9d98a443fc..49ea9bb47190 100644
--- a/arch/arm64/kernel/syscall.c
+++ b/arch/arm64/kernel/syscall.c
@@ -95,13 +95,29 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
{
unsigned long flags = current_thread_info()->flags;
- regs->orig_x0 = regs->regs[0];
- regs->syscallno = scno;
-
cortex_a76_erratum_1463225_svc_handler();
local_daif_restore(DAIF_PROCCTX);
user_exit();
+#ifdef CONFIG_ARM64_MTE
+ if (flags & _TIF_MTE_ASYNC_FAULT) {
+ /*
+ * We entered the kernel after an async tag fault due to a
+ * syscall, rather than for another reason (e.g. preemption).
+ * In this case, we don't want to service the syscall as it may
+ * mask the tag fault. Rewind the PC to the svc instruction in
+ * order to give a userspace signal handler an opportunity to
+ * handle the fault and resume, and skip all other syscall
+ * processing.
+ */
+ regs->pc -= 4;
+ return;
+ }
+#endif
+
+ regs->orig_x0 = regs->regs[0];
+ regs->syscallno = scno;
+
if (has_syscall_work(flags)) {
/* set default errno for user-issued syscall(-1) */
if (scno == NO_SYSCALL)
--
2.24.1.735.g03f4e72817-goog
