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

[H.J. Lu]

On Thu, Mar 9, 2023 at 6:03 PM H.J. Lu <hjl.tools@xxxxxxxxx> wrote:
>
> 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.
>

Currently glibc enables shadow stack as early as possible.  There
are only a few places where a function call in glibc never returns.
We can enable shadow stack just before calling main.   There are
quite some code paths without shadow stack protection.   Is this
an issue?

H.J.