Re: [issue871] gtk2 related: X mouse pointer jumps and sticks to top left corner

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

    Ok, it looks like firefox is messing up the signal stack and this
    caused a GP fault in the kernel due to reserved bits in the MXCSR
    field in the floating point saved state being set by userland.

    I have added code to report and clear the bits.  I do not know why
    firefox is blowing up the state, though.

    I also fixed a bug in krateprintf() in kern/subr_prf.c (not included)...
    I committed that fix directly.

    PATCH #2 enclosed.

					-Matt
					Matthew Dillon 
					<dillon@backplane.com>

Index: platform/pc32/i386/machdep.c
===================================================================
RCS file: /cvs/src/sys/platform/pc32/i386/machdep.c,v
retrieving revision 1.128
diff -u -p -r1.128 machdep.c
--- platform/pc32/i386/machdep.c	7 Nov 2007 17:42:50 -0000	1.128
+++ platform/pc32/i386/machdep.c	7 Dec 2007 02:56:39 -0000
@@ -498,6 +498,11 @@ 		tf->tf_eflags &= ~(PSL_VM | PSL_NT | P
 	}
 
 	/*
+	 * Save the FPU state and reinit the FP unit
+	 */
+	npxpush(&sf.sf_uc.uc_mcontext);
+
+	/*
 	 * Copy the sigframe out to the user's stack.
 	 */
 	if (copyout(&sf, sfp, sizeof(struct sigframe)) != 0) {
@@ -679,6 +684,11 @@ 		bcopy(&ucp->uc_mcontext.mc_gs, regs, s
 	}
 
 	/*
+	 * Restore the FPU state from the frame
+	 */
+	npxpop(&ucp->uc_mcontext);
+
+	/*
 	 * Merge saved signal mailbox pending flag to maintain interlock
 	 * semantics against system calls.
 	 */
Index: platform/pc32/include/md_var.h
===================================================================
RCS file: /cvs/src/sys/platform/pc32/include/md_var.h,v
retrieving revision 1.25
diff -u -p -r1.25 md_var.h
--- platform/pc32/include/md_var.h	9 Jan 2007 23:34:03 -0000	1.25
+++ platform/pc32/include/md_var.h	7 Dec 2007 01:15:37 -0000
@@ -69,6 +69,7 @@ struct	fpreg;
 struct  dbreg;
 struct  mdglobaldata;
 struct  thread;
+struct	__mcontext;
 
 void	busdma_swi (void);
 void	cpu_gdinit (struct mdglobaldata *gd, int cpu);
@@ -113,5 +114,7 @@ 		    int selec);
 void	userconfig (void);
 int     user_dbreg_trap (void);
 int     npxdna(void);
+void	npxpush(struct __mcontext *mctx);
+void	npxpop(struct __mcontext *mctx);
 
 #endif /* !_MACHINE_MD_VAR_H_ */
Index: platform/pc32/isa/npx.c
===================================================================
RCS file: /cvs/src/sys/platform/pc32/isa/npx.c,v
retrieving revision 1.42
diff -u -p -r1.42 npx.c
--- platform/pc32/isa/npx.c	22 Feb 2007 15:50:49 -0000	1.42
+++ platform/pc32/isa/npx.c	8 Dec 2007 19:56:47 -0000
@@ -210,6 +210,8 @@ 	iret							\n\
 ");
 #endif /* SMP */
 
+static struct krate badfprate = { 1 };
+
 /*
  * Probe routine.  Initialize cr0 to give correct behaviour for [f]wait
  * whether the device exists or not (XXX should be elsewhere).  Set flags
@@ -513,7 +515,7 @@  */
 void
 npxinit(u_short control)
 {
-	static union savefpu dummy;
+	static union savefpu dummy __aligned(16);
 
 	if (!npx_exists)
 		return;
@@ -852,15 +854,29 @@  */
 int
 npxdna(void)
 {
+	thread_t td = curthread;
 	u_long *exstat;
+	int didinit = 0;
 
 	if (!npx_exists)
 		return (0);
 	if (mdcpu->gd_npxthread != NULL) {
 		kprintf("npxdna: npxthread = %p, curthread = %p\n",
-		       mdcpu->gd_npxthread, curthread);
+		       mdcpu->gd_npxthread, td);
 		panic("npxdna");
 	}
+
+	/*
+	 * Setup the initial saved state if the thread has never before
+	 * used the FP unit.  This also occurs when a thread pushes a
+	 * signal handler and uses FP in the handler.
+	 */
+	if ((td->td_flags & TDF_USINGFP) == 0) {
+		td->td_flags |= TDF_USINGFP;
+		npxinit(__INITIAL_NPXCW__);
+		didinit = 1;
+	}
+
 	/*
 	 * The setting of gd_npxthread and the call to fpurstor() must not
 	 * be preempted by an interrupt thread or we will take an npxdna
@@ -873,8 +889,8 @@ 	stop_emulating();
 	/*
 	 * Record new context early in case frstor causes an IRQ13.
 	 */
-	mdcpu->gd_npxthread = curthread;
-	exstat = GET_FPU_EXSW_PTR(curthread);
+	mdcpu->gd_npxthread = td;
+	exstat = GET_FPU_EXSW_PTR(td);
 	*exstat = 0;
 	/*
 	 * The following frstor may cause an IRQ13 when the state being
@@ -888,7 +904,13 @@ 	 * 386/Cyrix 387 system, fnclex works c
 	 * fnsave are broken, so our treatment breaks fnclex if it is the
 	 * first FPU instruction after a context switch.
 	 */
-	fpurstor(curthread->td_savefpu);
+	if (td->td_savefpu->sv_xmm.sv_env.en_mxcsr & ~0xFFBF) {
+		krateprintf(&badfprate,
+			    "FXRSTR: illegal FP MXCSR %08x didinit = %d\n",
+			    td->td_savefpu->sv_xmm.sv_env.en_mxcsr, didinit);
+		td->td_savefpu->sv_xmm.sv_env.en_mxcsr &= 0xFFBF;
+	}
+	fpurstor(td->td_savefpu);
 	crit_exit();
 
 	return (1);
@@ -975,6 +997,90 @@ #endif
 		fnsave(addr);
 }
 
