Re: [PATCH v9 23/42] Documentation/x86: Add CET shadow stack description

From: Edgecombe, Rick P
Date: Thu Jun 22 2023 - 12:53:37 EST


On Thu, 2023-06-22 at 08:40 +0100, szabolcs.nagy@xxxxxxx wrote:
> The 06/21/2023 16:02, H.J. Lu wrote:
> > On Wed, Jun 21, 2023 at 11:54 AM Edgecombe, Rick P
> > <rick.p.edgecombe@xxxxxxxxx> wrote:
> > > HJ, can you link your patch that makes it extensible and we can
> > > clear
> > > this up? Maybe the issue extends beyond fnon-call-exceptions, but
> > > that
> > > is where I reproduced it.
> >
> > Here is the patch:
> >
> > https://gitlab.com/x86-gcc/gcc/-/commit/aab4c24b67b5f05b72e52a3eaae005c2277710b9
>
> ok i don't see how this is related to fnon-call-exceptions..

I don't know what to tell you. I'm not a compiler expert, but a simple
fnon-call-exceptions test was how I reproduced it. If there is some
other use case for throwing an exception out of a signal handler, I
don't see how it makes fnon-call-exceptions unrelated.

>
> but it is wrong if the shstk is ever discontinous even though the
> shstk format would allow that. this was my original point in this
> thread: with current unwinder code you cannot add altshstk later.
> and no, introducing new binary markings and rebuild the world just
> for altshstk is not OK. so we have to decide now if we want alt
> shstk in the future or not, i gave reasons why we would want it.

The point was that the old GCCs restrict expanding the shadow stack
signal frame, which is a prerequisite to supporting alt shadow stacks.

So the existing userspace prevents us from supporting regular alt
shadow stack, before we even get to fancy unwinding alt shadow stack.
Some kind of ABI opt in will likely be required. Maybe a new elf bit,
at which point we can take advantage of what we learned.

You previously said:

On Wed, 2023-06-21 at 12:36 +0100, szabolcs.nagy@xxxxxxx wrote:
> as far as i can tell the current unwinder handles shstk unwinding
> correctly across signal handlers (sync or async and
> cleanup/exceptions
> handlers too), i see no issue with "fixed shadow stack signal frame
> size of 8 bytes" other than future extensions and discontinous shstk.

I took that to mean that you didn't see how the the existing unwinder
prevented alt shadow stacks. Hopefully we're all on the same page now. 

BTW, when alt shadow stack's were POCed, I hadn't encountered this GCC
behavior yet. So it was assumed it could be bolted on later without
disturbing anything. If Linus or someone wants to say we're ok with
breaking these old GCCs in this way, the first thing I would do would
be to pad the shadow stack signal frame with room for alt shadow stack
and more. I actually have a patch to do this, but alas we are already
pushing it regression wise.