On Thu, Jun 07, 2018 at 11:30:34AM -0700, Andy Lutomirski wrote:
> On Thu, Jun 7, 2018 at 7:41 AM Yu-cheng Yu <yu-cheng.yu@xxxxxxxxx> wrote:
> >
> > Set and restore shadow stack pointer for signals.
>
> How does this interact with siglongjmp()?
>
> 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.
For sure it might cause problems for CRIU since we have
similar definitions for this structure inside our code.
That said if kernel is about to modify the structures it
should keep backward compatibility at least if a user
passes previous version of a structure @ssp should be
set to something safe by the kernel itself.
I didn't read the whole series of patches in details
yet, hopefully will be able tomorrow. Thanks Andy for
CC'ing!
