Re: [PATCH] x86: Let AS_WRUSS depend on X86_64

[Dave Hansen]

On 10/31/23 03:21, Jiri Slaby (SUSE) wrote:
...
> Provided the wruss instruction is 64-bit only (and used in pure 64-bit
> X86_USER_SHADOW_STACK), it has little sense to have AS_WRUSS=y set on
> 32-bit.
> 
> Therefore, make the whole test dependent on X86_64 to ensure it's set
> only on 64-bit.
...
>  config AS_WRUSS
>  	def_bool $(as-instr,wrussq %rax$(comma)(%rbx))
> +	depends on X86_64
>  	help
>  	  Supported by binutils >= 2.31 and LLVM integrated assembler

What's the downside to just leaving this alone?

This patch just seems wrong logically.  Suppose some deranged person
wanted 32-bit shadow stack support.  They'd have to go hunt this down
via trial and error instead of just enabling X86_USER_SHADOW_STACK.

Granted, that would take one crazy person five minutes to figure out why
their .config is broken, but it still seems wrong.  It's especially
wrong without a comment because it logically reads something along the
lines of "WRUSS is only available on x86_64 configs".

A better way to do this would be:

config HAS_SHADOW_STACKS
	depends on X86_64

config AS_WRUSS
	...
	# Avoid setting AS_WRUSS on configs that don't need it:
	depends on HAS_SHADOW_STACKS

config X86_USER_SHADOW_STACK
        bool "X86 userspace shadow stack"
        depends on AS_WRUSS
        depends on HAS_SHADOW_STACKS

But that honestly doesn't seem worth it because (circling back to the
first thing I wrote...) I don't really know what the benefit is to doing
this in the first place.