linux-kernel - Re: [PATCH v10 6/7] x86/arch_prctl: Add ARCH_[GET|SET]

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:   Wed, 9 Nov 2016 14:34:14 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Borislav Petkov <bp@...e.de>
cc:     Kyle Huey <me@...ehuey.com>,
        Robert O'Callahan <robert@...llahan.org>,
        Andy Lutomirski <luto@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        Jeff Dike <jdike@...toit.com>,
        Richard Weinberger <richard@....at>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Shuah Khan <shuah@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Len Brown <len.brown@...el.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Dmitry Safonov <dsafonov@...tuozzo.com>,
        David Matlack <dmatlack@...gle.com>,
        linux-kernel@...r.kernel.org,
        user-mode-linux-devel@...ts.sourceforge.net,
        user-mode-linux-user@...ts.sourceforge.net,
        linux-fsdevel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        kvm@...r.kernel.org
Subject: Re: [PATCH v10 6/7] x86/arch_prctl: Add ARCH_[GET|SET]_CPUID

On Wed, 9 Nov 2016, Borislav Petkov wrote:

> On Tue, Nov 08, 2016 at 09:06:31PM +0100, Thomas Gleixner wrote:
> > The upcoming ring3 mwait stuff can add its magic to tweak that MSR into
> > this function.
> > 
> > Stick the call at the end of init_scattered_cpuid_features() for now. I
> > still need to figure out a proper place for it.
> 
> So Thomas and I discussed this more on IRC and I think we can get rid
> of the MSR iterating in scattered.c and integrate both the R3 MWAIT and
> CPUID faulting like this:

I agree mostly.
 
> ---
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index fcd484d2bb03..5c38a85af2e7 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -452,6 +457,39 @@ static void intel_bsp_resume(struct cpuinfo_x86 *c)
>  	init_intel_energy_perf(c);
>  }
>  
> +static void init_misc_enables(struct cpuinfo_x86 *c)
> +{
> +	u64 val, misc_en;
> +
> +	if (rdmsrl_safe(MSR_MISC_FEATURES_ENABLES, &misc_en))
> +		return;
> +
> +	misc_en &= ~MSR_MISC_ENABLES_CPUID_FAULT_ENABLE;

I rather prefer to write this MSR to 0 right away and just enable the bits
which we really support.

Whatever Intel comes up with next, e.g. faulting of random other
instructions or whatever (mis)feature they think is valuable, will lead to
a debugging nightmare if some incompetent BIOS writer sets the bit and the
kernel does not know about it.

Yes, I know that there might be bits forced to 1 at some point in the
future, but let's deal with that when it happens.

Right now I can enable the CPUID FAULT bit on my broadwell and watch user
space programs die unexpectedly without a hint why. Simply because it's not
documented in the SDM. So we rather be safe than surprised.

@hpa: Thoughts?

Thanks,

	tglx