Re: Shadow Stack Locking Semantics between arch's

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2950 bytes --]

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.

> 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.

> 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.  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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]