On Wed, Aug 09, 2023 at 03:24:14PM +0100, Catalin Marinas wrote: > On Mon, Aug 07, 2023 at 11:00:08PM +0100, Mark Brown wrote: > > +* When set PR_SHADOW_STACK_ENABLE flag allocates a Guarded Control Stack for > > The 'for' at the end of the line above is not needed. > > > + and enables GCS for the thread, enabling the functionality controlled by I find it a little clearer that it's a per thread stack here but sure. > > +* PR_LOCK_SHADOW_STACK_STATUS allows any bit to be locked, this allows > > + userspace to prevent changes to any future features. > I presume a new lock prctl() won't allow unlocking but can only extend > the lock. I haven't looked at the patches yet but it may be worth > spelling this out. Yes, there is no unlock prctl() - the lock prctl() just lets you set locks. > > +* If GCS is disabled for a thread after having previously been enabled then > > + the stack will remain allocated for the lifetime of the thread. > Sorry if this has been discussed in other threads. What is the issue > with unmapping/freeing of the shadow stack? If something was in the middle of looking the GCS (eg, you're trying to disable GCS during signal handling that preempted something that is logging the call stack) and you pull the GCS storage from underneath it then things will go badly. > > At present > > + any attempt to reenable GCS for the thread will be rejected, this may be > > + revisited in future. > What's the rationale here? Is it that function returns won't work? We have to work out where to point GCSPR_EL0 when reenabling - at the top of the old stack, where it was before, on a new stack? It was easier to just not support reenabling than to work out what the sensible answer is when it's not clear that any real use case exists to inform what makes sense. We can add support for that later (you can probe by starting a thread, disabling and trying to reenable) if someone does come up with a use case. The use case for disabling is a non-enforcing mode which logs GCS faults and then disables GCS. > > +3. Allocation of Guarded Control Stacks > > +---------------------------------------- > > +* When GCS is enabled for a thread a new Guarded Control Stack will be > > + allocated for it of size RLIMIT_STACK / 2 or 2 gigabytes, whichever is > > + smaller. > Is this number based on the fact that a function call would only push > the LR to GCS while standard function prologue pushes at least two > registers? It's actually based on bitrot that I'd initially chosen a smaller value since it's likely that functions will push at least something as you suggest, the patches now just use RLIMIT_STACK. I'll fix. > > +* When GCS is disabled for a thread the Guarded Control Stack initially > > + allocated for that thread will be freed. Note carefully that if the > > + stack has been switched this may not be the stack currently in use by > > + the thread. > Does this not contradict an earlier statement that the GCS is not freed > for a thread when disabled? Yes, it was meant to say when the thread is freed rather than when GCS is disabled. > > +* When GCS is enabled for the interrupted thread a signal handling specific > > + GCS cap token will be written to the GCS, this is an architectural GCS cap > > + token with bit 63 set. The GCSPR_EL0 reported in the signal frame will > > + point to this cap token. > I lost track of the GCS spec versions. Has the valid cap token format > been updated? What I have in my spec (though most likely old) is: > An entry in the Guarded control stack is defined as a Valid cap entry, > if bits [63:12] of the value are same as bits [63:12] of the address > where the entry is stored and bits [11:0] contain a Valid cap token. You have a draft version of the spec, this was changed and now there is now a token field reserved in the low 12 bits of the register: #define GCS_CAP_ADDR_MASK GENMASK(63, 12) #define GCS_CAP_ADDR_SHIFT 12 #define GCS_CAP_ADDR_WIDTH 52 #define GCS_CAP_ADDR(x) FIELD_GET(GCS_CAP_ADDR_MASK, x) #define GCS_CAP_TOKEN_MASK GENMASK(11, 0) #define GCS_CAP_TOKEN_SHIFT 0 #define GCS_CAP_TOKEN_WIDTH 12 #define GCS_CAP_TOKEN(x) FIELD_GET(GCS_CAP_TOKEN_MASK, x) #define GCS_CAP_VALID_TOKEN 0x1 #define GCS_CAP_IN_PROGRESS_TOKEN 0x5 #define GCS_CAP(x) ((((unsigned long)x) & GCS_CAP_ADDR_MASK) | \ GCS_CAP_VALID_TOKEN)
Attachment:
signature.asc
Description: PGP signature
