On 3/12/21 6:38 AM, Joerg Roedel wrote: > From: Joerg Roedel <[email protected]> > > A malicious hypervisor could disable the CPUID intercept for an SEV or > SEV-ES guest and trick it into the no-SEV boot path, where it could > potentially reveal secrets. This is not an issue for SEV-SNP guests, > as the CPUID intercept can't be disabled for those. > > Remove the Hypervisor CPUID bit check from the SEV detection code to > protect against this kind of attack and add a Hypervisor bit equals > zero check to the SME detection path to prevent non-SEV guests from > trying to enable SME. > > This handles the following cases: > > 1) SEV(-ES) guest where CPUID intercept is disabled. The guest > will still see leaf 0x8000001f and the SEV bit. It can > retrieve the C-bit and boot normally. > > 2) Non-SEV guests with intercepted CPUID will check SEV_STATUS > MSR and find it 0 and will try to enable SME. This will > fail when the guest finds MSR_K8_SYSCFG to be zero, as it > is emulated by KVM. But we can't rely on that, as there > might be other hypervisors which return this MSR with bit > 23 set. The Hypervisor bit check will prevent that the > guest tries to enable SME in this case. > > 3) Non-SEV guests on SEV capable hosts with CPUID intercept > disabled (by a malicious hypervisor) will try to boot into > the SME path. This will fail, but it is also not considered > a problem because non-encrypted guests have no protection > against the hypervisor anyway. > > Signed-off-by: Joerg Roedel <[email protected]>
Acked-by: Tom Lendacky <[email protected]> > --- > arch/x86/boot/compressed/mem_encrypt.S | 6 ----- > arch/x86/kernel/sev-es-shared.c | 6 +---- > arch/x86/mm/mem_encrypt_identity.c | 35 ++++++++++++++------------ > 3 files changed, 20 insertions(+), 27 deletions(-) > > diff --git a/arch/x86/boot/compressed/mem_encrypt.S > b/arch/x86/boot/compressed/mem_encrypt.S > index aa561795efd1..a6dea4e8a082 100644 > --- a/arch/x86/boot/compressed/mem_encrypt.S > +++ b/arch/x86/boot/compressed/mem_encrypt.S > @@ -23,12 +23,6 @@ SYM_FUNC_START(get_sev_encryption_bit) > push %ecx > push %edx > > - /* Check if running under a hypervisor */ > - movl $1, %eax > - cpuid > - bt $31, %ecx /* Check the hypervisor bit */ > - jnc .Lno_sev > - > movl $0x80000000, %eax /* CPUID to check the highest leaf */ > cpuid > cmpl $0x8000001f, %eax /* See if 0x8000001f is available */ > diff --git a/arch/x86/kernel/sev-es-shared.c b/arch/x86/kernel/sev-es-shared.c > index cdc04d091242..387b71669818 100644 > --- a/arch/x86/kernel/sev-es-shared.c > +++ b/arch/x86/kernel/sev-es-shared.c > @@ -186,7 +186,6 @@ void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned > long exit_code) > * make it accessible to the hypervisor. > * > * In particular, check for: > - * - Hypervisor CPUID bit > * - Availability of CPUID leaf 0x8000001f > * - SEV CPUID bit. > * > @@ -194,10 +193,7 @@ void __init do_vc_no_ghcb(struct pt_regs *regs, unsigned > long exit_code) > * can't be checked here. > */ > > - if ((fn == 1 && !(regs->cx & BIT(31)))) > - /* Hypervisor bit */ > - goto fail; > - else if (fn == 0x80000000 && (regs->ax < 0x8000001f)) > + if (fn == 0x80000000 && (regs->ax < 0x8000001f)) > /* SEV leaf check */ > goto fail; > else if ((fn == 0x8000001f && !(regs->ax & BIT(1)))) > diff --git a/arch/x86/mm/mem_encrypt_identity.c > b/arch/x86/mm/mem_encrypt_identity.c > index 6c5eb6f3f14f..a19374d26101 100644 > --- a/arch/x86/mm/mem_encrypt_identity.c > +++ b/arch/x86/mm/mem_encrypt_identity.c > @@ -503,14 +503,10 @@ void __init sme_enable(struct boot_params *bp) > > #define AMD_SME_BIT BIT(0) > #define AMD_SEV_BIT BIT(1) > - /* > - * Set the feature mask (SME or SEV) based on whether we are > - * running under a hypervisor. > - */ > - eax = 1; > - ecx = 0; > - native_cpuid(&eax, &ebx, &ecx, &edx); > - feature_mask = (ecx & BIT(31)) ? AMD_SEV_BIT : AMD_SME_BIT; > + > + /* Check the SEV MSR whether SEV or SME is enabled */ > + sev_status = __rdmsr(MSR_AMD64_SEV); > + feature_mask = (sev_status & MSR_AMD64_SEV_ENABLED) ? AMD_SEV_BIT : > AMD_SME_BIT; > > /* > * Check for the SME/SEV feature: > @@ -530,19 +526,26 @@ void __init sme_enable(struct boot_params *bp) > > /* Check if memory encryption is enabled */ > if (feature_mask == AMD_SME_BIT) { > + /* > + * No SME if Hypervisor bit is set. This check is here to > + * prevent a guest from trying to enable SME. For running as a > + * KVM guest the MSR_K8_SYSCFG will be sufficient, but there > + * might be other hypervisors which emulate that MSR as non-zero > + * or even pass it through to the guest. > + * A malicious hypervisor can still trick a guest into this > + * path, but there is no way to protect against that. > + */ > + eax = 1; > + ecx = 0; > + native_cpuid(&eax, &ebx, &ecx, &edx); > + if (ecx & BIT(31)) > + return; > + > /* For SME, check the SYSCFG MSR */ > msr = __rdmsr(MSR_K8_SYSCFG); > if (!(msr & MSR_K8_SYSCFG_MEM_ENCRYPT)) > return; > } else { > - /* For SEV, check the SEV MSR */ > - msr = __rdmsr(MSR_AMD64_SEV); > - if (!(msr & MSR_AMD64_SEV_ENABLED)) > - return; > - > - /* Save SEV_STATUS to avoid reading MSR again */ > - sev_status = msr; > - > /* SEV state cannot be controlled by a command line option */ > sme_me_mask = me_mask; > sev_enabled = true; >
