linux-kernel - Re: [PATCH] x86: static_cpu_has_safe: discard dynamic check after init

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]

Date:	Tue, 19 Jan 2016 20:02:01 -0800
From:	"H. Peter Anvin" <hpa@...or.com>
To:	Borislav Petkov <bp@...e.de>
Cc:	Brian Gerst <brgerst@...il.com>,
	the arch/x86 maintainers <x86@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...nel.org>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Andy Lutomirski <luto@...capital.net>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] x86: static_cpu_has_safe: discard dynamic check after
 init

On 01/19/16 01:22, Borislav Petkov wrote:
> On Mon, Jan 18, 2016 at 05:33:03PM -0800, H. Peter Anvin wrote:
>> Why the f do we call a subroutine for what amounts to a single bt or
>> test instruction?
> 
> No real reason. You can kick me when you see me next time:
> 
> 4a90a99c4f80 ("x86: Add a static_cpu_has_safe variant")
> 

So, here is my suggestion:

1. Just get rid of static_cpu_has_safe() and make static_cpu_has() safe.

2. Get rid of the non-asm goto variant and just fall back to dynamic if
asm goto is unavailable.  It doesn't make any sense, really, if it is
supposed to be safe, and by now the asm goto-capable gcc is in more wide
use.  (Originally the gcc 3.x fallback to pure dynamic didn't exist,
either.)

3. Put the dynamic test in the .init.text section and inline it:

	.section .init.text,"ax"
	testb %2,%3
	jnz %[t_yes]
	jmp %[t_no]
	.previous

	... "i" (1 << (bit & 7)),
	    "m" (((const char *)boot_cpu_data->x86_capability)[bit >> 3]) ...

(The code would be slightly simpler/cleaner with testl, but that would
unnecessarily create a long immediate, or with btl, but that would be
slower.  We could use CONST_MASK_ADDR() and CONST_MASK() from
asm/bitops.h, but I'm slightly uncomfortable with the idea of leveraging
an interface which is ultimately an internal implementation detail of
bitops.h that might change in the future without people realizing its
implications.)

The only thing we lose is the 2-byte optimization in case we have the
good luck for it to actually work.  At this point I'm thinking it isn't
worth it, and instead that safety trumps it.  It *would* be interesting
to see if there are any call sites where it would actually kick in.

	-hpa