Messages in this thread | | | Date | Tue, 31 Oct 2023 06:53:09 -0700 | Subject | Re: [PATCH] x86: Let AS_WRUSS depend on X86_64 | From | 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.
| |