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>

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index c4d93f39b9f193359cb8c61eb27c2c13e18a9389..dad8635c3388f44d54714ac8431a7ff60170f43d 100644 (file)
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -237,6 +237,7 @@
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf\r
 !endif\r
   VmgExitLib|OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf\r
+  MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf\r
 \r
 [LibraryClasses.common.PEI_CORE]\r
   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf\r

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
index 8e3b8ddd5a953d21a4d0d3f9980e3c6c1de3609c..f2e162d68076b28ed7946f2830a97e37062bf532 100644 (file)
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
@@ -14,7 +14,7 @@
   FILE_GUID                      = c1594631-3888-4be4-949f-9c630dbc842b\r
   MODULE_TYPE                    = BASE\r
   VERSION_STRING                 = 1.0\r
-  LIBRARY_CLASS                  = MemEncryptSevLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER\r
+  LIBRARY_CLASS                  = MemEncryptSevLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_DRIVER\r
 \r
 #\r
 # The following information is for reference only and not required by the build\r

diff --git a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
index df14de3c21bca9fcada9bee38dd7e217f2f871a1..e6f6ea7972fd3b032a3c4a97be84d79df6e2a468 100644 (file)
--- a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
@@ -35,6 +35,8 @@
   BaseLib\r
   BaseMemoryLib\r
   DebugLib\r
+  LocalApicLib\r
+  MemEncryptSevLib\r
   PcdLib\r
 \r
 [FixedPcd]\r

diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
index b3c3e56ecff8a405043eb371fcde87eb92ea3571..c66c68726cdb236b1c964aa79b24780429084439 100644 (file)
--- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
@@ -35,4 +35,6 @@
   BaseLib\r
   BaseMemoryLib\r
   DebugLib\r
+  LocalApicLib\r
+  MemEncryptSevLib\r
 \r

diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index ce577e4677ebb93c9be62b58af7487d5626b757f..24259060fd652460103698fbbe92205f03420a49 100644 (file)
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -9,6 +9,7 @@
 #include <Base.h>\r
 #include <Uefi.h>\r
 #include <Library/BaseMemoryLib.h>\r
+#include <Library/LocalApicLib.h>\r
 #include <Library/MemEncryptSevLib.h>\r
 #include <Library/VmgExitLib.h>\r
 #include <Register/Amd/Msr.h>\r
@@ -595,6 +596,61 @@ UnsupportedExit (
   return Status;\r
 }\r
 \r
+/**\r
+  Validate that the MMIO memory access is not to encrypted memory.\r
+\r
+  Examine the pagetable entry for the memory specified. MMIO should not be\r
+  performed against encrypted memory. MMIO to the APIC page is always allowed.\r
+\r
+  @param[in] Ghcb           Pointer to the Guest-Hypervisor Communication Block\r
+  @param[in] MemoryAddress  Memory address to validate\r
+  @param[in] MemoryLength   Memory length to validate\r
+\r
+  @retval 0          Memory is not encrypted\r
+  @return            New exception value to propogate\r
+\r
+**/\r
+STATIC\r
+UINT64\r
+ValidateMmioMemory (\r
+  IN GHCB   *Ghcb,\r
+  IN UINTN  MemoryAddress,\r
+  IN UINTN  MemoryLength\r
+  )\r
+{\r
+  MEM_ENCRYPT_SEV_ADDRESS_RANGE_STATE  State;\r
+  GHCB_EVENT_INJECTION                 GpEvent;\r
+  UINTN                                Address;\r
+\r
+  //\r
+  // Allow APIC accesses (which will have the encryption bit set during\r
+  // SEC and PEI phases).\r
+  //\r
+  Address = MemoryAddress & ~(SIZE_4KB - 1);\r
+  if (Address == GetLocalApicBaseAddress ()) {\r
+    return 0;\r
+  }\r
+\r
+  State = MemEncryptSevGetAddressRangeState (\r
+            0,\r
+            MemoryAddress,\r
+            MemoryLength\r
+            );\r
+  if (State == MemEncryptSevAddressRangeUnencrypted) {\r
+    return 0;\r
+  }\r
+\r
+  //\r
+  // Any state other than unencrypted is an error, issue a #GP.\r
+  //\r
+  GpEvent.Uint64 = 0;\r
+  GpEvent.Elements.Vector = GP_EXCEPTION;\r
+  GpEvent.Elements.Type   = GHCB_EVENT_INJECTION_TYPE_EXCEPTION;\r
+  GpEvent.Elements.Valid  = 1;\r
+\r
+  return GpEvent.Uint64;\r
+}\r
+\r
 /**\r
   Handle an MMIO event.\r
 \r
@@ -653,6 +709,11 @@ MmioExit (
       return UnsupportedExit (Ghcb, Regs, InstructionData);\r
     }\r
 \r
+    Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes);\r
+    if (Status != 0) {\r
+      return Status;\r
+    }\r
+\r
     ExitInfo1 = InstructionData->Ext.RmData;\r
     ExitInfo2 = Bytes;\r
     CopyMem (Ghcb->SharedBuffer, &InstructionData->Ext.RegData, Bytes);\r
@@ -683,6 +744,11 @@ MmioExit (
     InstructionData->ImmediateSize = Bytes;\r
     InstructionData->End += Bytes;\r
 \r
+    Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes);\r
+    if (Status != 0) {\r
+      return Status;\r
+    }\r
+\r
     ExitInfo1 = InstructionData->Ext.RmData;\r
     ExitInfo2 = Bytes;\r
     CopyMem (Ghcb->SharedBuffer, InstructionData->Immediate, Bytes);\r
@@ -717,6 +783,11 @@ MmioExit (
       return UnsupportedExit (Ghcb, Regs, InstructionData);\r
     }\r
 \r
+    Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes);\r
+    if (Status != 0) {\r
+      return Status;\r
+    }\r
+\r
     ExitInfo1 = InstructionData->Ext.RmData;\r
     ExitInfo2 = Bytes;\r
 \r
@@ -748,6 +819,11 @@ MmioExit (
   case 0xB7:\r
     Bytes = (Bytes != 0) ? Bytes : 2;\r
 \r
+    Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes);\r
+    if (Status != 0) {\r
+      return Status;\r
+    }\r
+\r
     ExitInfo1 = InstructionData->Ext.RmData;\r
     ExitInfo2 = Bytes;\r
 \r
@@ -774,6 +850,11 @@ MmioExit (
   case 0xBF:\r
     Bytes = (Bytes != 0) ? Bytes : 2;\r
 \r
+    Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes);\r
+    if (Status != 0) {\r
+      return Status;\r
+    }\r
+\r
     ExitInfo1 = InstructionData->Ext.RmData;\r
     ExitInfo2 = Bytes;\r
 \r

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index bfa9dd7cac1fd38230bc20ef434adfdc1bb2162a..70ff2bcf2342a89dcd717a2fe8b39ee6c6e9ed4e 100644 (file)
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -266,6 +266,7 @@
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf\r
 !endif\r
   VmgExitLib|OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf\r
+  MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf\r
 \r
 [LibraryClasses.common.PEI_CORE]\r
   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf\r