Only perform uvm_map_inentry() checks for PROC_SP for userland pagefaults.

This should be sufficient for identifying pivoted ROP.  Doing so for other
traps is at best opportunistic for finding a straight-running ROP chain,
but the added (and rare) sleeping point has proven to be dangerous.
Discussed at length with kettenis and mortimer.
ok mortimer kettenis mpi

10 files changed, 91 insertions, 73 deletions

diff --git a/sys/arch/alpha/alpha/trap.c b/sys/arch/alpha/alpha/trap.c
index c145c8b1627..a3b526c46d2 100644
--- a/sys/arch/alpha/alpha/trap.c
+++ b/sys/arch/alpha/alpha/trap.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: trap.c,v 1.89 2020/08/19 10:10:57 mpi Exp $ */
+/* $OpenBSD: trap.c,v 1.90 2020/09/24 17:54:28 deraadt Exp $ */
 /* $NetBSD: trap.c,v 1.52 2000/05/24 16:48:33 thorpej Exp $ */
 
 /*-
@@ -244,10 +244,6 @@ trap(a0, a1, a2, entry, framep)
 	if (user) {
 		p->p_md.md_tf = framep;
 		refreshcreds(p);
-		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-		   "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-			goto out;
 	}
 
 	switch (entry) {
@@ -370,6 +366,12 @@ trap(a0, a1, a2, entry, framep)
 		break;
 
 	case ALPHA_KENTRY_MM:
+		if (user &&
+		    !uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		   "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto out;
+
 		switch (a1) {
 		case ALPHA_MMCSR_FOR:
 		case ALPHA_MMCSR_FOE:
diff --git a/sys/arch/amd64/amd64/trap.c b/sys/arch/amd64/amd64/trap.c
index c401042f03e..f18162c9084 100644
--- a/sys/arch/amd64/amd64/trap.c
+++ b/sys/arch/amd64/amd64/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.81 2020/09/14 12:51:28 kettenis Exp $	*/
+/*	$OpenBSD: trap.c,v 1.82 2020/09/24 17:54:29 deraadt Exp $	*/
 /*	$NetBSD: trap.c,v 1.2 2003/05/04 23:51:56 fvdl Exp $	*/
 
 /*-
@@ -343,11 +343,6 @@ usertrap(struct trapframe *frame)
 	p->p_md.md_regs = frame;
 	refreshcreds(p);
 
-	if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-	    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-	    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-		goto out;
-
 	switch (type) {
 	case T_PROTFLT:			/* protection fault */
 	case T_TSSFLT:
@@ -381,6 +376,10 @@ usertrap(struct trapframe *frame)
 		break;
 
 	case T_PAGEFLT:			/* page fault */
+		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto out;
 		if (pageflttrap(frame, cr2, 1))
 			goto out;
 		/* FALLTHROUGH */
diff --git a/sys/arch/hppa/hppa/trap.c b/sys/arch/hppa/hppa/trap.c
index 7ddb02b15ab..ddc148a75c1 100644
--- a/sys/arch/hppa/hppa/trap.c
+++ b/sys/arch/hppa/hppa/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.148 2020/09/14 19:04:30 kettenis Exp $	*/
+/*	$OpenBSD: trap.c,v 1.149 2020/09/24 17:54:29 deraadt Exp $	*/
 
 /*
  * Copyright (c) 1998-2004 Michael Shalayeff
@@ -213,13 +213,8 @@ trap(int type, struct trapframe *frame)
 		mtctl(frame->tf_eiem, CR_EIEM);
 	}
 
-	if (type & T_USER) {
+	if (type & T_USER)
 		refreshcreds(p);
-		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-			goto out;
-	}
 
 	switch (type) {
 	case T_NONEXIST:
@@ -462,6 +457,13 @@ datacc:
 	case T_ITLBMISS | T_USER:
 	case T_DTLBMISS:
 	case T_DTLBMISS | T_USER:
+		if (type & T_USER) {
+			if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+			    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+			    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+				goto out;
+		}
+
 		/*
 		 * it could be a kernel map for exec_map faults
 		 */
diff --git a/sys/arch/i386/i386/trap.c b/sys/arch/i386/i386/trap.c
index 14b5be0d6fb..e3a5c7e5264 100644
--- a/sys/arch/i386/i386/trap.c
+++ b/sys/arch/i386/i386/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.147 2020/09/24 11:36:50 deraadt Exp $	*/
+/*	$OpenBSD: trap.c,v 1.148 2020/09/24 17:54:29 deraadt Exp $	*/
 /*	$NetBSD: trap.c,v 1.95 1996/05/05 06:50:02 mycroft Exp $	*/
 
 /*-
@@ -154,10 +154,6 @@ trap(struct trapframe *frame)
 		type |= T_USER;
 		p->p_md.md_regs = frame;
 		refreshcreds(p);
-		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-			goto out;
 	}
 
 	switch (type) {
@@ -349,6 +345,10 @@ trap(struct trapframe *frame)
 		int error;
 		int signal, sicode;
 
+		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto out;
 		KERNEL_LOCK();
 	faultcommon:
 		vm = p->p_vmspace;
diff --git a/sys/arch/m88k/m88k/trap.c b/sys/arch/m88k/m88k/trap.c
index 6e05dedf245..18cc4efcdb4 100644
--- a/sys/arch/m88k/m88k/trap.c
+++ b/sys/arch/m88k/m88k/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.113 2020/09/23 19:45:32 deraadt Exp $	*/
+/*	$OpenBSD: trap.c,v 1.114 2020/09/24 17:54:29 deraadt Exp $	*/
 /*
  * Copyright (c) 2004, Miodrag Vallat.
  * Copyright (c) 1998 Steve Murphree, Jr.
@@ -239,10 +239,6 @@ m88100_trap(u_int type, struct trapframe *frame)
 		type |= T_USER;
 		p->p_md.md_tf = frame;	/* for ptrace/signals */
 		refreshcreds(p);
-		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-			goto userexit;
 	}
 	fault_type = SI_NOINFO;
 	fault_code = 0;
@@ -681,10 +677,6 @@ m88110_trap(u_int type, struct trapframe *frame)
 		type |= T_USER;
 		p->p_md.md_tf = frame;	/* for ptrace/signals */
 		refreshcreds(p);
-		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-			goto userexit;
 	}
 
 	if (sig != 0)
@@ -862,6 +854,11 @@ lose:
 		/* User mode instruction access fault */
 		/* FALLTHROUGH */
 	case T_DATAFLT+T_USER:
