Messages in this thread | | | Subject | Re: [PATCH 03/10] x86/cet: Signal handling for shadow stack | From | Florian Weimer <> | Date | Thu, 7 Jun 2018 20:58:56 +0200 |
| |
On 06/07/2018 08:30 PM, Andy Lutomirski wrote: > On Thu, Jun 7, 2018 at 7:41 AM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote: >> >> Set and restore shadow stack pointer for signals. > > How does this interact with siglongjmp()?
We plan to use some unused signal mask bits in the jump buffer (we have a lot of those in glibc for some reason) to store the shadow stack pointer.
> This patch makes me extremely nervous due to the possibility of ABI > issues and CRIU breakage. > >> diff --git a/arch/x86/include/uapi/asm/sigcontext.h b/arch/x86/include/uapi/asm/sigcontext.h >> index 844d60eb1882..6c8997a0156a 100644 >> --- a/arch/x86/include/uapi/asm/sigcontext.h >> +++ b/arch/x86/include/uapi/asm/sigcontext.h >> @@ -230,6 +230,7 @@ struct sigcontext_32 { >> __u32 fpstate; /* Zero when no FPU/extended context */ >> __u32 oldmask; >> __u32 cr2; >> + __u32 ssp; >> }; >> >> /* >> @@ -262,6 +263,7 @@ struct sigcontext_64 { >> __u64 trapno; >> __u64 oldmask; >> __u64 cr2; >> + __u64 ssp; >> >> /* >> * fpstate is really (struct _fpstate *) or (struct _xstate *) >> @@ -320,6 +322,7 @@ struct sigcontext { >> struct _fpstate __user *fpstate; >> __u32 oldmask; >> __u32 cr2; >> + __u32 ssp; > > Is it actually okay to modify these structures like this? They're > part of the user ABI, and I don't know whether any user code relies on > the size being constant.
Probably not. Historically, these things have been tacked at the end of the floating point state, see struct _xstate:
/* New processor state extensions go here: */
However, I'm not sure if this is really ideal because I doubt that everyone who needs the shadow stack pointer also wants to sacrifice space for the AVX-512 save area (which is already a backwards compatibility hazard). Other architectures have variable offsets and some TLV-style setup here.
Thanks, Florian
| |