Message-ID: <46a5ffc762bfd6ccb60c9568b7fb564d40c04c45.camel@intel.com>
Date:   Thu, 22 Jun 2023 16:47:41 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "broonie@...nel.org" <broonie@...nel.org>,
        "szabolcs.nagy@....com" <szabolcs.nagy@....com>
CC:     "Xu, Pengfei" <pengfei.xu@...el.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        "kcc@...gle.com" <kcc@...gle.com>,
        "Lutomirski, Andy" <luto@...nel.org>,
        "nadav.amit@...il.com" <nadav.amit@...il.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "david@...hat.com" <david@...hat.com>,
        "Schimpe, Christina" <christina.schimpe@...el.com>,
        "Yang, Weijiang" <weijiang.yang@...el.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "corbet@....net" <corbet@....net>, "nd@....com" <nd@....com>,
        "dethoma@...rosoft.com" <dethoma@...rosoft.com>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "debug@...osinc.com" <debug@...osinc.com>,
        "mike.kravetz@...cle.com" <mike.kravetz@...cle.com>,
        "bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "jamorris@...ux.microsoft.com" <jamorris@...ux.microsoft.com>,
        "pavel@....cz" <pavel@....cz>,
        "john.allen@....com" <john.allen@....com>,
        "bsingharora@...il.com" <bsingharora@...il.com>,
        "andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>,
        "oleg@...hat.com" <oleg@...hat.com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "gorcunov@...il.com" <gorcunov@...il.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "Yu, Yu-cheng" <yu-cheng.yu@...el.com>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "fweimer@...hat.com" <fweimer@...hat.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "Syromiatnikov, Eugene" <esyr@...hat.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "Torvalds, Linus" <torvalds@...ux-foundation.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "Eranian, Stephane" <eranian@...gle.com>
Subject: Re: [PATCH v9 23/42] Documentation/x86: Add CET shadow stack
 description

On Thu, 2023-06-22 at 09:27 +0100, szabolcs.nagy@....com wrote:


[ snip ]

> swapcontext is currently *not* supported: for it to work you have to
> be able to jump *back* into the signal handler, which does not work
> if
> the swapcontext target is on the original thread stack (instead of
> say a makecontext stack).
> 
> jumping back can only be supported if alt stack can be paired with
> an alt shadow stack.
> 
> unwinding across a series of signal interrupts should work even
> with discontinous shstk. libgcc does not implement this which is
> a problem i think.

I would be helpful if you could enumerate the cases you think are
important to support. I would like to see how we could support them in
the future in some mode.

> 
> > But how does the proposed token placed by the kernel on the
> > original
> > stack help this problem? The longjmp() would have to be able to
> > find
> > the location of the restore tokens somehow, which would not
> > necessarily
> > be near the setjmp() point. The signal token could even be on a
> > different shadow stack.
> 
> i posted the exact longjmp code and it takes care of this case.

I see how it works for the simple case of longjmp() from an alt shadow
stack. I would still prefer a different solution that works with the
overflow case. (probably new kernel functionality)

But I don't see how it works for unwinding off of a ucontext stack. Or
unwinding off of an ucontext stack that was swapped to from an alt
shadow stack.

> 
> setjmp does not need to do anything special.
> 
> the invariant is that an shstk is either capped by a restore token
> or in use by some executing task. this is guaranteed architecturally
> (when shstk is switched with an instruction) and should be guaranteed
> by the kernel too (when shstk is switched by the kernel).
> 
> > I'm also not sure leaving a token on signal doesn't weaken the
> > security
> > it it's own way as well. Any thread could then swap to that token.
> > Where as the shadow stack signal frame ssp pointer can only be used
> > from the shadow stack the signal was handled on.
> 
> as far as i'm concerned it is a valid programming model to switch
> to a stack that is currently not in use and we should always allow
> that. (signal handled on an alt stack may not return)

Some people just want shadow stack for like, a super stack canary, and
want everything to "just work". Some people want to lock things down as
much as possible and change their code to do it if needed.

It sure is a challenge to figure out where the happy medium is. Ideally
there would be several modes so all the users could be happy, but I
wouldn't want to start with that for multiple reasons. Although we do
have WRSS today, which can support pretty much everything
functionality-wise.

But in any case, we have limited room for movement. I actually had some
other ABI tweaks fully ready to post around the tracing usages, but I
just thought given the situation, it was better to start with what we
have. This project could really use a time machine...

> 
> > So I think, in addition to blocking the shadow stack overflow use
> > case
> > in the future, leaving a token behind on signal will not really
> > help
> > longjmp(). (or at least I'm not following)
> 
> the restore token must only be added if shstk is switched
> (currently it is not switched so don't add it, however if
> we agree on this then the unwinder can be fixed accordingly.)

I do agree that a token should not be added when the stack is not
switched, as today. But I don't agree on this solution for alt shadow
stacks. Again, I actually built that exact design in actual code, so
it's not a NIH thing. It just doesn't work for valid use cases.

Powered by blists - more mailing lists