linux-kernel - Re: [PATCH v7 2/2] x86/mm: Don't disable INVLPG if the kernel is running on a hypervisor

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] [day] [month] [year] [list]

Date: Tue, 16 Apr 2024 16:21:37 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Xi Ruoyao <xry111@...111.site>, Dave Hansen <dave.hansen@...ux.intel.com>, 
	Michael Kelley <mhklinux@...look.com>, Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>, 
	Andy Lutomirski <luto@...nel.org>, Peter Zijlstra <peterz@...radead.org>, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org, linux-kernel@...r.kernel.org, 
	Andrew Cooper <andrew.cooper3@...rix.com>
Subject: Re: [PATCH v7 2/2] x86/mm: Don't disable INVLPG if the kernel is
 running on a hypervisor

On Thu, Apr 11, 2024, Dave Hansen wrote:
> On 4/11/24 09:22, Sean Christopherson wrote:
> > In other words, simply checking HYPERVISOR *might* be safe, but it might not.
> > If we wanted to be paranoid, this could also check X86_FEATURE_VMX, which also
> > doesn't guarantee VMX non-root mode and would unnecessarily restrict PCID usage
> > to setups that allow nested VMX, but AFAIK there aren't any hypervisors which
> > fully emulate VMX.
> 
> X86_FEATURE_HYPERVISOR is most commonly used for vulnerabilities to see
> if the data coming out of CPUID is likely to be garbage or not.  I think
> that's the most important thing to focus on.
> 
> It's arguable that x86_match_cpu() itself should just have a:
> 
> 	/*
> 	 * Don't even waste our time when running under a hypervisor.
> 	 * They lie.
> 	 */
> 	if (boot_cpu_bas(X86_FEATURE_HYPERVISOR))
> 		return NULL;
> 
> (well, it should probably actually be in the for() loop because folks
> might be looking for an X86_FEATURE_* that is set by software or derived
> from actually agreed-upon host<->guest ABI, but you get my point...)
> 
> If the hypervisor is duplicitous enough to keep X86_FEATURE_HYPERVISOR
> from getting set, then the hypervisor gets to clean up the mess.  The
> kernel can just wash its hands of the whole thing.
> 
> So, there are two broad cases and a few sub-cases:
> 
> 1. "Nice" hypervisor.  Kernel sees X86_FEATURE_HYPERVISOR and knows that
>    x86_match_cpu() and invlpg_miss_ids[] are irrelevant because:
>  1a.  It is running in VMX non-root mode and is not vulnerable, or
>  1b.  CPUID is a lie and x86_match_cpu() is meaningless, or
>  1c.  The kernel is in ring3 and can't execute INVLPG anyway.  Whatever
>       is running in ring0 will have to deal with it.
> 2. X86_FEATURE_HYPERVISOR is unset.
>  2a. "Mean" hypervisor. All bets are off anyway.
>  2b. Actual bare metal. Actually look for the bug.
> 
> I _think_ I'm OK with skipping the mitigation in all of the #1 cases and
> applying it in both of the #2 cases.  I don't think that checking for
> VMX makes it much better.
> 
> Am I missing anything?

I'm a-ok with just checking HYPERVISOR, I agree that the hypervisor is fully
responsible for correctly emulating PCID and INVLPG stuff for (1c).

My reaction was really just to the changelog equating HYPERVSIOR to VMX non-root.