When operating in NX mode, GUCODE_SEL can cover the entire userland

address space permanently, and the line-in-the-sand manipulation
T_PROTFLT|T_USER fixup handling can be skipped.  This change was written
to side-step the segment-descriptor mismanagement issues, commited recently
for the "go on i386"
tested by sthen
ok kettenis guenther

4 files changed, 41 insertions, 22 deletions

diff --git a/sys/arch/i386/i386/machdep.c b/sys/arch/i386/i386/machdep.c
index a8b8529437e..f029a9c2c78 100644
--- a/sys/arch/i386/i386/machdep.c
+++ b/sys/arch/i386/i386/machdep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: machdep.c,v 1.639 2020/09/13 05:57:28 jsg Exp $	*/
+/*	$OpenBSD: machdep.c,v 1.640 2020/09/24 11:36:50 deraadt Exp $	*/
 /*	$NetBSD: machdep.c,v 1.214 1996/11/10 03:16:17 thorpej Exp $	*/
 
 /*-
@@ -2955,15 +2955,7 @@ setregs(struct proc *p, struct exec_package *pack, u_long stack,
 	p->p_md.md_flags &= ~MDP_USEDFPU;
 #endif
 
-	/*
-	 * Reset the code segment limit to I386_MAX_EXE_ADDR in the pmap;
-	 * this gets copied into the GDT for GUCODE_SEL by pmap_activate().
-	 * Similarly, reset the base of each of the two thread data
-	 * segments to zero in the pcb; they'll get copied into the
-	 * GDT for GUFS_SEL and GUGS_SEL.
-	 */
-	setsegment(&pmap->pm_codeseg, 0, atop(I386_MAX_EXE_ADDR) - 1,
-	    SDT_MEMERA, SEL_UPL, 1, 1);
+	initcodesegment(&pmap->pm_codeseg);
 	setsegment(&pcb->pcb_threadsegs[TSEG_FS], 0,
 	    atop(VM_MAXUSER_ADDRESS) - 1, SDT_MEMRWA, SEL_UPL, 1, 1);
 	setsegment(&pcb->pcb_threadsegs[TSEG_GS], 0,
@@ -3052,6 +3044,30 @@ setregion(struct region_descriptor *rd, void *base, size_t limit)
 }
 
 void
