From: Mike Kelly Date: Mon, 2 Feb 2026 07:25:02 +0000 (+0000) Subject: hurd: handling pending signals could result in corruption of FPU state X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=826484111a81295056a98507a493876cd31cb21d;p=thirdparty%2Fglibc.git 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 eefc5bbeb8..0e12e26da9 100644 --- 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 d1c30fbb49..e511481b48 100644 --- 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 94061e7a91..770523949c 100644 --- 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. */