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 <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>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <0cf28470ad5e694af45f7f0b35296628f819567d.1610045305.git.thomas.lendacky@amd.com>

6 files changed, 88 insertions, 1 deletions

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index c4d93f39b9..dad8635c33 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -237,6 +237,7 @@
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf

 !endif

   VmgExitLib|OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf

+  MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf

 

 [LibraryClasses.common.PEI_CORE]

   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf

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 <Base.h>

 #include <Uefi.h>

 #include <Library/BaseMemoryLib.h>

+#include <Library/LocalApicLib.h>

 #include <Library/MemEncryptSevLib.h>

 #include <Library/VmgExitLib.h>

 #include <Register/Amd/Msr.h>

@@ -596,6 +597,61 @@ UnsupportedExit (
 }

 

 /**

+  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.

 

   Use the VMGEXIT instruction to handle either an MMIO read or an MMIO write.

@@ -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;

 

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index bfa9dd7cac..70ff2bcf23 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -266,6 +266,7 @@
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf

 !endif

   VmgExitLib|OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf

+  MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf

 

 [LibraryClasses.common.PEI_CORE]

   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf