linux-kernel - Re: [RFC 05/10] x86/speculation: Add basic IBRS support infrastructure

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: <1516784541.13558.90.camel@infradead.org>
Date:   Wed, 24 Jan 2018 09:02:21 +0000
From:   David Woodhouse <dwmw2@...radead.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        KarimAllah Ahmed <karahmed@...zon.de>,
        linux-kernel@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Andy Lutomirski <luto@...nel.org>,
        Arjan van de Ven <arjan@...ux.intel.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Asit Mallick <asit.k.mallick@...el.com>,
        Borislav Petkov <bp@...e.de>,
        Dan Williams <dan.j.williams@...el.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "H . Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        Janakarajan Natarajan <Janakarajan.Natarajan@....com>,
        Joerg Roedel <joro@...tes.org>,
        Jun Nakajima <jun.nakajima@...el.com>,
        Laura Abbott <labbott@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Tom Lendacky <thomas.lendacky@....com>, kvm@...r.kernel.org,
        x86@...nel.org
Subject: Re: [RFC 05/10] x86/speculation: Add basic IBRS support
 infrastructure

On Wed, 2018-01-24 at 09:47 +0100, Peter Zijlstra wrote:
> Typically tglx likes to use x86_match_cpu() for these things; see also
> commit: bd9240a18edfb ("x86/apic: Add TSC_DEADLINE quirk due to
> errata").

Thanks, will fix. I think we might also end up in whitelist mode,
adding "known good" microcodes to the list as they get released or
retroactively blessed.

I would really have liked a new bit in IA32_ARCH_CAPABILITIES to say
that it's safe, but that's not possible for *existing* microcode which
actually turns out to be OK in the end.

That means the whitelist ends up basically empty right now. Should I
add a command line parameter to override it? Otherwise we end up having
to rebuild the kernel every time there's a microcode release which
covers a new CPU SKU (which is why I kind of hate the whitelist, but
Arjan is very insistent...)

I'm kind of tempted to turn it into a whitelist just by adding 1 to the
microcode revision in each table entry. Sure, that N+1 might be another
microcode build that also has issues but never saw the light of day...
but that's OK as long it never *does*. And yes we'd have to tweak it if
revisions that are blacklisted in the Intel doc are subsequently
cleared. But at least it'd require *less* tweaking.

> > 
> > +
> > +static int bad_spectre_microcode(struct cpuinfo_x86 *c)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(spectre_bad_microcodes); i++) {
> > +		if (c->x86_model == spectre_bad_microcodes[i].model &&
> > +		    c->x86_mask == spectre_bad_microcodes[i].stepping)
> > +			return (c->microcode <= spectre_bad_microcodes[i].microcode);
> > +	}
> > +	return 0;
> > +}
> The above is Intel only, you should check vendor too I think.

It's in intel.c, called from early_init_intel(). Isn't that sufficient?

> > 
> >  static void early_init_intel(struct cpuinfo_x86 *c)
> >  {
> >  	u64 misc_enable;
> > @@ -122,6 +173,18 @@ static void early_init_intel(struct cpuinfo_x86 *c)
> >  	if (c->x86 >= 6 && !cpu_has(c, X86_FEATURE_IA64))
> >  		c->microcode = intel_get_microcode_revision();
> >  
> > +	if ((cpu_has(c, X86_FEATURE_SPEC_CTRL) ||
> > +	     cpu_has(c, X86_FEATURE_AMD_SPEC_CTRL) ||
> > +	     cpu_has(c, X86_FEATURE_AMD_PRED_CMD) ||
> > +	     cpu_has(c, X86_FEATURE_AMD_STIBP)) && bad_spectre_microcode(c)) {
> > +		pr_warn("Intel Spectre v2 broken microcode detected; disabling SPEC_CTRL\n");
> > +		clear_cpu_cap(c, X86_FEATURE_SPEC_CTRL);
> > +		clear_cpu_cap(c, X86_FEATURE_STIBP);
> > +		clear_cpu_cap(c, X86_FEATURE_AMD_SPEC_CTRL);
> > +		clear_cpu_cap(c, X86_FEATURE_AMD_PRED_CMD);
> > +		clear_cpu_cap(c, X86_FEATURE_AMD_STIBP);
> > +	}
> And since its Intel only, what are those AMD features doing there?

Hypervisors which only want to expose PRED_CMD may do so using the AMD
feature bit. SPEC_CTRL requires save/restore and live migration
support, and isn't needed with retpoline anyway (since guests won't be
calling directly into firmware).


Download attachment "smime.p7s" of type "application/x-pkcs7-signature" (5213 bytes)