On Tue, Aug 01, 2023 at 05:07:00PM +0000, Edgecombe, Rick P wrote:
On Tue, 2023-08-01 at 15:01 +0100, Mark Brown wrote:
It could be a different flag, like SHADOW_STACK_SET_TOKEN_MARKER, or it could be SHADOW_STACK_SET_MARKER, and callers could pass (SHADOW_STACK_SET_TOKEN | SHADOW_STACK_SET_MARKER) to get what you have implemented here. What do you think?
For arm64 code this would mean that it would be possible (and fairly easy) to create stacks which don't have a termination record which would make life harder for unwinders to rely on. I don't think this is insurmountable, creating manually shouldn't be the standard and it'll already be an issue on x86 anyway.
If you are going to support optionally writing to shadow stacks (which x86 needed for CRIU, and also seems like a nice thing for several other reasons), you are already at that point. Can't you also do a bunch of gcspopm's to the top of the GCS stack, and have no marker to hit before the end of the stack? (maybe not in GCS, I don't know...)
It's definitely possible to use writes or pops to achive the same effect, it's just that it's less likely to be something that happens through simple oversight than missing a flag off the initial map call would be.
The other minor issue is that the current arm64 marker is all bits 0 so by itself for arm64 _MARKER would have no perceptible impact, it would only serve to push the token down a slot in the stack (I'm guessing that's the intended meaning?).
Pushing the token down a frame is what flags==0 does in this patch, right?
Yes, exactly - if we make the top of stack record optional then if that flag is omitted we'd not do that.
You don't have to support all the flags actually, you could just support the one mode you already have and reject all other combinations... Then it matches between arch's, and you still have the guaranteed-ish end marker.
Sure, though if we're going to the trouble of checking for the flag we probably may as well implement it. I guess x86 is locked in at this point by existing userspace. I guess I'll implement it assuming nobody from userspace complains, it's trivial for a kernel.
So the question is not what mode should arm support, but should we have the flags match between x86 and ARM?
The flags should definitely match, no disagreement there.