+		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto userexit;
+
 		KERNEL_LOCK();
 m88110_user_fault:
 		if (type == T_INSTFLT+T_USER) {
diff --git a/sys/arch/mips64/mips64/trap.c b/sys/arch/mips64/mips64/trap.c
index 2a657cd6cb2..42a1827b845 100644
--- a/sys/arch/mips64/mips64/trap.c
+++ b/sys/arch/mips64/mips64/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.146 2020/08/19 10:10:58 mpi Exp $	*/
+/*	$OpenBSD: trap.c,v 1.147 2020/09/24 17:54:30 deraadt Exp $	*/
 
 /*
  * Copyright (c) 1988 University of Utah.
@@ -261,16 +261,11 @@ trap(struct trapframe *trapframe)
 	}
 #endif
 
-	if (type & T_USER) {
+	if (type & T_USER)
 		refreshcreds(p);
-		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-			goto out;
-	}
 
 	itsa(trapframe, ci, p, type);
-out:
+
 	if (type & T_USER)
 		userret(p);
 }
@@ -394,6 +389,11 @@ itsa(struct trapframe *trapframe, struct cpu_info *ci, struct proc *p,
 		ftype = PROT_WRITE;
 		pcb = &p->p_addr->u_pcb;
 fault_common:
+		if ((type & T_USER) &&
+		    !uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			return;
 
 #ifdef CPU_R4000
 		if (r4000_errata != 0) {
diff --git a/sys/arch/powerpc/powerpc/trap.c b/sys/arch/powerpc/powerpc/trap.c
index b6c67e732fc..f039aeda230 100644
--- a/sys/arch/powerpc/powerpc/trap.c
+++ b/sys/arch/powerpc/powerpc/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.115 2020/08/19 10:10:58 mpi Exp $	*/
+/*	$OpenBSD: trap.c,v 1.116 2020/09/24 17:54:30 deraadt Exp $	*/
 /*	$NetBSD: trap.c,v 1.3 1996/10/13 03:31:37 christos Exp $	*/
 
 /*
@@ -245,10 +245,6 @@ trap(struct trapframe *frame)
 	if (frame->srr1 & PSL_PR) {
 		type |= EXC_USER;
 		refreshcreds(p);
-		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-			goto out;
 	}
 
 	switch (type) {
@@ -310,6 +306,11 @@ trap(struct trapframe *frame)
 		    frame->dar, frame->dsisr, 0))
 			break;
 
+		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto out;
+
 		KERNEL_LOCK();
 		if (frame->dsisr & DSISR_STORE) {
 			ftype = PROT_READ | PROT_WRITE;
diff --git a/sys/arch/powerpc64/powerpc64/trap.c b/sys/arch/powerpc64/powerpc64/trap.c
index 79287ed1560..20b01e93e83 100644
--- a/sys/arch/powerpc64/powerpc64/trap.c
+++ b/sys/arch/powerpc64/powerpc64/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.37 2020/09/15 07:47:24 kettenis Exp $	*/
+/*	$OpenBSD: trap.c,v 1.38 2020/09/24 17:54:30 deraadt Exp $	*/
 
 /*
  * Copyright (c) 2020 Mark Kettenis <kettenis@openbsd.org>
@@ -92,10 +92,6 @@ trap(struct trapframe *frame)
 		type |= EXC_USER;
 		p->p_md.md_regs = frame;
 		refreshcreds(p);
-		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-			goto out;
 	}
 
 	switch (type) {
@@ -172,6 +168,11 @@ trap(struct trapframe *frame)
 		/* FALLTHROUGH */
 
 	case EXC_DSI|EXC_USER:
+		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto out;
+
 		map = &p->p_vmspace->vm_map;
 		va = frame->dar;
 		if (frame->dsisr & DSISR_STORE)
@@ -214,6 +215,11 @@ trap(struct trapframe *frame)
 		/* FALLTHROUGH */
 
 	case EXC_ISI|EXC_USER:
+		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto out;
+
 		map = &p->p_vmspace->vm_map;
 		va = frame->srr0;
 		ftype = PROT_READ | PROT_EXEC;
diff --git a/sys/arch/sh/sh/trap.c b/sys/arch/sh/sh/trap.c
index 026114fc519..0f84e2fe593 100644
--- a/sys/arch/sh/sh/trap.c
+++ b/sys/arch/sh/sh/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.42 2020/09/22 15:50:20 deraadt Exp $	*/
+/*	$OpenBSD: trap.c,v 1.43 2020/09/24 17:54:30 deraadt Exp $	*/
 /*	$NetBSD: exception.c,v 1.32 2006/09/04 23:57:52 uwe Exp $	*/
 /*	$NetBSD: syscall.c,v 1.6 2006/03/07 07:21:50 thorpej Exp $	*/
 
@@ -173,12 +173,6 @@ general_exception(struct proc *p, struct trapframe *tf, uint32_t va)
 		KDASSERT(p->p_md.md_regs == tf); /* check exception depth */
 		expevt |= EXP_USER;
 		refreshcreds(p);
-		if (tra != _SH_TRA_SYSCALL << 2) {
-			if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-			    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-			    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-				goto out;
-		}
 	}
 
 	switch (expevt) {
@@ -370,7 +364,7 @@ tlb_exception(struct proc *p, struct trapframe *tf, uint32_t va)
 		if (usermode) {
 			sv.sival_ptr = (void *)va;
 			trapsignal(p, SIGSEGV, tf->tf_expevt, SEGV_ACCERR, sv);
-			goto user_fault;
+			goto out;
 		} else {
 			TLB_ASSERT(p->p_md.md_pcb->pcb_onfault != NULL,
 			    "no copyin/out fault handler (load protection)");
@@ -389,6 +383,11 @@ tlb_exception(struct proc *p, struct trapframe *tf, uint32_t va)
 
 	/* Select address space */
 	if (usermode) {
+		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto out;
+
 		TLB_ASSERT(p != NULL, "no curproc");
 		map = &p->p_vmspace->vm_map;
 		pmap = map->pmap;
@@ -446,7 +445,7 @@ tlb_exception(struct proc *p, struct trapframe *tf, uint32_t va)
 			trapsignal(p, SIGKILL, tf->tf_expevt, SEGV_MAPERR, sv);
 		} else
 			trapsignal(p, SIGSEGV, tf->tf_expevt, SEGV_MAPERR, sv);
-		goto user_fault;
+		goto out;
 	} else {
 		TLB_ASSERT(p->p_md.md_pcb->pcb_onfault,
 		    "no copyin/out fault handler (page not found)");
@@ -454,7 +453,7 @@ tlb_exception(struct proc *p, struct trapframe *tf, uint32_t va)
 	}
 	return;
 
-user_fault:
+out:
 	userret(p);
 	ast(p, tf);
 	return;
diff --git a/sys/arch/sparc64/sparc64/trap.c b/sys/arch/sparc64/sparc64/trap.c
index e8c36d61e94..360321ee896 100644
--- a/sys/arch/sparc64/sparc64/trap.c
+++ b/sys/arch/sparc64/sparc64/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.103 2020/08/19 10:10:58 mpi Exp $	*/
+/*	$OpenBSD: trap.c,v 1.104 2020/09/24 17:54:30 deraadt Exp $	*/
 /*	$NetBSD: trap.c,v 1.73 2001/08/09 01:03:01 eeh Exp $ */
 
 /*
@@ -426,10 +426,6 @@ trap(struct trapframe64 *tf, unsigned type, vaddr_t pc, long tstate)
 	pcb = &p->p_addr->u_pcb;
 	p->p_md.md_tf = tf;	/* for ptrace/signals */
 	refreshcreds(p);
-	if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
-	    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
-	    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
-		goto out;
 
 	switch (type) {
 
@@ -678,7 +674,7 @@ dopanic:
 		trapsignal(p, SIGFPE, FPE_INTOVF_TRAP, FPE_INTOVF, sv);
 		break;
 	}
-out:
+
 	userret(p);
 	share_fpu(p, tf);
 #undef ADVANCE
@@ -791,8 +787,13 @@ data_access_fault(struct trapframe64 *tf, unsigned type, vaddr_t pc,
 			goto kfault;
 		}
 	} else {
-		KERNEL_LOCK();
 		p->p_md.md_tf = tf;
+		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto out;
+
+		KERNEL_LOCK();
 	}
 
 	vm = p->p_vmspace;
