Message-ID: <e83ee9fc1a6e98cab62b681de7209598394df911.camel@intel.com>
Date:   Fri, 10 Mar 2023 01:13:42 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "bp@...en8.de" <bp@...en8.de>,
        "joao@...rdrivepizza.com" <joao@...rdrivepizza.com>
CC:     "david@...hat.com" <david@...hat.com>,
        "bsingharora@...il.com" <bsingharora@...il.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "Syromiatnikov, Eugene" <esyr@...hat.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "rdunlap@...radead.org" <rdunlap@...radead.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "Eranian, Stephane" <eranian@...gle.com>,
        "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "fweimer@...hat.com" <fweimer@...hat.com>,
        "nadav.amit@...il.com" <nadav.amit@...il.com>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "dethoma@...rosoft.com" <dethoma@...rosoft.com>,
        "kcc@...gle.com" <kcc@...gle.com>,
        "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
        "pavel@....cz" <pavel@....cz>, "oleg@...hat.com" <oleg@...hat.com>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "Lutomirski, Andy" <luto@...nel.org>,
        "jamorris@...ux.microsoft.com" <jamorris@...ux.microsoft.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "Schimpe, Christina" <christina.schimpe@...el.com>,
        "mike.kravetz@...cle.com" <mike.kravetz@...cle.com>,
        "debug@...osinc.com" <debug@...osinc.com>,
        "Yang, Weijiang" <weijiang.yang@...el.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>,
        "john.allen@....com" <john.allen@....com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "rppt@...nel.org" <rppt@...nel.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "corbet@....net" <corbet@....net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "gorcunov@...il.com" <gorcunov@...il.com>
Subject: Re: [PATCH v7 28/41] x86: Introduce userspace API for shadow stack

+Joao regarding mixed mode designs

On Fri, 2023-03-10 at 00:51 +0100, Borislav Petkov wrote:
> On Thu, Mar 09, 2023 at 04:56:37PM +0000, Edgecombe, Rick P wrote:
> > There is a proc that shows if shadow stack is enabled in a thread.
> > It
> > does indeed come later in the series.
> 
> Not good enough:
> 
> 1. buried somewhere in proc where no one knows about it
> 
> 2. it is per thread so user needs to grep *all*

See "x86: Expose thread features in /proc/$PID/status" for the patch.
We could emit something in dmesg I guess? The logic would be:
 - Record the presence of elf SHSTK bit on exec
 - On shadow stack disable, if it had the elf bit, pr_info("bad!")

> 
> >   ... We previously tried to add some batch operations to improve
> > the
> >   performance, but tglx had suggested to start with something
> > simple.
> >   So we end up with this simple composable API.
> 
> I agree with starting simple and thanks for explaining this in
> detail.
> 
> TBH, though, it already sounds like a mess to me. I guess a mess
> we'll
> have to deal with because there will always be this case of some
> shared object/lib not being enabled for shstk because of raisins.

The compatibility problems are totally the mess in this whole thing.
When you try to look at a "permissive" mode that actually works it gets
even more complex. Joao and I have been banging our heads on that
problem for months.

But there are some expected users of this that say: we compile and
check our known set of binaries, we won't get any surprises. So it's
more of a distro problem.

> 
> And TBH #2, I would've done it even simpler: if some shared object
> can't
> do shadow stack, we disable it for the whole process. I mean, what's
> the
> point? 

You mean a late loaded dlopen()ed DSO? The enabling logic can't know
this will happen ahead of time.

If you mean if the shared objects in the elf all support shadow stack,
then this is what happens. The complication is that the loader wants to
enable shadow stack before it has checked the elf libs so it doesn't
underflow the shadow stack when it returns from the function that does
this checking.

So it does:
1. Enable shadow stack
2. Call elf libs checking functions
3. If all good, lock shadow stack. Else, disable shadow stack.
4. Return from elf checking functions and if shstk is enabled, don't
underflow because it was enabled in step 1 and we have return addresses
from 2 on the shadow stack

I'm wondering if this can't be improved in glibc to look like:
1. Check elf libs, and record it somewhere
2. Wait until just the right spot
3. If all good, enable and lock shadow stack.

But it depends on the loader code design which I don't know well
enough.

> Only some of the stack is shadowed so an attacker could find
> a way to keep the process perhaps run this shstk-unsupporting shared
> object more/longer and ROP its way around the system.

I hope non-permissive mode is the standard usage eventually.

> 
> But I tend to oversimplify things sometimes so...
> 
> What I'd like to have, though, is a kernel cmdline param which
> disables
> permissive mode and userspace can't do anything about it. So that
> once
> you boot your kernel, you can know that everything that runs on the
> machine has shstk and is properly protected.

Szabolcs Nagy was commenting something similar in another thread, for
supporting kernel enforced security policies. I think the way to do it
would have the kernel detect the the elf bit itself (like it used to)
and enable shadow stack on exec. If you can't rely on userspace to call
in to enable it, it's not clear at what point the kernel should check
that it did.

But then if you trigger off of the elf bit in the kernel, you get all
the regression issues of the old glibcs at that point. But it is
already an "I don't care if I crash" mode, so...

I think if you trust your libc, glibc could implement this in userspace
too. It would be useful even as as testing override.

> 
> Also, it'll allow for faster fixing of all those shared objects to
> use
> shstk by way of political pressure.

Powered by blists - more mailing lists