From 85b8eac59b8c5bd9c7eb9afdb64357ce1aa2e803 Mon Sep 17 00:00:00 2001 From: Tom Lendacky Date: Thu, 7 Jan 2021 12:48:25 -0600 Subject: OvfmPkg/VmgExitLib: Validate #VC MMIO is to un-encrypted memory BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3108 When SEV-ES is active, and MMIO operation will trigger a #VC and the VmgExitLib exception handler will process this MMIO operation. A malicious hypervisor could try to extract information from encrypted memory by setting a reserved bit in the guests nested page tables for a non-MMIO area. This can result in the encrypted data being copied into the GHCB shared buffer area and accessed by the hypervisor. Prevent this by ensuring that the MMIO source/destination is un-encrypted memory. For the APIC register space, access is allowed in general. Cc: Jordan Justen Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Brijesh Singh Acked-by: Laszlo Ersek Signed-off-by: Tom Lendacky Message-Id: <0cf28470ad5e694af45f7f0b35296628f819567d.1610045305.git.thomas.lendacky@amd.com> --- .../BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf | 2 +- OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 2 + OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 2 + OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 81 ++++++++++++++++++++++ 4 files changed, 86 insertions(+), 1 deletion(-) (limited to 'OvmfPkg/Library') diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf index 8e3b8ddd5a..f2e162d680 100644 --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf @@ -14,7 +14,7 @@ FILE_GUID = c1594631-3888-4be4-949f-9c630dbc842b MODULE_TYPE = BASE VERSION_STRING = 1.0 - LIBRARY_CLASS = MemEncryptSevLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER + LIBRARY_CLASS = MemEncryptSevLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER # # The following information is for reference only and not required by the build diff --git a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf index df14de3c21..e6f6ea7972 100644 --- a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf +++ b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf @@ -35,6 +35,8 @@ BaseLib BaseMemoryLib DebugLib + LocalApicLib + MemEncryptSevLib PcdLib [FixedPcd] diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf index b3c3e56ecf..c66c68726c 100644 --- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf +++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf @@ -35,4 +35,6 @@ BaseLib BaseMemoryLib DebugLib + LocalApicLib + MemEncryptSevLib diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c index ce577e4677..24259060fd 100644 --- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c +++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -595,6 +596,61 @@ UnsupportedExit ( return Status; } +/** + Validate that the MMIO memory access is not to encrypted memory. + + Examine the pagetable entry for the memory specified. MMIO should not be + performed against encrypted memory. MMIO to the APIC page is always allowed. + + @param[in] Ghcb Pointer to the Guest-Hypervisor Communication Block + @param[in] MemoryAddress Memory address to validate + @param[in] MemoryLength Memory length to validate + + @retval 0 Memory is not encrypted + @return New exception value to propogate + +**/ +STATIC +UINT64 +ValidateMmioMemory ( + IN GHCB *Ghcb, + IN UINTN MemoryAddress, + IN UINTN MemoryLength + ) +{ + MEM_ENCRYPT_SEV_ADDRESS_RANGE_STATE State; + GHCB_EVENT_INJECTION GpEvent; + UINTN Address; + + // + // Allow APIC accesses (which will have the encryption bit set during + // SEC and PEI phases). + // + Address = MemoryAddress & ~(SIZE_4KB - 1); + if (Address == GetLocalApicBaseAddress ()) { + return 0; + } + + State = MemEncryptSevGetAddressRangeState ( + 0, + MemoryAddress, + MemoryLength + ); + if (State == MemEncryptSevAddressRangeUnencrypted) { + return 0; + } + + // + // Any state other than unencrypted is an error, issue a #GP. + // + GpEvent.Uint64 = 0; + GpEvent.Elements.Vector = GP_EXCEPTION; + GpEvent.Elements.Type = GHCB_EVENT_INJECTION_TYPE_EXCEPTION; + GpEvent.Elements.Valid = 1; + + return GpEvent.Uint64; +} + /** Handle an MMIO event. @@ -653,6 +709,11 @@ MmioExit ( return UnsupportedExit (Ghcb, Regs, InstructionData); } + Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes); + if (Status != 0) { + return Status; + } + ExitInfo1 = InstructionData->Ext.RmData; ExitInfo2 = Bytes; CopyMem (Ghcb->SharedBuffer, &InstructionData->Ext.RegData, Bytes); @@ -683,6 +744,11 @@ MmioExit ( InstructionData->ImmediateSize = Bytes; InstructionData->End += Bytes; + Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes); + if (Status != 0) { + return Status; + } + ExitInfo1 = InstructionData->Ext.RmData; ExitInfo2 = Bytes; CopyMem (Ghcb->SharedBuffer, InstructionData->Immediate, Bytes); @@ -717,6 +783,11 @@ MmioExit ( return UnsupportedExit (Ghcb, Regs, InstructionData); } + Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes); + if (Status != 0) { + return Status; + } + ExitInfo1 = InstructionData->Ext.RmData; ExitInfo2 = Bytes; @@ -748,6 +819,11 @@ MmioExit ( case 0xB7: Bytes = (Bytes != 0) ? Bytes : 2; + Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes); + if (Status != 0) { + return Status; + } + ExitInfo1 = InstructionData->Ext.RmData; ExitInfo2 = Bytes; @@ -774,6 +850,11 @@ MmioExit ( case 0xBF: Bytes = (Bytes != 0) ? Bytes : 2; + Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes); + if (Status != 0) { + return Status; + } + ExitInfo1 = InstructionData->Ext.RmData; ExitInfo2 = Bytes; -- cgit v1.2.3