[PATCH v2 1/2] arch/x86: handle ARCH_SHSTK_ENABLE explicitly

[PATCH v2 1/2] arch/x86: handle ARCH_SHSTK_ENABLE explicitly

[Bill Roberts]

The arch_prctl() handling for shadow stack operations checks that the
operation being specified is a specific value, except for the
ARCH_SHSTK_ENABLE condition, which is treated like a default case in a
switch.

However, the special handling for ARCH_SHSTK_UNLOCK is only performed when
task != current. As a result, an ARCH_SHSTK_UNLOCK request for the current
task bypasses the unlock check and falls through to the ARCH_SHSTK_ENABLE
path, causing the request to be interpreted as an enable operation
instead.

To fix this, handle ARCH_SHSTK_ENABLE explicitly rather than relying on it
as the default case. This fixes the incorrect dispatch and makes the
operation handling more robust by requiring each operation to be matched
explicitly.

Signed-off-by: Bill Roberts <bill.roberts@arm.com>
---
 arch/x86/kernel/shstk.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
index 0ca64900192f..65c896d02379 100644
--- a/arch/x86/kernel/shstk.c
+++ b/arch/x86/kernel/shstk.c
@@ -607,11 +607,13 @@ long shstk_prctl(struct task_struct *task, int option, unsigned long arg2)
 		return -EINVAL;
 	}
 
-	/* Handle ARCH_SHSTK_ENABLE */
-	if (features & ARCH_SHSTK_SHSTK)
-		return shstk_setup();
-	if (features & ARCH_SHSTK_WRSS)
-		return wrss_control(true);
+	if (option == ARCH_SHSTK_ENABLE) {
+		if (features & ARCH_SHSTK_SHSTK)
+			return shstk_setup();
+		if (features & ARCH_SHSTK_WRSS)
+			return wrss_control(true);
+	}
+
 	return -EINVAL;
 }
 
-- 
2.54.0

[PATCH v2 2/2] selftest/x86: test ARCH_SHSTK_UNLOCK

[Bill Roberts]

Test that ARCH_SHSTK_UNLOCK can't be called from normal (non-ptrace)
context.

Signed-off-by: Bill Roberts <bill.roberts@arm.com>
---
 tools/testing/selftests/x86/test_shadow_stack.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/testing/selftests/x86/test_shadow_stack.c b/tools/testing/selftests/x86/test_shadow_stack.c
index 3d6ca33edba4..cbe7ca4ebd90 100644
--- a/tools/testing/selftests/x86/test_shadow_stack.c
+++ b/tools/testing/selftests/x86/test_shadow_stack.c
@@ -1059,6 +1059,14 @@ int main(int argc, char *argv[])
 {
 	int ret = 0;
 
+	/* Test that ARCH_SHSTK_UNLOCK can't be called from normal (non-ptrace) context. */
+	if (!ARCH_PRCTL(ARCH_SHSTK_UNLOCK, ARCH_SHSTK_SHSTK)) {
+		printf("[FAIL]\tARCH_SHSTK_UNLOCK should fail in a non-ptrace context\n");
+		goto out;
+	}
+
+	printf("[OK]\tCouldn't enable Shadow stack via UNLOCK\n");
+
 	if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
 		printf("[SKIP]\tCould not enable Shadow stack\n");
 		return 1;
-- 
2.54.0

Re: [PATCH v2 1/2] arch/x86: handle ARCH_SHSTK_ENABLE explicitly

[Edgecombe, Rick P]

On Tue, 2026-07-07 at 14:11 -0500, Bill Roberts wrote:
> The arch_prctl() handling for shadow stack operations checks that the
> operation being specified is a specific value, except for the
> ARCH_SHSTK_ENABLE condition, which is treated like a default case in a
> switch.

Seems a bit unspecific "checks that the operation being specified is a specific
value".

> 
> However, the special handling for ARCH_SHSTK_UNLOCK is only performed when
> task != current. As a result, an ARCH_SHSTK_UNLOCK request for the current
> task bypasses the unlock check and falls through to the ARCH_SHSTK_ENABLE
> path, causing the request to be interpreted as an enable operation
> instead.
> 
> To fix this, handle ARCH_SHSTK_ENABLE explicitly rather than relying on it
> as the default case. This fixes the incorrect dispatch and makes the
> operation handling more robust by requiring each operation to be matched
> explicitly.

I don't know if maintainers will be happy with the log. I'd have written it like
this. But no warranty:

   On x86 user shadow stack is controlled via an arch_prctl() interface, in
   order to allow ptrace to actuate some of the options, and allow normal
   syscall execution of arch_prctl() to execute others. Only valid shadow stack
   options, for both ptrace and normal userspace callers, are directed into the
   shstk_prctl() handler via checking in do_arch_prctl_64(). Then in
   shstk_prctl(), it differentiates ptrace callers by checking if task !=
   current.
   
   The trouble starts when the handling of the task == current (normal syscall)
   case treats the enabling option, ARCH_SHSTK_ENABLE, like a default case. It
   reasons that shstk_prctl() has handled all other possible shadow stack
   options, so ARCH_SHSTK_ENABLE can be the only remaining. Or so it thinks.
   Because the ARCH_SHSTK_UNLOCK option is only handled in the ptrace case.
   Since it has no handler in the syscall case, ARCH_SHSTK_UNLOCK can get
   misinterpreted as ARCH_SHSTK_ENABLE by the erroneous default case logic.
   
   The impact is that doing an arch_prtcl(ARCH_SHSTK_UNLOCK) syscall actually
   behaves like an arch_prtcl(ARCH_SHSTK_ENABLE) syscall. This is a surprising
   behavior, but does not expose any control to userspace that it doesn't
   already have.
   
   So drop the buggy default assumptions by checking for ARCH_SHSTK_ENABLE
   explicitly.


> 
> Signed-off-by: Bill Roberts <bill.roberts@arm.com>
> ---

Fix looks good. I think it's correct that this isn't marked for stable.

>  arch/x86/kernel/shstk.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
> index 0ca64900192f..65c896d02379 100644
> --- a/arch/x86/kernel/shstk.c
> +++ b/arch/x86/kernel/shstk.c
> @@ -607,11 +607,13 @@ long shstk_prctl(struct task_struct *task, int option, unsigned long arg2)
>  		return -EINVAL;
>  	}
>  
> -	/* Handle ARCH_SHSTK_ENABLE */
> -	if (features & ARCH_SHSTK_SHSTK)
> -		return shstk_setup();
> -	if (features & ARCH_SHSTK_WRSS)
> -		return wrss_control(true);
> +	if (option == ARCH_SHSTK_ENABLE) {
> +		if (features & ARCH_SHSTK_SHSTK)
> +			return shstk_setup();
> +		if (features & ARCH_SHSTK_WRSS)
> +			return wrss_control(true);
> +	}
> +
>  	return -EINVAL;
>  }
>  

Re: [PATCH v2 2/2] selftest/x86: test ARCH_SHSTK_UNLOCK

[Edgecombe, Rick P]

On Tue, 2026-07-07 at 14:11 -0500, Bill Roberts wrote:
> Test that ARCH_SHSTK_UNLOCK can't be called from normal (non-ptrace)
> context.
> 
> Signed-off-by: Bill Roberts <bill.roberts@arm.com>
> ---

Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>