Message-ID: <ZJR545en+dYx399c@arm.com>
Date:   Thu, 22 Jun 2023 17:42:11 +0100
From:   "szabolcs.nagy@....com" <szabolcs.nagy@....com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "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>,
        "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>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "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>,
        "pavel@....cz" <pavel@....cz>, "bp@...en8.de" <bp@...en8.de>,
        "mike.kravetz@...cle.com" <mike.kravetz@...cle.com>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "jamorris@...ux.microsoft.com" <jamorris@...ux.microsoft.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "john.allen@....com" <john.allen@....com>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "bsingharora@...il.com" <bsingharora@...il.com>,
        "oleg@...hat.com" <oleg@...hat.com>,
        "andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "gorcunov@...il.com" <gorcunov@...il.com>,
        "Yu, Yu-cheng" <yu-cheng.yu@...el.com>,
        "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>,
        "Torvalds, Linus" <torvalds@...ux-foundation.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "Yang, Weijiang" <weijiang.yang@...el.com>,
        "Eranian, Stephane" <eranian@...gle.com>
Subject: Re: [PATCH v9 23/42] Documentation/x86: Add CET shadow stack
 description

The 06/22/2023 08:26, Andy Lutomirski wrote:
> On Thu, Jun 22, 2023 at 2:28 AM szabolcs.nagy@....com
> <szabolcs.nagy@....com> wrote:
> >
> > The 06/21/2023 18:54, Edgecombe, Rick P wrote:
> > > On Wed, 2023-06-21 at 12:36 +0100, szabolcs.nagy@....com wrote:
> > > > > The 06/20/2023 19:34, Edgecombe, Rick P wrote:
> > > > > > > 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?
> >
> > that is not what i said.
> >
> > > 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.
> >
> > i'm not against deprecating the elf bit, but i think binary
> > marking will be difficult for this kind of feature no matter what
> > (code may be incompatible for complex runtime dependent 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.
> >
> > you may not get a second chance to fix a security feature.
> > it will be just disabled if it causes problems.
> 
> *I* would use altshadowstack.
> 
> I run a production system (that cares about correctness *and*
> performance, but that's not really relevant here -- SHSTK ought to be
> fast).  And, if it crashes, I want to know why.  So I handle SIGSEGV,
> etc so I have good logs if it crashes.  And I want those same logs if
> I overflow the stack.
> 
> That being said, I have no need for longjmp or siglongjmp for this.  I
> use exit(2) to escape.

the same crash handler that prints a log on shstk overflow should
work when a different cause of SIGSEGV is recoverable via longjmp.
to me this means that alt shstk must work with longjmp at least in
the non-shstk overflow case (which can be declared non-recoverable).

> For what it's worth, setjmp/longjmp is a bad API.  The actual pattern
> that ought to work well (and that could be supported well by fancy
> compilers and non-C languages, as I understand it) is more like a
> function call that has two ways out.  Like this (pseudo-C):
> 
> void function(struct better_jmp_buf &buf, args...)
> {
>    ...
>        if (condition)
>           better_long_jump(buf);  // long jumps out!
>        // could also pass buf to another function
>    ...
>        // could also return normally
> }
> 
> better_call_with_jmp_buf(function, args);
> 
> *This* could support altshadowstack just fine.  And many users might
> be okay with the understanding that, if altshadowstack is on, you have
> to use a better long jump to get out (or a normal sigreturn or _exit).

i don't understand why this would work fine when longjmp does not.
how does the shstk switch happen?

> No one is getting an altshadowstack signal handler without code
> changes.

assuming the same component is doing the alt shstk setup as the
longjmp.

> siglongjmp() could support altshadowstack with help from the kernel,
> but we probably don't want to go there.

what kind of help? maybe we need that help..

e.g. if the signal frame token is detected by longjmp on
the shstk then doing an rt_sigreturn with the right signal
frame context allows longjmp to continue unwinding the shstk.
however kernel sigcontext layout can change so userspace may
not know it so longjmp needs a helper, but only in the jump
across signal frame case.

(this is a different design than what i proposed earlier,
it also makes longjmp from alt shstk work without wrss,
the downside is that longjmp across makecontext needs a
separate solution then which implies that all shstk needs
a detectable token at the end of the shstk.. so again
something that we have to get right now and cannot add
later.)

Powered by blists - more mailing lists