On Wed, Jun 21, 2023 at 11:54 AM Edgecombe, Rick P <rick.p.edgecombe@xxxxxxxxx> wrote: > > On Wed, 2023-06-21 at 12:36 +0100, szabolcs.nagy@xxxxxxx wrote: > > > The 06/20/2023 19:34, Edgecombe, Rick P wrote: > > > > > On Tue, 2023-06-20 at 10:17 +0100, szabolcs.nagy@xxxxxxx wrote: > > > > > > > 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 > > > > > > > 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. > > > > > > > > > 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. > > > > > > > > > > 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 > > > > > > > > > > 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 > > > > > > > can't > > > > > push the shadow stack token to the old stack. And shadow stack > > > > > > > overflow > > > > > is exactly the alt shadow stack use case. So it doesn't really > > > > > > > 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.
