Re: [PATCH] x86/shstk: Enable shadow stack for x32

[Edgecombe, Rick P]

On Fri, 2024-03-22 at 09:07 -0700, H.J. Lu wrote:
> > I mean it will require kernel work in the future to maintain
> > support.
> > That we will have to think about x32 effects when making other
> > shadow
> > stack changes.
> 
> It is way more than kernel SHSTK self tests.
> 
> > I'll paste my other comment in this thread:
> > 
> > The main usage of shadow stack is security, and comes with some
> > overhead. IIUC the main usage of x32 is performance benchmarking
> > type
> > stuff. Why would someone want to use shadow stack and x32 together?
> 
> Improve x32 security and user space IBT will add more improvement.

Please elaborate on the users that will use x32 and shadow stack
together. How many people are we talking about? They care enough about
performance to use x32, but also don't mind overhead to increase
security? Perhaps I'm missing something on what x32 is used for today.


[snip]

> > 
> > The mapping above 4G was because Peterz raised the possibility that
> > a
> > 64 bit process could far call into a 32 bit segment and start doing
> > signal stuff that would encounter undefined behavior. He wanted it
> > cleanly blocked. So by keeping the shadow stack above 4GB, existing
> > processes that turned on shadow stack would be preventing from
> > transitioning to 32 bit and encountering the missing 32 bit signal
> > support (because the CPU would #GP during the 32 bit transition if
> > SSP
> > is above 4GB).
> > 
> > Probably there is some interplay between the x32 mmap logic and
> > shadow
> > stacks mapping, where it then becomes possible to get below 4GB.
> > Since
> > x32 needs the shadow stack to be below 4GB, it's incompatible with
> > that
> > solution. So this patch is not sufficient to enable x32 without
> > side
> > effects that were previously considered bad.
> 
> Mapping shadow stack below 4GB on x32 still provides security
> improvements
> over no show stack.

Agreed on this point. I don't think x32 shadow stack has to be perfect
to improve security of the x32 apps.

But Peterz's concern (I think it could probably be re-opened) was that
the ia32 signal stuff should not be just declared unsupported with
shadow stack, but blocked from being used somehow. So it was really
about being able to not have to think about the implications of the
undefined behavior. (was my understanding at least)

This patch is just turning things on and finding that nothing crashes.
It needs more analysis.