@@ -857,12 +858,12 @@ kfault:
 		trapsignal(p, signal, access_type, sicode, sv);
 	}
 
+	KERNEL_UNLOCK();
+
+out:
 	if ((tstate & TSTATE_PRIV) == 0) {
-		KERNEL_UNLOCK();
 		userret(p);
 		share_fpu(p, tf);
-	} else {
-		KERNEL_UNLOCK();
 	}
 }
 
@@ -934,8 +935,8 @@ data_access_error(struct trapframe64 *tf, unsigned type, vaddr_t afva,
 	}
 
 	trapsignal(p, SIGSEGV, PROT_READ | PROT_WRITE, SEGV_MAPERR, sv);
-out:
 
+out:
 	if ((tstate & TSTATE_PRIV) == 0) {
 		userret(p);
 		share_fpu(p, tf);
@@ -975,8 +976,13 @@ text_access_fault(struct trapframe64 *tf, unsigned type, vaddr_t pc,
 		(void) splhigh();
 		panic("kernel text_access_fault: pc=%lx va=%lx", pc, va);
 		/* NOTREACHED */
-	} else
+	} else {
 		p->p_md.md_tf = tf;
+		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto out;
+	}
 
 	KERNEL_LOCK();
 
@@ -1020,6 +1026,7 @@ text_access_fault(struct trapframe64 *tf, unsigned type, vaddr_t pc,
 
 	KERNEL_UNLOCK();
 
+out:
 	if ((tstate & TSTATE_PRIV) == 0) {
 		userret(p);
 		share_fpu(p, tf);
@@ -1077,8 +1084,13 @@ text_access_error(struct trapframe64 *tf, unsigned type, vaddr_t pc,
 		panic("kernel text error: pc=%lx sfsr=%lb", pc,
 		    sfsr, SFSR_BITS);
 		/* NOTREACHED */
-	} else
+	} else {
 		p->p_md.md_tf = tf;
+		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
+		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
+		    uvm_map_inentry_sp, p->p_vmspace->vm_map.sserial))
+			goto out;
+	}
 
 	KERNEL_LOCK();