linux-kernel - Re: [PATCH 1/3] x86/cpu: Process all CPUID dependencies after identifying CPU info

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: <Y5IQNY/fZw2JFA0B@google.com>
Date:   Thu, 8 Dec 2022 16:26:29 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Subject: Re: [PATCH 1/3] x86/cpu: Process all CPUID dependencies after
 identifying CPU info

On Thu, Dec 08, 2022, Borislav Petkov wrote:
> On Sat, Dec 03, 2022 at 12:37:43AM +0000, Sean Christopherson wrote:
> > Process all CPUID dependencies to ensure that a dependent is disabled if
> > one or more of its parent features is unsupported.
> 
> Just out of curiosity: this is some weird guest configuration, right?

No, it's also relevant for bare metal.

> Not addressing a real hw issue...

But it's not really a hardware issue either.  More like an admin/user issue.

The problem is that if a kernel is built for subset of CPU types, e.g. just Intel
or just Centaur, and then booted on an "unsupported" CPU type, init_ia32_feat_ctl()
will never be invoked because ->c_init() will point a default_init(), and so the
kernel never checks MSR_IA32_FEAT_CTL to see if VMX and/or SGX are fully enabled.

E.g. if someone booted an "unsupported" kernel and also disabled VMX in BIOS, then
the CPU will enumerate support for VMX in CPUID, but attempting to actually enable
VMX will fail due to VMX being disabled in MSR_IA32_FEAT_CTL.

> > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> > index bf4ac1cb93d7..094fc69dba63 100644
> > --- a/arch/x86/kernel/cpu/common.c
> > +++ b/arch/x86/kernel/cpu/common.c
> > @@ -1887,6 +1887,12 @@ static void identify_cpu(struct cpuinfo_x86 *c)
> >  
> >  	ppin_init(c);
> >  
> > +	/*
> > +	 * Apply CPUID dependencies to ensure dependent features are disabled
> > +	 * if a parent feature is unsupported but wasn't explicitly disabled.
> > +	 */
> > +	apply_cpuid_deps(c);
> 
> I'd probably call that resolve_cpuid_deps()...

"resolve" works for me.