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?
