linux-kernel - Re: [PATCH] x86: Let AS_WRUSS depend on X86

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <786d3451-e3ac-4868-8d8b-28f106e13407@kernel.org>
Date:   Tue, 31 Oct 2023 15:04:21 +0100
From:   Jiri Slaby <jirislaby@...nel.org>
To:     Dave Hansen <dave.hansen@...el.com>, bp@...en8.de
Cc:     peterz@...radead.org, linux-kernel@...r.kernel.org,
        Yu-cheng Yu <yu-cheng.yu@...el.com>,
        Rick Edgecombe <rick.p.edgecombe@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Kees Cook <keescook@...omium.org>,
        Mike Rapoport <rppt@...nel.org>,
        Pengfei Xu <pengfei.xu@...el.com>,
        John Allen <john.allen@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org
Subject: Re: [PATCH] x86: Let AS_WRUSS depend on X86_64

On 31. 10. 23, 14:53, Dave Hansen wrote:
> 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.

All wrussq, rax, rbx can never be right on 32-bit anyway...

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

Which is right, or what am I missing?

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

Again, to avoid nonsense in 32bit configs produced by oldconfig (as I 
noted in the commit log).

thanks,
-- 
js
suse labs