hurd: handling pending signals could result in corruption of FPU state

Handling a pending signal calls _hurd_setup_sighandler() once again
after the initial signal handling. In this case a pointer to the
previous sigcontext is available to supply the interrupted thread's
original basic state, fpu state and fpu XSTATE. The original XSTATE
was not being preserved by the pending signal but instead overwritten
with the active XSTATE. XSTATE register values modified by the
signal handling code could therefore be wrongly propogated back to
the interrupted user code.

diff --git a/sysdeps/mach/hurd/i386/bits/sigcontext.h b/sysdeps/mach/hurd/i386/bits/sigcontext.h
index eefc5bbeb8e74e1bf2f0e0a3074949a73b61ed82..0e12e26da9ce1d9cc96c77e83431d951b86af9dc 100644 (file)
--- a/sysdeps/mach/hurd/i386/bits/sigcontext.h
+++ b/sysdeps/mach/hurd/i386/bits/sigcontext.h
@@ -90,6 +90,7 @@ struct sigcontext
     int sc_fpexcsr;            /* FPSR including exception bits.  */
 
     struct i386_xfloat_state *xstate;
+    size_t xstate_size;
   };
 
 /* Traditional BSD names for some members.  */

diff --git a/sysdeps/mach/hurd/x86/trampoline.c b/sysdeps/mach/hurd/x86/trampoline.c
index d1c30fbb493d77d94a4bb49db7a4766591489c64..e511481b48e12f94d7050d5cfaa1f416283e786f 100644 (file)
--- a/sysdeps/mach/hurd/x86/trampoline.c
+++ b/sysdeps/mach/hurd/x86/trampoline.c
@@ -151,9 +151,9 @@ _hurd_setup_sighandler (struct hurd_sigstate *ss, const struct sigaction *action
       ucontext_t ucontext;
       siginfo_t siginfo;
 #ifdef __x86_64__
-      char _pad2[56];
+      char _pad2[48];
 #else
-      char _pad2[20];
+      char _pad2[16];
 #endif
       char xstate[];
       /* Don't add anything after xstate, as it's dynamically
@@ -170,29 +170,30 @@ _hurd_setup_sighandler (struct hurd_sigstate *ss, const struct sigaction *action
       /* We have a previous sigcontext that sigreturn was about
         to restore when another signal arrived.  We will just base
         our setup on that.  */
-      if (! _hurdsig_catch_memory_fault (ss->context))
-       {
-         memcpy (&state->basic, &ss->context->sc_i386_thread_state,
-                 sizeof (state->basic));
-         memcpy (&state->fpu, &ss->context->sc_i386_float_state,
-                 sizeof (state->fpu));
-         state->set |= (1 << i386_REGS_SEGS_STATE) | (1 << i386_FLOAT_STATE);
-       }
-    }
-
-  if (! machine_get_basic_state (ss->thread, state))
-    return NULL;
+      memcpy (&state->basic, &ss->context->sc_i386_thread_state,
+             sizeof (state->basic));
+      memcpy (&state->fpu, &ss->context->sc_i386_float_state,
+             sizeof (state->fpu));
+      state->set |= (1 << i386_REGS_SEGS_STATE) | (1 << i386_FLOAT_STATE);
 
