linux-kernel - Re: [PATCH] x86/boot: Fail the boot if !M486 and CPUID is missing

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: <20161120173244.a2odm3rupohvatiq@pd.tnic>
Date:   Sun, 20 Nov 2016 18:32:44 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Andy Lutomirski <luto@...capital.net>
Cc:     Matthew Whitehead <tedheadster@...il.com>,
        Brian Gerst <brgerst@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        X86 ML <x86@...nel.org>
Subject: Re: [PATCH] x86/boot: Fail the boot if !M486 and CPUID is missing

On Sun, Nov 20, 2016 at 08:22:25AM -0800, Andy Lutomirski wrote:
> This makes me nervous: don't some CPUs actually need the cpuid
> instruction when patching alternatives?

Nope, we use boot_cpu_has() in apply_alternatives() if that is what you
mean.

> And with this approach, we won't have the cpuid instruction there
> until after patching.

We will have set (or not) the X86_FEATURE_CPUID bit at
early_identify_cpu() time. Looking at the code, we do call sync_core()
pretty early. :-\

Hmm, that #ifdef CONFIG_M486 is there for a reason.

> Why not change this function entirely:
> 
> write_cr2(0);
> 
> CR2 should be available on all 32-bit CPUs.  It clobbers fewer
> registers.

Yap, just one. And not only that - it clobbers a register which gcc
doesn't have to reload at all.

> More usefully, CPUID causes an exit when running under
> most hypervisors, and that's quite slow.  The only case I can think of
> where CPUID should be faster than MOV to CR2 is on Xen PV before Ivy
> Bridge, and I'm not sure I care about performance there.
> 
> (On Xen PV, it will do a hypercall instead, but the hypercall should
> be good enough to serialize, too.)

A nop hypercall or whatever...

But yeah, pending a nod from hw people, this one sounds nice too. You
can do it basically on every CPU which supports paging. And that should
be all we support in Linux anyway.

> Or we could do it dynamically:
> 
> bt $X86_FEATURE_CPUID, CPU_FLAGS(boot_cpu_data) # or whatever -- I
> think we need to add an asm offset
> jnc 1f # here's our jump
> cpuid
> 1:

We could... we did move the X86_FEATURE* things to a separate header so
that they can be used in asm too.

write_cr2(0) doesn't sound so bad either. Except what happens if
someone decides to sync_core() before the first line of do_page_fault()
executes... I know, it is unlikely but we do unlikely things :)

But yeah, the write_cr2() sounds better if one considers the lower
register pressure. Which is nice.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.