Use a critical region to protect pushing of the parent's npx state to the

pcb for fork().  It was possible for the state to be saved twice when an
interrupt handler saved it concurrently.  This corrupted (reset) the state
because fnsave has the (in)convenient side effect of doing an implicit
fninit.  Mundane null pointer bugs were not possible, because we save to
an "arbitrary" process's pcb and not to the "right" place (npxproc).

Push the parent's %gs to the pcb for fork().  Changes to %gs before
fork() were not preserved in the child unless an accidental context
switch did the pushing.  Updated the list of pcb contents which is
supposed to inhibit bugs like this.  pcb_dr*, pcb_gs and pcb_ext were
missing.  Copying is correct for pcb_dr*, and pcb_ext is already
handled specially (although XXX'ly).

Reducing the savectx() call to an npxsave() call in rev.1.80 was a
mistake.  The above bugs are duplicated in many places, including in
savectx() itself.

The arbitraryness of the parent process pointer for the fork()
subroutines, the pcb pointer for savectx(), and the save87 pointer
for npxsave(), is illusory.  These functions don't work "right" unless
the pointers are precisely curproc, curpcb, and the address of npxproc's
save87 area, respectively, although the special context in which they
are called allows savectx(&dumppcb) to sort of work and npxsave(&dummy)
to work.  cpu_fork() just doesn't work unless the parent process
pointer is curproc, or the caller has pushed %gs to the pcb, or %gs
happens to already be in the pcb.

1 files changed, 14 insertions, 4 deletions

diff --git a/sys/i386/i386/vm_machdep.c b/sys/i386/i386/vm_machdep.c
index 019c45a..fd626a3 100644
--- a/sys/i386/i386/vm_machdep.c
+++ b/sys/i386/i386/vm_machdep.c
@@ -121,6 +121,9 @@ cpu_fork(p1, p2, flags)
 	int flags;
 {
 	struct pcb *pcb2;
+#ifdef DEV_NPX
+	int savecrit;
+#endif
 
 	if ((flags & RFPROC) == 0) {
 		if ((flags & RFMEM) == 0) {
@@ -137,10 +140,14 @@ cpu_fork(p1, p2, flags)
 		return;
 	}
 
-#ifdef DEV_NPX
 	/* Ensure that p1's pcb is up to date. */
+#ifdef DEV_NPX
+	if (p1 == curproc)
+		p1->p_addr->u_pcb.pcb_gs = rgs();
+	savecrit = critical_enter();
 	if (PCPU_GET(npxproc) == p1)
 		npxsave(&p1->p_addr->u_pcb.pcb_savefpu);
+	critical_exit(savecrit);
 #endif
 
 	/* Copy p1's pcb. */
@@ -150,7 +157,7 @@ cpu_fork(p1, p2, flags)
 	/*
 	 * Create a new fresh stack for the new process.
 	 * Copy the trap frame for the return to user mode as if from a
-	 * syscall.  This copies the user mode register values.
+	 * syscall.  This copies most of the user mode register values.
 	 */
 	p2->p_md.md_regs = (struct trapframe *)
 			   ((int)p2->p_addr + UPAGES * PAGE_SIZE - 16) - 1;
@@ -171,11 +178,14 @@ cpu_fork(p1, p2, flags)
 	pcb2->pcb_esp = (int)p2->p_md.md_regs - sizeof(void *);
 	pcb2->pcb_ebx = (int)p2;		/* fork_trampoline argument */
 	pcb2->pcb_eip = (int)fork_trampoline;
-	/*
+	/*-
+	 * pcb2->pcb_dr*:	cloned above.
 	 * pcb2->pcb_ldt:	duplicated below, if necessary.
 	 * pcb2->pcb_savefpu:	cloned above.
-	 * pcb2->pcb_flags:	cloned above (always 0 here?).
+	 * pcb2->pcb_flags:	cloned above.
 	 * pcb2->pcb_onfault:	cloned above (always NULL here?).
+	 * pcb2->pcb_gs:	cloned above.
+	 * pcb2->pcb_ext:	cleared below.
 	 */
 
 	/*