-  /* Initialize the size of the CPU extended state, to be saved during
-   * signal handling */
+      xstate_size = ss->context->xstate_size;
+    }
+  else
+    {
+      /* Initialize the size of the CPU extended state, to be saved during
+       * signal handling */
 #ifdef i386_XFLOAT_STATE
-  _Static_assert ((sizeof(*stackframe) + sizeof(struct i386_xfloat_state)) % 64 == 0,
-                  "stackframe size must be multiple of 64-byte minus "
-                  "sizeof(struct i386_xfloat_state), please adjust _pad2");
+      _Static_assert ((sizeof(*stackframe) + sizeof(struct i386_xfloat_state)) % 64 == 0,
+                     "stackframe size must be multiple of 64-byte minus "
+                     "sizeof(struct i386_xfloat_state), please adjust _pad2");
 
-  if (__i386_get_xstate_size(__mach_host_self(), &xstate_size))
+      if (__i386_get_xstate_size(__mach_host_self(), &xstate_size))
 #endif
-    xstate_size = 0;
+       xstate_size = 0;
+    }
+
+  if (! machine_get_basic_state (ss->thread, state))
+    return NULL;
 
   /* Save the original SP in the gratuitous `esp' slot.
      We may need to reset the SP (the `uesp' slot) to avoid clobbering an
@@ -279,33 +280,51 @@ _hurd_setup_sighandler (struct hurd_sigstate *ss, const struct sigaction *action
       memcpy (&scp->sc_i386_thread_state,
              &state->basic, sizeof (state->basic));
 
+      scp->xstate_size = 0;
       scp->xstate = NULL;
 #ifdef i386_XFLOAT_STATE
       if (xstate_size > 0)
         {
-          mach_msg_type_number_t got = (xstate_size / sizeof (int));
+         if (ss->context != NULL)
+           {
+             assert (ss->context->xstate != NULL);
 
-          ok = (! __thread_get_state (ss->thread, i386_XFLOAT_STATE,
-                                      (thread_state_t) stackframe->xstate, &got)
-                && got == (xstate_size / sizeof (int)));
+             /* Copy the xstate preserved at the time of handling the first
+                signal rather than that currently in the FPU. */
+             memcpy (stackframe->xstate, ss->context->xstate, xstate_size);
+             ok = 1;
+           }
+         else
+           {
+             mach_msg_type_number_t got = (xstate_size / sizeof (int));
 
-         if (ok && ((struct i386_xfloat_state*) stackframe->xstate)->fp_save_kind > 5)
-           /* We support up to XSAVES */
-           ok = 0;
+             ok = (! __thread_get_state (ss->thread, i386_XFLOAT_STATE,
+                                         (thread_state_t) stackframe->xstate, &got)
+                   && got == (xstate_size / sizeof (int)));
 
-          if (ok)
+             if (ok && ((struct i386_xfloat_state*) stackframe->xstate)->fp_save_kind > 5)
+               /* We support up to XSAVES */
+               ok = 0;
+           }
+
+         if (ok)
            {
              scp->xstate = (struct i386_xfloat_state*) stackframe->xstate;
+             scp->xstate_size = xstate_size;
              assert((uintptr_t)scp->xstate->hw_state % 64 == 0);
            }
-        }
+       }
       else
 #endif
         ok = 0;
       if (!ok)
         {
           /* struct sigcontext is laid out so that starting at sc_fpkind mimics
-            a struct i386_float_state.  */
+            a struct i386_float_state. In the event that we are processing a
+           previous sigcontext (ss->context != NULL) 'state' correctly contains
+           the FPU state saved from the previous handler (see memcpy above)
+           rather than that currently in the FPU */
+
           _Static_assert (offsetof (struct sigcontext, sc_i386_float_state)
                          % __alignof__ (struct i386_float_state) == 0,
                          "sc_i386_float_state layout mismatch");

diff --git a/sysdeps/mach/hurd/x86_64/bits/sigcontext.h b/sysdeps/mach/hurd/x86_64/bits/sigcontext.h
index 94061e7a910cef984c38d89d21c6369e4fe1970b..770523949caa6d88ed493c21ca1e8241d5e9d7af 100644 (file)
--- a/sysdeps/mach/hurd/x86_64/bits/sigcontext.h
+++ b/sysdeps/mach/hurd/x86_64/bits/sigcontext.h
@@ -98,6 +98,7 @@ struct sigcontext
     int sc_fpexcsr;            /* FPSR including exception bits.  */
 
     struct i386_xfloat_state *xstate;
+    size_t xstate_size;
   };
 
 /* Traditional BSD names for some members.  */