Re: [OPTIONAL/RFC v2 39/39] x86: Add alt shadow stack support

[Edgecombe, Rick P]

On Mon, 2022-10-03 at 16:21 -0700, Andy Lutomirski wrote:
> On 9/29/22 15:29, Rick Edgecombe wrote:
> > To handle stack overflows, applications can register a separate
> > signal alt
> > stack to use for the stack to handle signals. To handle shadow
> > stack
> > overflows the kernel can similarly provide the ability to have an
> > alt
> > shadow stack.
> 
> 
> The overall SHSTK mechanism has a concept of a shadow stack that is 
> valid and not in use and a shadow stack that is in use.  This is
> used, 
> for example, by RSTORSSP.  I would like to imagine that this serves
> a 
> real purpose (presumably preventing two different threads from using
> the 
> same shadow stack and thus corrupting each others' state).
> 
> So maybe altshstk should use exactly the same mechanism.  Either
> signal 
> delivery should do the atomic very-and-mark-busy routine or
> registering 
> the stack as an altstack should do it.
> 
> I think your patch has this maybe 1/3 implemented

I'm not following how it breaks down into 3 parts, so hopefully I'm not
missing something. We could do a software busy bit for the token at the
end of alt shstk though. It seems like a good idea.

The busy-like bit in the RSTORSSP-type token is not called out as a
busy bit, but instead defined as reserved (must be 0) in some states.
(Note, it is different than the supervisor shadow stack format). Yea,
we could just probably use it like RSTORSSP does for this operation.

Or just invent another new token format and stay away from bits marked
reserved. Then it wouldn't have to be atomic either, since userspace
couldn't use it.

> , but I don't see any 
> atomics, and you seem to have removed (?) the code that actually 
> modifies the token on the stack.

The past series didn't do any busy bit like operation. The token just
marked where the sigreturn should be called. There was actually a
similar problem to what you described above, in that the token marking
the sigreturn point could have been usable by RSTORSSP from another
thread. In this version (even back in the non-RFC patches) using a made
up token format that RSTORSSP knows nothing about, avoids this a
different way than a busy bit. Two threads couldn't use a shstk
sigframe at the same time unless they somehow were already using the
same shadow stack.

> 
> >    
> > +static bool on_alt_shstk(unsigned long ssp)
> > +{
> > +     unsigned long alt_ss_start = current->thread.sas_shstk_sp;
> > +     unsigned long alt_ss_end = alt_ss_start + current-
> > >thread.sas_shstk_size;
> > +
> > +     return ssp >= alt_ss_start && ssp < alt_ss_end;
> > +}
> 
> We're forcing AUTODISARM behavior (right?), so I don't think this is 
> needed at all.  User code is never "on the alt stack".  It's either
> "on 
> the alt stack but the alt stack is disarmed, so it's not on the alt 
> stack" or it's just straight up not on the alt stack.

Err, right. This can be dropped. Thanks.