From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, thomas.lendacky@amd.com
Cc: Brijesh Singh <brijesh.singh@amd.com>,
James Bottomley <jejb@linux.ibm.com>,
Jordan Justen <jordan.l.justen@intel.com>,
Ard Biesheuvel <ard.biesheuvel@arm.com>
Subject: Re: [edk2-devel] [PATCH v2 03/15] OvmfPkg/ResetVector: Validate the encryption bit position for SEV/SEV-ES
Date: Thu, 7 Jan 2021 15:43:40 +0100 [thread overview]
Message-ID: <600ae771-1fd5-1a77-728d-215c783f4b4c@redhat.com> (raw)
In-Reply-To: <1bc8d8d9f1d6ad1e3640ae0df955087f073dbec2.1609968101.git.thomas.lendacky@amd.com>
On 01/06/21 22:21, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3108
>
> To help mitigate against ROP attacks, add some checks to validate the
> encryption bit position that is reported by the hypervisor.
>
> The first check is to ensure that the hypervisor reports a bit position
> above bit 31. After extracting the encryption bit position from the CPUID
> information, the code checks that the value is above 31. If the value is
> not above 31, then the bit position is not valid, so the code enters a
> HLT loop.
>
> The second check is specific to SEV-ES guests and is a two step process.
> The first step will obtain random data using RDRAND and store that data to
> memory before paging is enabled. When paging is not enabled, all writes to
> memory are encrypted. The random data is maintained in registers, which
> are protected. The second step is that, after enabling paging, the random
> data in memory is compared to the register contents. If they don't match,
> then the reported bit position is not valid, so the code enters a HLT
> loop.
>
> The third check is after switching to 64-bit long mode. Use the fact that
> instruction fetches are automatically decrypted, while a memory fetch is
> decrypted only if the encryption bit is set in the page table. By
> comparing the bytes of an instruction fetch against a memory read of that
> same instruction, the encryption bit position can be validated. If the
> compare is not equal, then SEV/SEV-ES is active but the reported bit
> position is not valid, so the code enters a HLT loop.
>
> To keep the changes local to the OvmfPkg, an OvmfPkg version of the
> Flat32ToFlat64.asm file has been created based on the UefiCpuPkg file
> UefiCpuPkg/ResetVector/Vtf0/Ia32/Flat32ToFlat64.asm.
>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> OvmfPkg/Include/Library/MemEncryptSevLib.h | 4 +
> OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm | 118 ++++++++++++++++++++
> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 13 ++-
> OvmfPkg/ResetVector/ResetVector.nasmb | 4 +-
> 4 files changed, 136 insertions(+), 3 deletions(-)
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
>
> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> index a6d82dac7fac..dc09c61e58bb 100644
> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> @@ -21,10 +21,14 @@
> // This structure is also used by assembler files:
> // OvmfPkg/ResetVector/ResetVector.nasmb
> // OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +// OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> // any changes must stay in sync with its usage.
> //
> typedef struct _SEC_SEV_ES_WORK_AREA {
> UINT8 SevEsEnabled;
> + UINT8 Reserved1[7];
> +
> + UINT64 RandomData;
> } SEC_SEV_ES_WORK_AREA;
>
> /**
> diff --git a/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm b/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> new file mode 100644
> index 000000000000..c6d0d898bcd1
> --- /dev/null
> +++ b/OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
> @@ -0,0 +1,118 @@
> +;------------------------------------------------------------------------------
> +; @file
> +; Transition from 32 bit flat protected mode into 64 bit flat protected mode
> +;
> +; Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +;------------------------------------------------------------------------------
> +
> +BITS 32
> +
> +;
> +; Modified: EAX, ECX, EDX
> +;
> +Transition32FlatTo64Flat:
> +
> + OneTimeCall SetCr3ForPageTables64
> +
> + mov eax, cr4
> + bts eax, 5 ; enable PAE
> + mov cr4, eax
> +
> + mov ecx, 0xc0000080
> + rdmsr
> + bts eax, 8 ; set LME
> + wrmsr
> +
> + ;
> + ; SEV-ES mitigation check support
> + ;
> + xor ebx, ebx
> +
> + cmp byte[SEV_ES_WORK_AREA], 0
> + jz EnablePaging
> +
> + ;
> + ; SEV-ES is active, perform a quick sanity check against the reported
> + ; encryption bit position. This is to help mitigate against attacks where
> + ; the hypervisor reports an incorrect encryption bit position.
> + ;
> + ; This is the first step in a two step process. Before paging is enabled
> + ; writes to memory are encrypted. Using the RDRAND instruction (available
> + ; on all SEV capable processors), write 64-bits of random data to the
> + ; SEV_ES_WORK_AREA and maintain the random data in registers (register
> + ; state is protected under SEV-ES). This will be used in the second step.
> + ;
> +RdRand1:
> + rdrand ecx
> + jnc RdRand1
> + mov dword[SEV_ES_WORK_AREA_RDRAND], ecx
> +RdRand2:
> + rdrand edx
> + jnc RdRand2
> + mov dword[SEV_ES_WORK_AREA_RDRAND + 4], edx
> +
> + ;
> + ; Use EBX instead of the SEV_ES_WORK_AREA memory to determine whether to
> + ; perform the second step.
> + ;
> + mov ebx, 1
> +
> +EnablePaging:
> + mov eax, cr0
> + bts eax, 31 ; set PG
> + mov cr0, eax ; enable paging
> +
> + jmp LINEAR_CODE64_SEL:ADDR_OF(jumpTo64BitAndLandHere)
> +BITS 64
> +jumpTo64BitAndLandHere:
> +
> + ;
> + ; Check if the second step of the SEV-ES mitigation is to be performed.
> + ;
> + test ebx, ebx
> + jz InsnCompare
> +
> + ;
> + ; SEV-ES is active, perform the second step of the encryption bit postion
> + ; mitigation check. The ECX and EDX register contain data from RDRAND that
> + ; was stored to memory in encrypted form. If the encryption bit position is
> + ; valid, the contents of ECX and EDX will match the memory location.
> + ;
> + cmp dword[SEV_ES_WORK_AREA_RDRAND], ecx
> + jne SevEncBitHlt
> + cmp dword[SEV_ES_WORK_AREA_RDRAND + 4], edx
> + jne SevEncBitHlt
> +
> + ;
> + ; If SEV or SEV-ES is active, perform a quick sanity check against
> + ; the reported encryption bit position. This is to help mitigate
> + ; against attacks where the hypervisor reports an incorrect encryption
> + ; bit position. If SEV is not active, this check will always succeed.
> + ;
> + ; The cmp instruction compares the first four bytes of the cmp instruction
> + ; itself (which will be read decrypted if SEV or SEV-ES is active and the
> + ; encryption bit position is valid) against the immediate within the
> + ; instruction (an instruction fetch is always decrypted correctly by
> + ; hardware) based on RIP relative addressing.
> + ;
> +InsnCompare:
> + cmp dword[rel InsnCompare], 0xFFF63D81
> + je GoodCompare
> +
> + ;
> + ; The hypervisor provided an incorrect encryption bit position, do not
> + ; proceed.
> + ;
> +SevEncBitHlt:
> + cli
> + hlt
> + jmp SevEncBitHlt
> +
> +GoodCompare:
> + debugShowPostCode POSTCODE_64BIT_MODE
> +
> + OneTimeCallRet Transition32FlatTo64Flat
> +
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 4032719c3075..ccc95ad4715d 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -140,9 +140,18 @@ GetSevEncBit:
> ; Get pte bit position to enable memory encryption
> ; CPUID Fn8000_001F[EBX] - Bits 5:0
> ;
> + and ebx, 0x3f
> mov eax, ebx
> - and eax, 0x3f
> - jmp SevExit
> +
> + ; The encryption bit position is always above 31
> + sub ebx, 32
> + jns SevExit
> +
> + ; Encryption bit was reported as 31 or below, enter a HLT loop
> +SevEncBitLowHlt:
> + cli
> + hlt
> + jmp SevEncBitLowHlt
>
> NoSev:
> xor eax, eax
> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
> index c5e0fe93abf4..d3aa87982959 100644
> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> @@ -3,6 +3,7 @@
> ; This file includes all other code files to assemble the reset vector code
> ;
> ; Copyright (c) 2008 - 2013, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
> ; SPDX-License-Identifier: BSD-2-Clause-Patent
> ;
> ;------------------------------------------------------------------------------
> @@ -67,13 +68,14 @@
> %endif
>
> %define PT_ADDR(Offset) (FixedPcdGet32 (PcdOvmfSecPageTablesBase) + (Offset))
> -%include "Ia32/Flat32ToFlat64.asm"
>
> %define GHCB_PT_ADDR (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase))
> %define GHCB_BASE (FixedPcdGet32 (PcdOvmfSecGhcbBase))
> %define GHCB_SIZE (FixedPcdGet32 (PcdOvmfSecGhcbSize))
> %define SEV_ES_WORK_AREA (FixedPcdGet32 (PcdSevEsWorkAreaBase))
> + %define SEV_ES_WORK_AREA_RDRAND (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 8)
> %define SEV_ES_VC_TOP_OF_STACK (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) + FixedPcdGet32 (PcdOvmfSecPeiTempRamSize))
> +%include "Ia32/Flat32ToFlat64.asm"
> %include "Ia32/PageTables64.asm"
> %endif
>
>
next prev parent reply other threads:[~2021-01-07 14:43 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-06 21:21 [PATCH v2 00/15] SEV-ES security mitigations Lendacky, Thomas
2021-01-06 21:21 ` [PATCH v2 01/15] Ovmf/ResetVector: Simplify and consolidate the SEV features checks Lendacky, Thomas
2021-01-06 21:21 ` [PATCH v2 02/15] OvmfPkg/Sec: Move SEV-ES SEC workarea definition to common header file Lendacky, Thomas
2021-01-06 21:21 ` [PATCH v2 03/15] OvmfPkg/ResetVector: Validate the encryption bit position for SEV/SEV-ES Lendacky, Thomas
2021-01-07 14:43 ` Laszlo Ersek [this message]
2021-01-06 21:21 ` [PATCH v2 04/15] OvmfPkg/ResetVector: Perform a simple SEV-ES sanity check Lendacky, Thomas
2021-01-07 14:44 ` [edk2-devel] " Laszlo Ersek
2021-01-06 21:21 ` [PATCH v2 05/15] OvmfPkg/MemEncryptSevLib: Save the encryption mask at boot time Lendacky, Thomas
2021-01-07 14:52 ` [edk2-devel] " Laszlo Ersek
2021-01-06 21:21 ` [PATCH v2 06/15] OvmfPkg/MemEncryptSevLib: Add an interface to retrieve the encryption mask Lendacky, Thomas
2021-01-07 15:50 ` [edk2-devel] " Laszlo Ersek
2021-01-06 21:21 ` [PATCH v2 07/15] OvmfPkg/MemEncryptSevLib: Obtain encryption mask using the new interface Lendacky, Thomas
2021-01-07 15:56 ` [edk2-devel] " Laszlo Ersek
2021-01-06 21:21 ` [PATCH v2 08/15] OvmfPkg/AmdSevDxe: Clear encryption bit on PCIe MMCONFIG range Lendacky, Thomas
2021-01-07 17:11 ` [edk2-devel] " Laszlo Ersek
2021-01-06 21:21 ` [PATCH v2 09/15] OvmfPkg/VmgExitLib: Check for an explicit DR7 cached value Lendacky, Thomas
2021-01-06 21:21 ` [PATCH v2 10/15] OvmfPkg/MemEncryptSevLib: Coding style fixes in prep for SEC library Lendacky, Thomas
2021-01-07 17:12 ` [edk2-devel] " Laszlo Ersek
2021-01-06 21:21 ` [PATCH v2 11/15] OvmfPkg/MemEncryptSevLib: Make the MemEncryptSevLib available for SEC Lendacky, Thomas
2021-01-07 17:22 ` [edk2-devel] " Laszlo Ersek
2021-01-06 21:21 ` [PATCH v2 12/15] OvmfPkg/MemEncryptSevLib: Address range encryption state interface Lendacky, Thomas
2021-01-06 21:21 ` [PATCH v2 13/15] OvmfPkg/VmgExitLib: Support nested #VCs Lendacky, Thomas
2021-01-06 21:21 ` [PATCH v2 14/15] OvmfPkg/PlatformPei: Reserve GHCB backup pages if S3 is supported Lendacky, Thomas
2021-01-07 17:25 ` [edk2-devel] " Laszlo Ersek
2021-01-06 21:21 ` [PATCH v2 15/15] OvfmPkg/VmgExitLib: Validate #VC MMIO is to un-encrypted memory Lendacky, Thomas
2021-01-07 17:27 ` [edk2-devel] " Laszlo Ersek
2021-01-07 17:33 ` Lendacky, Thomas
2021-01-07 17:48 ` Laszlo Ersek
2021-01-07 18:37 ` Lendacky, Thomas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=600ae771-1fd5-1a77-728d-215c783f4b4c@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox