On Thu, 2026-08-27 at 19:14 +0100, Mark Brown wrote: > On Thu, Aug 27, 2026 at 12:52:28PM -0500, Bill Roberts wrote: > > > I have noticed some semantic differences between the arches. Given this > > example of current thread state, current thread locking state and the > > new features requested, as shown below: > > > unsigned long locked = 0x2; // Kernel Task State -> LOCK WRITE > > unsigned long cur_val = 0x3; // Kernel Task State -> WRITE and SHADOWSTACK > > ENABLED > > unsigned long new_val = 0x0; // Userspace Feature Change via syscall -> > > DISABLE > > > > x86-64 | risc-v | arm64 | > > > ---------- | -------- | --------- | > > > Works | Fails | Fails | > > I would not have expected that combination to work at all with the > prctl() (as opposed to arch_prctl()) interface TBH, if you've locked > write on you shouldn't be able to disable it. The reason that works on > x86 at the minute is that for x86 you can only change one bit at a time > so the new value when disabling is effectively 0x2, not 0x0.
makes sense. > > > I am proposing and have questions over the following: > > 1. What should the behavior be if you had write locked and disable the > > shadow stack? > > - I can argue both ways here, -EPERM or success. I think I and most arches > > lead to failure. > > Given that RISC-V doesn't support control of writes it's moot there > at the minute, and x86 currently uses arch_prctl() so will need an > additional API, it seems the path of least resistance is to allow it. > This also avoids locking writes (or pushes, for arm64) on effectively > also locking enable which seems neater. Hmm. I can't think of a reason to lock writes and not lock shadow stack too. Given likely no one is doing this, I wonder if we could change x86's behavior to match the others? The API would makes more sense to prevent disabling shadow stack if writes were enabled. It could return an EINVAL regardless if it is locked or not? Is that the arm behavior (forgetting about locked)? > > > 2. riscv should check that the low bit is set in locking not just that its > > 0, it should be 1 > > I think for ABI compatibility RISC-V will have to continue accepting 0 > as being equivalent to locking PR_SHADOW_STACK_ENABLE (or everything, > but it only supports that one bit right now). > > > 3. riscv should return -EPERM vs -EINVAL > > If you mean for arch_lock_shadow_stack_status() I think -EINVAL is a > sensible error code when the system or task does not support shadow > stacks, I'm not sure we should return -EPERM at all. On arm64 we > support locking any bit, not just the ones that we currently know about. > This is for future proofing, userspace can lock unknown flags. > > > If we can all agree on item 1, that locked bits check can be refactored and > > shared in one of > > two ways: > > 1. within prctl itself, before the arch hook is called, we would need > > helpers per-arch to extract the thread features and lock bits > > 2. as a helper where folks just pass the unsigned long of the bits to get > > the result > > For me option 1 seems a bit nicer, and moves more of the implementation > into generic code which is something we should really be doing in > general with the shadow stack support - there's a lot of cross arch > duplication at the minute. > +1 > It has been on my list to look at this > repitition at some point, it had been held up by the clone3() stuff but > that seems to have died a death for now. Oh? What was the blocker?