+/*
+ * Save the FP state to the mcontext structure.
+ *
+ * WARNING: If you want to try to npxsave() directly to mctx->mc_fpregs,
+ * then it MUST be 16-byte aligned.  Currently this is not guarenteed.
+ */
+void
+npxpush(mcontext_t *mctx)
+{
+	thread_t td = curthread;
+
+	if (td->td_flags & TDF_USINGFP) {
+		if (mdcpu->gd_npxthread == td) {
+			/*
+			 * XXX Note: This is a bit inefficient if the signal
+			 * handler uses floating point, extra faults will
+			 * occur.
+			 */
+			mctx->mc_ownedfp = _MC_FPOWNED_FPU;
+			npxsave(td->td_savefpu);
+		} else {
+			mctx->mc_ownedfp = _MC_FPOWNED_PCB;
+		}
+		bcopy(td->td_savefpu, mctx->mc_fpregs, sizeof(mctx->mc_fpregs));
+		td->td_flags &= ~TDF_USINGFP;
+	} else {
+		mctx->mc_ownedfp = _MC_FPOWNED_NONE;
+	}
+}
+
+/*
+ * Restore the FP state from the mcontext structure.
+ */
+void
+npxpop(mcontext_t *mctx)
+{
+	thread_t td = curthread;
+
+	switch(mctx->mc_ownedfp) {
+	case _MC_FPOWNED_NONE:
+		/*
+		 * If the signal handler used the FP unit but the interrupted
+		 * code did not, release the FP unit.  Clear TDF_USINGFP will
+		 * force the FP unit to reinit so the interrupted code sees
+		 * a clean slate.
+		 */
+		if (td->td_flags & TDF_USINGFP) {
+			if (td == mdcpu->gd_npxthread)
+				npxsave(td->td_savefpu);
+			td->td_flags &= ~TDF_USINGFP;
+		}
+		break;
+	case _MC_FPOWNED_FPU:
+	case _MC_FPOWNED_PCB:
+		/*
+		 * Clear ownership of the FP unit and restore our saved state.
+		 *
+		 * NOTE: The signal handler may have set-up some FP state and
+		 * enabled the FP unit, so we have to restore no matter what.
+		 *
+		 * XXX: This is bit inefficient, if the code being returned
+		 * to is actively using the FP this results in multiple
+		 * kernel faults.
+		 *
+		 * WARNING: The saved state was exposed to userland and may
+		 * have to be sanitized to avoid a GP fault in the kernel.
+		 */
+		if (td == mdcpu->gd_npxthread)
+			npxsave(td->td_savefpu);
+		bcopy(mctx->mc_fpregs, td->td_savefpu, sizeof(*td->td_savefpu));
+		if (td->td_savefpu->sv_xmm.sv_env.en_mxcsr & ~0xFFBF) {
+			krateprintf(&badfprate,
+				    "pid %d (%s) signal return from user: "
+				    "illegal FP MXCSR %08x\n",
+				    td->td_proc->p_pid,
+				    td->td_proc->p_comm,
+				    td->td_savefpu->sv_xmm.sv_env.en_mxcsr);
+			td->td_savefpu->sv_xmm.sv_env.en_mxcsr &= 0xFFBF;
+		}
+		td->td_flags |= TDF_USINGFP;
+		break;
+	}
+}
+
 #ifndef CPU_DISABLE_SSE
 /*
  * On AuthenticAMD processors, the fxrstor instruction does not restore
Index: sys/thread.h
===================================================================
RCS file: /cvs/src/sys/sys/thread.h,v
retrieving revision 1.89
diff -u -p -r1.89 thread.h
--- sys/thread.h	18 Nov 2007 09:53:19 -0000	1.89
+++ sys/thread.h	7 Dec 2007 01:12:37 -0000
@@ -284,6 +284,7 @@ #define TDF_PANICWARN		0x00080000	/* pan
 #define TDF_BLOCKQ		0x00100000	/* on block queue */
 #define TDF_MPSAFE		0x00200000	/* (thread creation) */
 #define TDF_EXITING		0x00400000	/* thread exiting */
+#define TDF_USINGFP		0x00800000	/* thread using fp coproc */
 
 /*
  * Thread priorities.  Typically only one thread from any given