On Wed, Aug 23, 2023 at 11:09:59AM +0100, Szabolcs Nagy wrote:
The 08/22/2023 18:53, Mark Brown wrote:
On Tue, Aug 22, 2023 at 05:49:51PM +0100, Catalin Marinas wrote:
the what but not the why. For example roughly the current behaviour was already in place in v10 of the original series:
well the original shstk patches predate clone3 so no surprise there. e.g. v6 is from 2018 and clone3 is 2019 linux 5.3 https://lore.kernel.org/lkml/20181119214809.6086-1-yu-cheng.yu@intel.com/
Ah, that'd do it. People weren't excited enough on about clone3() when reviewing the shadow stack patches, I hadn't realised clone3() was so new.
I do worry about the story for users calling the underlying clone3() API (or legacy clone() for that matter) directly, and we would also need to handle the initial GCS enable via prctl() - that's not insurmountable, we could add a size argument there that only gets interpreted during the initial enable for example.
musl and bionic currently use plain clone for threads.
and there is user code doing raw clone threads (such threads are technically not allowed to call into libc) it's not immediately clear to me if having gcs in those threads is better or worse.
Right, that's my big worry - I hadn't realised it was extending as far as musl/bionic.
one difference is that userspace can then set gcspr of a new thread and e.g. two threads can have overlapping gcs, however i don't think this impacts security much since if clone3 is attacker controlled then likely all bets are off.
Yeah, I think that's a "you broke it, you get all the pieces" thing.
My sense is that they deployment story is going to be smoother with defaults being provided since it avoids dealing with the issue of what to do if userspace creates a thread without a GCS in a GCS enabled process but like I say I'd be totally happy to extend clone3(). I will put some patches together for that (probably once the x86 stuff lands). Given the size of this series it might be better split out for manageability if nothing else.
i would make thread without gcs to implicitly disable gcs, since that's what's bw compat with clones outside of libc (the libc can guarantee gcs allocation when gcs is enabled).
That'd create a pretty substantial divergence with the x86 patches if they land this time around, there's not enough time to rework them now - I suppose it'd mainly bite people implementing libc type stuff but still, doesn't feel great.