kernel - Fix signal FP save/restore issues when AVX is enabled

* The kernel was not saving/restoring the full FP context when entering into
  or returning from a signal, leading to corrupt FP registers even when
  AVX is not used, when AVX is enabled in the kernel.

  ANY SIGNAL COULD CORRUPT THE FP STATE.

* Fixed by adjusting the on-user-stack fpsave area sizes and operation.

* This unfortunately changes a number of user visible structures.
  ucontext_t, mcontext_t, sigcontext, sigframe.

  It is POSSIBLE that most userland use cases will be unaffected, but I'm
  not holding my breath.

Major-Sleuthing-by: ftigeot
Testing-by: ftigeot, dillon

diff --git a/sys/cpu/x86_64/include/signal.h b/sys/cpu/x86_64/include/signal.h
index f0ae11f..2b43e38 100644 (file)
--- a/sys/cpu/x86_64/include/signal.h
+++ b/sys/cpu/x86_64/include/signal.h
@@ -93,14 +93,11 @@ struct      sigcontext {
        unsigned int    sc_fpformat;
        unsigned int    sc_ownedfp;
        unsigned int    sc_reserved;
-       unsigned int    sc_unused01;
-       unsigned int    sc_unused02;
+       unsigned int    sc_unused[8];
 
-       /* 16 byte aligned */
-
-       int             sc_fpregs[128];
-       int             __spare__[16];
-};
+       /* 64 byte aligned */
+       int             sc_fpregs[256]; /* 1024 bytes */
+} __attribute__((aligned(64)));
 
 #endif /* !_ANSI_SOURCE && !_POSIX_SOURCE */
 

diff --git a/sys/cpu/x86_64/include/ucontext.h b/sys/cpu/x86_64/include/ucontext.h
index f0df094..f19575b 100644 (file)
--- a/sys/cpu/x86_64/include/ucontext.h
+++ b/sys/cpu/x86_64/include/ucontext.h
@@ -33,7 +33,7 @@
 #ifndef _CPU_UCONTEXT_H_
 #define        _CPU_UCONTEXT_H_
 
-typedef struct __mcontext {
+struct __mcontext {
        /*
         * The first 20 fields must match the definition of
         * sigcontext. So that we can support sigcontext
@@ -73,18 +73,18 @@ typedef struct __mcontext {
        unsigned int    mc_fpformat;
        unsigned int    mc_ownedfp;
        unsigned int    mc_reserved;
-       unsigned int    mc_unused01;
-       unsigned int    mc_unused02;
+       unsigned int    mc_unused[8];
 
-       /* 16 byte aligned */
+       /* 64 byte aligned */
+       int             mc_fpregs[256]; /* 1024 bytes */
+} __attribute__((aligned(64)));
 
-       int             mc_fpregs[128];
-       int             __spare__[16];
-} mcontext_t;
+typedef struct __mcontext mcontext_t;
 
 #define _MC_FPFMT_NODEV                0x10000 /* device not present or configured */
 #define _MC_FPFMT_387          0x10001
 #define _MC_FPFMT_XMM          0x10002
+#define _MC_FPFMT_YMM          0x10003
 
 #define _MC_FPOWNED_NONE       0x20000 /* FP state not used */
 #define _MC_FPOWNED_FPU                0x20001 /* FP state came from FPU */

diff --git a/sys/platform/pc64/x86_64/machdep.c b/sys/platform/pc64/x86_64/machdep.c
index d1ba94f..681810a 100644 (file)
--- a/sys/platform/pc64/x86_64/machdep.c
+++ b/sys/platform/pc64/x86_64/machdep.c
@@ -482,7 +482,15 @@ sendsig(sig_t catcher, int sig, sigset_t *mask, u_long code)
                sp = (char *)regs->tf_rsp - sizeof(struct sigframe) - 128;
        }
 
-       /* Align to 16 bytes */
+       /*
+        * XXX AVX needs 64-byte alignment but sigframe has other fields and
+        * the embedded ucontext is not at the front, so aligning this won't
+        * help us.  Fortunately we bcopy in/out of the sigframe, so the
+        * kernel is ok.
+        *
+        * The problem though is if userland winds up trying to use the
+        * context directly.
+        */
        sfp = (struct sigframe *)((intptr_t)sp & ~(intptr_t)0xF);
 
        /* Translate the signal is appropriate */

diff --git a/sys/platform/pc64/x86_64/npx.c b/sys/platform/pc64/x86_64/npx.c
index 3150539..9361d32 100644 (file)
--- a/sys/platform/pc64/x86_64/npx.c
+++ b/sys/platform/pc64/x86_64/npx.c
@@ -445,13 +445,20 @@ npxpush(mcontext_t *mctx)
                } else {
                        mctx->mc_ownedfp = _MC_FPOWNED_PCB;
                }
-               bcopy(td->td_savefpu, mctx->mc_fpregs, sizeof(mctx->mc_fpregs));
+               KKASSERT(sizeof(*td->td_savefpu) <= sizeof(mctx->mc_fpregs));
+               bcopy(td->td_savefpu, mctx->mc_fpregs, sizeof(*td->td_savefpu));
                td->td_flags &= ~TDF_USINGFP;
-               mctx->mc_fpformat =
+#ifndef CPU_DISABLE_AVX
+       if (cpu_xsave)
+               mctx->mc_fpformat = _MC_FPFMT_YMM;
+       else
+#endif
 #ifndef CPU_DISABLE_SSE
-                       (cpu_fxsr) ? _MC_FPFMT_XMM :
+       if (cpu_fxsr)
+               mctx->mc_fpformat = _MC_FPFMT_XMM;
+       else
 #endif
-                       _MC_FPFMT_387;
+               mctx->mc_fpformat = _MC_FPFMT_387;
        } else {
                mctx->mc_ownedfp = _MC_FPOWNED_NONE;
                mctx->mc_fpformat = _MC_FPFMT_NODEV;
@@ -497,6 +504,7 @@ npxpop(mcontext_t *mctx)
                 */
                if (td == mdcpu->gd_npxthread)
                        npxsave(td->td_savefpu);
+               KKASSERT(sizeof(*td->td_savefpu) <= sizeof(mctx->mc_fpregs));
                bcopy(mctx->mc_fpregs, td->td_savefpu, sizeof(*td->td_savefpu));
                if ((td->td_savefpu->sv_xmm.sv_env.en_mxcsr & ~0xFFBF)
 #ifndef CPU_DISABLE_SSE