Re: [PATCH 00/35] Shadow stacks for userspace

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Tue, 2022-05-31 at 19:36 +0300, Mike Rapoport wrote:
> > WRSS is a feature where you would usually want to lock it as
> > disabled,
> > but WRSS cannot be enabled if shadow stack is not enabled. Locking
> > shadow stack and WRSS off together doesn't have any security
> > benefits
> > in theory. so I'm thinking glibc doesn't need to do this. The
> > kernel
> > could even refuse to lock WRSS without shadow stack being enabled.
> > Could we avoid the extra ptrace functionality then?
> 
> What I see for is that a program can support shadow stack, glibc
> enables
> shadow stack, does not enable WRSS and than calls
> 
>         arch_prctl(ARCH_X86_FEATURE_LOCK,
>                    LINUX_X86_FEATURE_SHSTK | LINUX_X86_FEATURE_WRSS);

I see the logic is glibc will lock SHSTK|IBT if either is enabled in
the elf header. I guess that is why I didn't see the locking happening
for me, because my manual enablement test doesn't have either set in
the header.

It can't see where that glibc knows about WRSS though...

The glibc logic seems wrong to me also, because shadow stack or IBT
could be force-disabled via glibc tunables. I don't see why the elf
header bit should exclusively control the feature locking. Or why both
should be locked if only one is in the header.

> 
> so that WRSS cannot be re-enabled.
> 
> For the programs that do not support shadow stack, both SHSTK and
> WRSS are
> disabled, but still there is the same call to
> arch_prctl(ARCH_X86_FEATURE_LOCK, ...) and then neither shadow stack
> nor
> WRSS can be enabled.
> 
> My original plan was to run CRIU with no shadow stack, enable shadow
> stack
> and WRSS in the restored tasks using arch_prct() and after the shadow
> stack
> contents is restored disable WRSS.
> 
> Obviously, this didn't work with glibc I have :)

Were you disabling shadow stack via glibc tunnable? Or was the elf
header marked for IBT? If it was a plain old binary, the code looks to
me like it should not lock any features.

> 
> On the bright side, having a ptrace call to unlock shadow stack and
> wrss
> allows running CRIU itself with shadow stack.
>  

Yea, having something working is really great. My only hesitancy is
that, per a discussion on the LAM patchset, we are going to make this
enabling API CET only (same semantics for though). I suppose the
locking API arch_prctl() could still be support other arch features,
but it might be a second CET only regset. It's not the end of the
world.

I guess the other consideration is tieing CRIU to glibc peculiarities.
Like even if we fix glibc, then CRIU may not work with some other libc
or app that force disables for some weird reason. Is it supposed to be
libc-agnostic?