+initcodesegment(struct segment_descriptor *cs)
+{
+	if (cpu_pae) {
+		/*
+		 * When code execution is managed using NX feature
+		 * in pmapae.c, GUCODE_SEL should cover userland.
+		 */
+		setsegment(cs, 0, atop(VM_MAXUSER_ADDRESS - 1),
+		    SDT_MEMERA, SEL_UPL, 1, 1);
+	} else {
+		/*
+		 * For pmap.c's non-PAE/NX line-in-the-sand execution, reset
+		 * the code segment limit to I386_MAX_EXE_ADDR in the pmap;
+		 * this gets copied into the GDT for GUCODE_SEL by
+		 * pmap_activate().  Similarly, reset the base of each of
+		 * the two thread data segments to zero in the pcb; they'll
+		 * get copied into the GDT for GUFS_SEL and GUGS_SEL.
+		 */
+		setsegment(cs, 0, atop(I386_MAX_EXE_ADDR - 1),
+		    SDT_MEMERA, SEL_UPL, 1, 1);
+	}
+}
+
+void
 setsegment(struct segment_descriptor *sd, void *base, size_t limit, int type,
     int dpl, int def32, int gran)
 {
diff --git a/sys/arch/i386/i386/pmap.c b/sys/arch/i386/i386/pmap.c
index 4f6025e5851..51096f4e4ad 100644
--- a/sys/arch/i386/i386/pmap.c
+++ b/sys/arch/i386/i386/pmap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pmap.c,v 1.208 2020/09/24 11:18:37 kettenis Exp $	*/
+/*	$OpenBSD: pmap.c,v 1.209 2020/09/24 11:36:50 deraadt Exp $	*/
 /*	$NetBSD: pmap.c,v 1.91 2000/06/02 17:46:37 thorpej Exp $	*/
 
 /*
@@ -582,6 +582,9 @@ pmap_exec_account(struct pmap *pm, vaddr_t va,
 	if ((opte ^ npte) & PG_X)
 		pmap_tlb_shootpage(pm, va);
 			
+	if (cpu_pae)
+		return;
+
 	/*
 	 * Executability was removed on the last executable change.
 	 * Reset the code segment to something conservative and
@@ -615,6 +618,8 @@ pmap_exec_fixup(struct vm_map *map, struct trapframe *tf, vaddr_t gdt_cs,
 	vaddr_t va = 0;
 	vaddr_t pm_cs;
 
+	KERNEL_LOCK();
+
 	vm_map_lock(map);
 	RBT_FOREACH_REVERSE(ent, uvm_map_addr, &map->addr) {
 		if (ent->protection & PROT_EXEC)
@@ -642,6 +647,7 @@ pmap_exec_fixup(struct vm_map *map, struct trapframe *tf, vaddr_t gdt_cs,
 	 */
 	if (va <= pm->pm_hiexec && pm_cs == pm->pm_hiexec &&
 	    gdt_cs == pm->pm_hiexec) {
+		KERNEL_UNLOCK();
 		return (0);
 	}
 
@@ -654,6 +660,7 @@ pmap_exec_fixup(struct vm_map *map, struct trapframe *tf, vaddr_t gdt_cs,
 	 */
 	setcslimit(pm, tf, pcb, va);
 
+	KERNEL_UNLOCK();
 	return (1);
 }
 
@@ -1346,8 +1353,7 @@ pmap_create(void)
 	pmap->pm_hiexec = 0;
 	pmap->pm_flags = 0;
 
-	setsegment(&pmap->pm_codeseg, 0, atop(I386_MAX_EXE_ADDR) - 1,
-	    SDT_MEMERA, SEL_UPL, 1, 1);
+	initcodesegment(&pmap->pm_codeseg);
 
 	pmap_pinit_pd(pmap);
 	return (pmap);
diff --git a/sys/arch/i386/i386/trap.c b/sys/arch/i386/i386/trap.c
index edb6ff82189..14b5be0d6fb 100644
--- a/sys/arch/i386/i386/trap.c
+++ b/sys/arch/i386/i386/trap.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: trap.c,v 1.146 2020/09/24 11:18:37 kettenis Exp $	*/
+/*	$OpenBSD: trap.c,v 1.147 2020/09/24 11:36:50 deraadt Exp $	*/
 /*	$NetBSD: trap.c,v 1.95 1996/05/05 06:50:02 mycroft Exp $	*/
 
 /*-
@@ -259,18 +259,14 @@ trap(struct trapframe *frame)
 		return;
 
 	case T_PROTFLT|T_USER:		/* protection fault */
-		KERNEL_LOCK();
-
 		/* If pmap_exec_fixup does something, let's retry the trap. */
-		if (pmap_exec_fixup(&p->p_vmspace->vm_map, frame, gdt_cs,
-		    &p->p_addr->u_pcb)) {
-			KERNEL_UNLOCK();
+		if (cpu_pae == 0 &&
+		    pmap_exec_fixup(&p->p_vmspace->vm_map, frame, gdt_cs,
+		    &p->p_addr->u_pcb))
 			goto out;
-		}
 
 		sv.sival_int = frame->tf_eip;
 		trapsignal(p, SIGSEGV, type &~ T_USER, SEGV_MAPERR, sv);
-		KERNEL_UNLOCK();
 		goto out;
 
 	case T_TSSFLT|T_USER:
diff --git a/sys/arch/i386/include/segments.h b/sys/arch/i386/include/segments.h
index d74c27d307e..646c7b3d538 100644
--- a/sys/arch/i386/include/segments.h
+++ b/sys/arch/i386/include/segments.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: segments.h,v 1.27 2020/09/24 11:18:37 kettenis Exp $	*/
+/*	$OpenBSD: segments.h,v 1.28 2020/09/24 11:36:50 deraadt Exp $	*/
 /*	$NetBSD: segments.h,v 1.23 1996/02/01 22:31:03 mycroft Exp $	*/
 
 /*-
@@ -121,6 +121,7 @@ void setgate(struct gate_descriptor *, void *, int, int, int, int);
 void setregion(struct region_descriptor *, void *, size_t);
 void setsegment(struct segment_descriptor *, void *, size_t, int, int,
     int, int);
+void initcodesegment(struct segment_descriptor *);
 void unsetgate(struct gate_descriptor *);
 void cpu_init_idt(void);