Re: [PATCH] x86/sev: Check whether SEV or SME is supported first

From: Sean Christopherson <seanjc@google.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>, Pu Wen <puwen@hygon.cn>,
	Joerg Roedel <jroedel@suse.de>,
	x86@kernel.org, joro@8bytes.org, dave.hansen@linux.intel.com,
	peterz@infradead.org, tglx@linutronix.de, mingo@redhat.com,
	hpa@zytor.com, sashal@kernel.org, gregkh@linuxfoundation.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH] x86/sev: Check whether SEV or SME is supported first
Date: Tue, 1 Jun 2021 18:08:19 +0000	[thread overview]
Message-ID: <YLZ3k77CK+F9v8fF@google.com> (raw)
In-Reply-To: <YLZy+JR7TNEeNA6C@zn.tnic>

On Tue, Jun 01, 2021, Borislav Petkov wrote:
> On Tue, Jun 01, 2021 at 05:16:12PM +0000, Sean Christopherson wrote:
> > The bug isn't limited to out-of-spec hardware.  At the point of #GP, sme_enable()
> > has only verified the max leaf is greater than 0x8000001f, it has not verified
> > that 0x8000001f is actually supported.  The APM itself declares several leafs
> > between 0x80000000 and 0x8000001f as reserved/unsupported, so we can't argue that
> > 0x8000001f must be supported if the max leaf is greater than 0x8000001f.
> 
> If a hypervisor says that 0x8000001f is supported but then we explode
> when reading MSR_AMD64_SEV, then hypervisor gets to keep both pieces.

But in my scenario, the hypervisor has not said that 0x8000001f is valid, it has
only said that at least one leaf > 0x8000001f is valid.

E.g. if a (virtual) CPU supports CPUID ranges:

  0x80000000 - 0x8000000A
  0x80000020 - 0x80000021

then the below check will pass as eax will be 0x80000021.

	/* Check for the SME/SEV support leaf */
	eax = 0x80000000;
	ecx = 0;
	native_cpuid(&eax, &ebx, &ecx, &edx);
	if (eax < 0x8000001f)
		return;

But we have not yet verified that 0x8000001f is supported, only that the result
of CPUID.0x8000001f can be trusted (to handle Intel CPUs which return data from
the highest supported leaf if the provided leaf function is greater than the max
supported leaf).  Verifying that 0x8000001f is supported doesn't happen until
0x8000001f is actually read, which is currently done after the RDMSR that #GPs
and explodes.

> We're not going to workaround all possible insane hardware/hypervisor
> configurations just because they dropped the ball.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette