Re: [PATCH] x86/shstk: Enable shadow stack for x32

[H.J. Lu]

On Fri, Mar 22, 2024 at 8:58 AM Edgecombe, Rick P
<rick.p.edgecombe@xxxxxxxxx> wrote:
>
> On Fri, 2024-03-22 at 08:06 -0700, H.J. Lu wrote:
> > On Fri, Mar 22, 2024 at 7:07 AM Edgecombe, Rick P
> > <rick.p.edgecombe@xxxxxxxxx> wrote:
> > >
> > > On Fri, 2024-03-15 at 07:34 -0700, H.J. Lu wrote:
> > > > > How many people do you think will use this?
> > >
> > > I'm concerned that the only use of this will ever be exercise via
> > > the
> > > glibc unit tests, but will still require work to support.
> >
> >  Correct.  A small glibc change is needed.  Will post it after
> > my kernel change is merged.
>
> I mean it will require kernel work in the future to maintain support.
> That we will have to think about x32 effects when making other shadow
> stack changes.

It is way more than kernel SHSTK self tests.

> I'll paste my other comment in this thread:
>
> The main usage of shadow stack is security, and comes with some
> overhead. IIUC the main usage of x32 is performance benchmarking type
> stuff. Why would someone want to use shadow stack and x32 together?

Improve x32 security and user space IBT will add more improvement.

> >
> >
> > > > >
> > > > > I would have thought it would require more changes for basic
> > > > > x32
> > > >
> > > > This is all needed.
> > > >
> > > > > operation. What was the testing exactly?
> > > >
> > > > I configured x32 glibc with --enable-cet, build glibc and
> > > > run all glibc tests with shadow stack enabled.  There are
> > > > no regressions.  I verified that shadow stack is enabled
> > > > via /proc/pid/status.
> > >
> > > The shadow stack is supposed to be mapped above 4G, so how is this
> > > supposed to work for x32?
> >
> > This is not what I see:
> >
> > (gdb) info reg
> > ...
> > pl3_ssp        0xf7dcbfe8          0xf7dcbfe8
>
> The mapping above 4G was because Peterz raised the possibility that a
> 64 bit process could far call into a 32 bit segment and start doing
> signal stuff that would encounter undefined behavior. He wanted it
> cleanly blocked. So by keeping the shadow stack above 4GB, existing
> processes that turned on shadow stack would be preventing from
> transitioning to 32 bit and encountering the missing 32 bit signal
> support (because the CPU would #GP during the 32 bit transition if SSP
> is above 4GB).
>
> Probably there is some interplay between the x32 mmap logic and shadow
> stacks mapping, where it then becomes possible to get below 4GB. Since
> x32 needs the shadow stack to be below 4GB, it's incompatible with that
> solution. So this patch is not sufficient to enable x32 without side
> effects that were previously considered bad.

Mapping shadow stack below 4GB on x32 still provides security improvements
over no show stack.

> I see this is in tip now. I don't think it's a good idea to support
> upstream. The implications need more discussion, and there doesn't seem
> to be any real end user value.



-- 
H.J.