On Wed, Jun 21, 2023 at 11:54 AM Edgecombe, Rick P
[off-list ref] wrote:

On Wed, 2023-06-21 at 12:36 +0100, szabolcs.nagy@arm.com wrote:

quoted

quoted

The 06/20/2023 19:34, Edgecombe, Rick P wrote:

quoted

quoted

On Tue, 2023-06-20 at 10:17 +0100, szabolcs.nagy@arm.com wrote:

quoted

quoted

if there is a fix that's good, i haven't seen it.

my point was that the current unwinder works with current
kernel
patches, but does not allow future extensions which
prevents
sigaltshstk to work. the unwinder is not versioned so this
cannot
be fixed later. it only works if distros ensure shstk is
disabled
until the unwinder is fixed. (however there is no way to
detect
old unwinder if somebody builds gcc from source.)

This is a problem the kernel is having to deal with, not
causing. > > The
userspace changes were upstreamed before the kernel. Userspace

quoted

quoted

folks

are adamantly against moving to a new elf bit, to start over
with a
clean slate. I tried everything to influence this and was not
successful. So I'm still not sure what the proposal here is for
the
kernel.

i agree, the glibc and libgcc patches should not have been accepted
before a linux abi.

but the other direction also holds: the linux patches should not be
pushed before the userspace design is discussed. (the current code
upstream is wrong, and new code for the proposed linux abi is not
posted yet. this is not your fault, i'm saying it here, because the
discussion is here.)

This series has been discussed with glibc/gcc developers regularly
throughout the enabling effort. In fact there have been ongoing
discussions about future shadow stack functionality.

It's not like this feature has been a fast or hidden effort. You are
just walking into the tail end of it. (much of it predates my
involvement BTW, including the initial glibc support)

AFAIK HJ presented the enabling changes at some glibc meeting. The
signal side of glibc is unchanged from what is already upstream. So I'm
not sure characterizing it that way is fair. It seems you were not part
of those old discussions, but that might be because your interest is
new. In any case we are constrained by some of these earlier outcomes.
More on that below.

quoted

quoted

quoted

quoted

I am guessing that the fnon-call-exceptions/expanded frame size
incompatibilities could end up causing something to grow an
opt-in > > at
some point.

there are independent userspace components and not every component
has a chance to opt-in.

quoted

quoted

quoted

quoted

how does "fixed shadow stack signal frame size" relates to
"-fnon-call-exceptions"?

if there were instruction boundaries within a function
where the
ret addr is not yet pushed or already poped from the shstk
then
the flag would be relevant, but since push/pop happens
atomically
at function entry/return -fnon-call-exceptions makes no
difference as far as shstk unwinding is concerned.

As I said, the existing unwinding code for fnon-call-
excecptions
assumes a fixed shadow stack signal frame size of 8 bytes.
Since > > the
exception is thrown out of a signal, it needs to know how to
unwind
through the shadow stack signal frame.

sorry but there is some misunderstanding about -fnon-call-
exceptions.

it is for emitting cleanup and exception handler data for a
function
such that throwing from certain instructions within that function
works, while normally only throwing from calls work.

it is not about *unwinding* from an async signal handler, which is
-fasynchronous-unwind-tables and should always work on linux, nor
for
dealing with cleanup/exception handlers above the interrupted frame
(likewise it works on linux without special cflags).

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.

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

quoted

quoted

quoted

quoted

quoted

quoted

there is no magic, longjmp should be implemented as:

        target_ssp = read from jmpbuf;
        current_ssp = read ssp;
        for (p = target_ssp; p != current_ssp; p--) {
                if (*p == restore-token) {
                        // target_ssp is on a different
shstk.
                        switch_shstk_to(p);
                        break;
                }
        }
        for (; p != target_ssp; p++)
                // ssp is now on the same shstk as target.
                inc_ssp();

this is what setcontext is doing and longjmp can do the
same:
for programs that always longjmp within the same shstk the
first
loop is just p = current_ssp, but it also works when
longjmp
target is on a different shstk assuming nothing is running
on
that shstk, which is only possible if there is a restore
token
on top.

this implies if the kernel switches shstk on signal entry
it has
to add a restore-token on the switched away shstk.

I actually did a POC for this, but rejected it. The problem is,
if
there is a shadow stack overflow at that point then the kernel

quoted

quoted

can't

push the shadow stack token to the old stack. And shadow stack

quoted

quoted

overflow

is exactly the alt shadow stack use case. So it doesn't really

quoted

quoted

solve

the problem.

the restore token in the alt shstk case does not regress anything
but
makes some use-cases work.

alt shadow stack is important if code tries to jump in and out of
signal handlers (dosemu does this with swapcontext) and for that a
restore token is needed.

alt shadow stack is important if the original shstk did not
overflow
but the signal handler would overflow it (small thread stack, huge
sigaltstack case).

alt shadow stack is also important for crash reporting on shstk
overflow even if longjmp does not work then. longjmp to a
makecontext
stack would still work and longjmp back to the original stack can
be
made to mostly work by an altshstk option to overwrite the top
entry
with a restore token on overflow (this can break unwinding though).

There was previously a request to create an alt shadow stack for the
purpose of handling shadow stack overflow. So you are now suggesting to
to exclude that and instead target a different use case for alt shadow
stack?

But I'm not sure how much we should change the ABI at this point since
we are constrained by existing userspace. If you read the history, we
may end up needing to deprecate the whole elf bit for this and other
reasons.

So should we struggle to find a way to grow the existing ABI without
disturbing the existing userspace? Or should we start with something,
finally, and see where we need to grow and maybe get a chance at a
fresh start to grow it?

Like, maybe 3 people will show up saying "hey, I *really* need to use
shadow stack and longjmp from a ucontext stack", and no one says
anything about shadow stack overflow. Then we know what to do. And
maybe dosemu decides it doesn't need to implement shadow stack (highly
likely I would think). Now that I think about it, AFAIU SS_AUTODISARM
was created for dosemu, and the alt shadow stack patch adopted this
behavior. So it's speculation that there is even a problem in that
scenario.

Or maybe people just enable WRSS for longjmp() and directly jump back
to the setjmp() point. Do most people want fast setjmp/longjmp() at the
cost of a little security?

Even if, with enough discussion, we could optimize for all
hypotheticals without real user feedback, I don't see how it helps
users to hold shadow stack. So I think we should move forward with the
current ABI.

-- 
H.J.