Re: [PATCH v7 28/41] x86: Introduce userspace API for shadow stack

[H.J. Lu]

On Thu, Mar 9, 2023 at 5:13 PM Edgecombe, Rick P
<rick.p.edgecombe@xxxxxxxxx> wrote:
>
> +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.

I will try it out.

> 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.



-- 
H.J.