[off-list ref],Dave Martin [off-list ref],Weijiang Yang [off-list ref],Pengfei Xu [off-list ref],Haitao Huang [off-list ref]
From: "H. Peter Anvin" <hpa@zytor.com>
Message-ID: [off-list ref]

There are a few words at the end of the FXSAVE area for software use for this reason: so we can extend the signal frame – originally to enable saving the XSAVE state but that doesn't use up the whole software available area.

On May 2, 2021 4:23:49 PM PDT, Andy Lutomirski [off-list ref] wrote:

On Fri, Apr 30, 2021 at 10:47 AM Andy Lutomirski [off-list ref]
wrote:

quoted

On Fri, Apr 30, 2021 at 10:00 AM Yu, Yu-cheng [off-list ref]

wrote:

quoted

quoted

On 4/28/2021 4:03 PM, Andy Lutomirski wrote:

quoted

On Tue, Apr 27, 2021 at 1:44 PM Yu-cheng Yu

[off-list ref] wrote:

quoted

quoted

quoted

quoted

When shadow stack is enabled, a task's shadow stack states must

be saved

quoted

quoted

quoted

quoted

along with the signal context and later restored in sigreturn. 

However,

quoted

quoted

quoted

quoted

currently there is no systematic facility for extending a signal

context.

quoted

quoted

quoted

quoted

There is some space left in the ucontext, but changing ucontext

is likely

quoted

quoted

quoted

quoted

to create compatibility issues and there is not enough space for

further

quoted

quoted

quoted

quoted

extensions.

Introduce a signal context extension struct 'sc_ext', which is

used to save

quoted

quoted

quoted

quoted

shadow stack restore token address.  The extension is located

above the fpu

quoted

quoted

quoted

quoted

states, plus alignment.  The struct can be extended (such as the

ibt's

quoted

quoted

quoted

quoted

wait_endbr status to be introduced later), and sc_ext.total_size

field

quoted

quoted

quoted

quoted

keeps track of total size.

I still don't like this.

Here's how the signal layout works, for better or for worse:

The kernel has:

struct rt_sigframe {
     char __user *pretcode;
     struct ucontext uc;
     struct siginfo info;
     /* fp state follows here */
};

This is roughly the actual signal frame.  But userspace does not

have

quoted

quoted

quoted

this struct declared, and user code does not know the sizes of

the

quoted

quoted

quoted

fields.  So it's accessed in a nonsensical way.  The signal

handler

quoted

quoted

quoted

function is passed a pointer to the whole sigframe implicitly in

RSP,

quoted

quoted

quoted

a pointer to &frame->info in RSI, anda pointer to &frame->uc in

RDX.

quoted

quoted

quoted

User code can *find* the fp state by following a pointer from
mcontext, which is, in turn, found via uc:

struct ucontext {
     unsigned long      uc_flags;
     struct ucontext  *uc_link;
     stack_t          uc_stack;
     struct sigcontext uc_mcontext;  <-- fp pointer is in here
     sigset_t      uc_sigmask;    /* mask last for extensibility

*/

quoted

quoted

quoted

};

The kernel, in sigreturn, works a bit differently.  The sigreturn
variants know the base address of the frame but don't have the

benefit

quoted

quoted

quoted

of receiving pointers to the fields.  So instead the kernel takes
advantage of the fact that it knows the offset to uc and parses

uc

quoted

quoted

quoted

accordingly.  And the kernel follows the pointer in mcontext to

find

quoted

quoted

quoted

the fp state.  The latter bit is quite important later.  The

kernel

quoted

quoted

quoted

does not parse info at all.

The fp state is its own mess.  When XSAVE happened, Intel kindly

(?)

quoted

quoted

quoted

gave us a software defined area between the "legacy" x87 region

and

quoted

quoted

quoted

the modern supposedly extensible part.  Linux sticks the

following

quoted

quoted

quoted

structure in that hole:

struct _fpx_sw_bytes {
     /*
      * If set to FP_XSTATE_MAGIC1 then this is an xstate

context.

quoted

quoted

quoted

      * 0 if a legacy frame.
      */
     __u32                magic1;

     /*
      * Total size of the fpstate area:
      *
      *  - if magic1 == 0 then it's sizeof(struct _fpstate)
      *  - if magic1 == FP_XSTATE_MAGIC1 then it's sizeof(struct

_xstate)

quoted

quoted

quoted

      *    plus extensions (if any)
      */
     __u32                extended_size;

     /*
      * Feature bit mask (including FP/SSE/extended state) that

is present

quoted

quoted

quoted

      * in the memory layout:
      */
     __u64                xfeatures;

     /*
      * Actual XSAVE state size, based on the xfeatures saved in

the layout.

quoted

quoted

quoted

      * 'extended_size' is greater than 'xstate_size':
      */
     __u32                xstate_size;

     /* For future use: */
     __u32                padding[7];
};


That's where we are right now upstream.  The kernel has a parser

for

quoted

quoted

quoted

the FPU state that is bugs piled upon bugs and is going to have

to be

quoted

quoted

quoted

rewritten sometime soon.  On top of all this, we have two

upcoming

quoted

quoted

quoted

features, both of which require different kinds of extensions:

1. AVX-512.  (Yeah, you thought this story was over a few years

ago,

quoted

quoted

quoted

but no.  And AMX makes it worse.)  To make a long story short, we
promised user code many years ago that a signal frame fit in 2048
bytes with some room to spare.  With AVX-512 this is false.  With

AMX

quoted

quoted

quoted

it's so wrong it's not even funny.  The only way out of the mess
anyone has come up with involves making the length of the FPU

state

quoted

quoted

quoted

vary depending on which features are INIT, i.e. making it more

compact

quoted

quoted

quoted

than "compact" mode is.  This has a side effect: it's no longer
possible to modify the state in place, because enabling a feature

with

quoted

quoted

quoted

no space allocated will make the structure bigger, and the stack

won't

quoted

quoted

quoted

have room.  Fortunately, one can relocate the entire FPU state,

update

quoted

quoted

quoted

the pointer in mcontext, and the kernel will happily follow the
pointer.  So new code on a new kernel using a super-compact state
could expand the state by allocating new memory (on the heap?

very

quoted

quoted

quoted

awkwardly on the stack?) and changing the pointer.  For all we

know,

quoted

quoted

quoted

some code already fiddles with the pointer.  This is great,

except

quoted

quoted

quoted

that your patch sticks more data at the end of the FPU block that

no

quoted

quoted

quoted

one is expecting, and your sigreturn code follows that pointer,

and

quoted

quoted

quoted

will read off into lala land.

Then, what about we don't do that at all.  Is it possible from now

on we

quoted

quoted

don't stick more data at the end, and take the relocating-fpu

approach?

quoted

quoted

quoted

2. CET.  CET wants us to find a few more bytes somewhere, and

those

quoted

quoted

quoted

bytes logically belong in ucontext, and here we are.

Fortunately, we can spare CET the need of ucontext extension.  When

the

quoted

quoted

kernel handles sigreturn, the user-mode shadow stack pointer is

right at

quoted

quoted

the restore token.  There is no need to put that in ucontext.

That seems entirely reasonable.  This might also avoid needing to
teach CRIU about CET at all.

Wait, what's the actual shadow stack token format?  And is the token
on the new stack or the old stack when sigaltstack is in use?  For
that matter, is there any support for an alternate shadow stack for
signals?

--Andy

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.