linux-kernel - Re: [PATCH v2 5/7] x86/boot/compressed/64: Add CPUID sanity check to 32-bit boot-path

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: <YEjvBfJg8P1SQt98@google.com>
Date:   Wed, 10 Mar 2021 08:08:37 -0800
From:   Sean Christopherson <seanjc@...gle.com>
To:     Joerg Roedel <joro@...tes.org>
Cc:     x86@...nel.org, Joerg Roedel <jroedel@...e.de>, hpa@...or.com,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Jiri Slaby <jslaby@...e.cz>,
        Dan Williams <dan.j.williams@...el.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        Juergen Gross <jgross@...e.com>,
        Kees Cook <keescook@...omium.org>,
        David Rientjes <rientjes@...gle.com>,
        Cfir Cohen <cfir@...gle.com>,
        Erdem Aktas <erdemaktas@...gle.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Mike Stunes <mstunes@...are.com>,
        Martin Radev <martin.b.radev@...il.com>,
        Arvind Sankar <nivedita@...m.mit.edu>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v2 5/7] x86/boot/compressed/64: Add CPUID sanity check to
 32-bit boot-path

On Wed, Mar 10, 2021, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@...e.de>
> 
> The 32-bit #VC handler has no GHCB and can only handle CPUID exit codes.
> It is needed by the early boot code to handle #VC exceptions raised in
> verify_cpu() and to get the position of the C bit.
> 
> But the CPUID information comes from the hypervisor, which is untrusted
> and might return results which trick the guest into the no-SEV boot path
> with no C bit set in the page-tables. All data written to memory would
> then be unencrypted and could leak sensitive data to the hypervisor.
> 
> Add sanity checks to the 32-bit boot #VC handler to make sure the
> hypervisor does not pretend that SEV is not enabled.
> 
> Signed-off-by: Joerg Roedel <jroedel@...e.de>
> ---
>  arch/x86/boot/compressed/mem_encrypt.S | 36 ++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/arch/x86/boot/compressed/mem_encrypt.S b/arch/x86/boot/compressed/mem_encrypt.S
> index 2ca056a3707c..8941c3a8ff8a 100644
> --- a/arch/x86/boot/compressed/mem_encrypt.S
> +++ b/arch/x86/boot/compressed/mem_encrypt.S
> @@ -145,6 +145,34 @@ SYM_CODE_START(startup32_vc_handler)
>  	jnz	.Lfail
>  	movl	%edx, 0(%esp)		# Store result
>  
> +	/*
> +	 * Sanity check CPUID results from the Hypervisor. See comment in
> +	 * do_vc_no_ghcb() for more details on why this is necessary.
> +	 */
> +
> +	/* Fail if Hypervisor bit not set in CPUID[1].ECX[31] */

This check is flawed, as is the existing check in 64-bit boot.  Or I guess more
accurately, the check in get_sev_encryption_bit() is flawed.  AIUI, SEV-ES
doesn't require the hypervisor to intercept CPUID.  A malicious hypervisor can
temporarily pass-through CPUID to bypass the CPUID[1].ECX[31] check.  The
hypervisor likely has access to the guest firmware source, so it wouldn't be
difficult for the hypervisor to disable CPUID interception once it detects that
firmware is handing over control to the kernel.

> +	cmpl    $1, %ebx
> +	jne     .Lcheck_leaf
> +	btl     $31, 4(%esp)
> +	jnc     .Lfail
> +	jmp     .Ldone
> +
> +.Lcheck_leaf:
> +	/* Fail if SEV leaf not available in CPUID[0x80000000].EAX */
> +	cmpl    $0x80000000, %ebx
> +	jne     .Lcheck_sev
> +	cmpl    $0x8000001f, 12(%esp)
> +	jb      .Lfail
> +	jmp     .Ldone
> +
> +.Lcheck_sev:
> +	/* Fail if SEV bit not set in CPUID[0x8000001f].EAX[1] */
> +	cmpl    $0x8000001f, %ebx
> +	jne     .Ldone
> +	btl     $1, 12(%esp)
> +	jnc     .Lfail
> +
> +.Ldone:
>  	popl	%edx
>  	popl	%ecx
>  	popl	%ebx
> @@ -158,6 +186,14 @@ SYM_CODE_START(startup32_vc_handler)
>  
>  	iret
>  .Lfail:
> +	/* Send terminate request to Hypervisor */
> +	movl    $0x100, %eax
> +	xorl    %edx, %edx
> +	movl    $MSR_AMD64_SEV_ES_GHCB, %ecx
> +	wrmsr
> +	rep; vmmcall
> +
> +	/* If request fails, go to hlt loop */
>  	hlt
>  	jmp .Lfail
>  SYM_CODE_END(startup32_vc_handler)
> -- 
> 2.30.1
>