On Fri, 2022-02-11 at 09:54 -0800, Andy Lutomirski wrote:
> > Like just using VM_SHADOW_STACK | VM_GROWSDOWN to get a regular
> > stack
> > sized gap? I think it could work. It also simplifies the mm-
> > >stack_vm
> > accounting.
>
> Seems not crazy. Do we want automatically growing shadow stacks? I
> don't really like the historical unix behavior where the main thread
> has
> a sort-of-infinite stack and every other thread has a fixed stack.
Ah! I was scratching my head why glibc did that - it's historical. Yea,
probably it's not needed and adding strange behavior.
>
> >
> > It would no longer get a gap at the end though. I don't think it's
> > needed.
> >
>
> I may have missed something about the oddball way the mm code works,
> but
> it seems if you have a gap at one end of every shadow stack, you
> automatically have a gap at the other end.
Right, we only need one, and this patch added a gap on both ends.
Per Andy's comment about the "oddball way" the mm code does the gaps -
The previous version of this (PROT_SHADOW_STACK) had an issue where if
you started with writable memory, then mprotect() with
PROT_SHADOW_STACK, the internal rb tree would get confused over the
sudden appearance of a gap. This new version follows closer to how
MAP_STACK avoids the problem I saw. But the way these guard gaps work
seem to barely avoid problems when you do things like split the vma by
mprotect()ing the middle of one. I wasn't sure if it's worth a
refactor. I guess the solution is quite old and there hasn't been
problems. I'm not even sure what the change would be, but it does feel
like adding to something fragile. Maybe shadow stack should just place
a guard page manually and not add any special shadow stack logic to the
mm code...
Other than that I'm inclined to remove the end gap and justify this
patch better in the commit log. Something like this (borrowing some
from Dave's comments):
The architecture of shadow stack constrains the ability of
userspace to move the shadow stack pointer (ssp) in order to
prevent corrupting or switching to other shadow stacks. The
RSTORSSP can move the spp to different shadow stacks, but it
requires a specially placed token in order to switch shadow
stacks. However, the architecture does not prevent
incrementing or decrementing shadow stack pointer to wander
onto an adjacent shadow stack. To prevent this in software,
enforce guard pages at the beginning of shadow stack vmas, such
that t
here will always be a gap between adjacent shadow stacks.
Make the gap big enough so that no userspace ssp changing
operations (besides RSTORSSP), can move the ssp from one stack
to the next. The ssp can increment or decrement by CALL, RET
and INCSSP. CALL and RET can move the ssp by a maximum of 8
bytes, at which point the shadow stack would be accessed.
The INCSSP instruction can also increment the shadow stack
pointer. It is the shadow stack analog of an instruction like:
addq $0x80, %rsp
However, there is one important difference between an ADD on
%rsp and INCSSP. In addition to modifying SSP, INCSSP also
reads from the memory of the first and last elements that were
"popped". It can be thought of as acting like this:
READ_ONCE(ssp); // read+discard top element on stack
ssp += nr_to_pop * 8; // move the shadow stack
READ_ONCE(ssp-8); // read+discard last popped stack element
The maximum distance INCSSP can move the ssp is 2040 bytes,
before it would read the memory. There for a single page gap
will be enough to prevent any operation from shifting the ssp
to an adjacent gap, before the shadow stack would be read and
cause a fault.
This could be accomplished by using VM_GROWSDOWN, but this has
two downsides.
1. VM_GROWSDOWN will have a 1MB gap which is on the large
side for 32 bit address spaces
2. The behavior would allow shadow stack's to grow, which
is unneeded and adds a strange difference to how most
regular stacks work.
