[edk2] [PATCH 11/12] OvmfPkg/IoMmuDxe: abort harder on memory encryption mask failures

Laszlo Ersek posted 12 patches 7 years, 4 months ago
[edk2] [PATCH 11/12] OvmfPkg/IoMmuDxe: abort harder on memory encryption mask failures
Posted by Laszlo Ersek 7 years, 4 months ago
Upon a MemEncryptSevClearPageEncMask() failure in Map(), it wouldn't be
difficult to release the bounce buffer that was implicitly allocated for
BusMasterRead[64] and BusMasterWrite[64] operations. However, undoing any
partial memory encryption mask changes -- partial page splitting and PTE
modifications -- is practically impossible. (For example, restoring the
encryption mask on the entire range has no reason to fare any better than
the MemEncryptSevClearPageEncMask() call itself.)

For this reason, keep ASSERT_EFI_ERROR(), but hang in RELEASE builds too,
if MemEncryptSevClearPageEncMask() or MemEncryptSevSetPageEncMask() fails.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
index 5049d19e9cb7..ee94cd4efbe2 100644
--- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
+++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
@@ -89,175 +89,178 @@ EFIAPI
 IoMmuMap (
   IN     EDKII_IOMMU_PROTOCOL                       *This,
   IN     EDKII_IOMMU_OPERATION                      Operation,
   IN     VOID                                       *HostAddress,
   IN OUT UINTN                                      *NumberOfBytes,
   OUT    EFI_PHYSICAL_ADDRESS                       *DeviceAddress,
   OUT    VOID                                       **Mapping
   )
 {
   EFI_STATUS                                        Status;
   MAP_INFO                                          *MapInfo;
   EFI_ALLOCATE_TYPE                                 AllocateType;
   COMMON_BUFFER_HEADER                              *CommonBufferHeader;
   VOID                                              *DecryptionSource;
 
   if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL ||
       Mapping == NULL) {
     return EFI_INVALID_PARAMETER;
   }
 
   //
   // Allocate a MAP_INFO structure to remember the mapping when Unmap() is
   // called later.
   //
   MapInfo = AllocatePool (sizeof (MAP_INFO));
   if (MapInfo == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
     goto Failed;
   }
 
   //
   // Initialize the MAP_INFO structure, except the PlainTextAddress field
   //
   MapInfo->Operation         = Operation;
   MapInfo->NumberOfBytes     = *NumberOfBytes;
   MapInfo->NumberOfPages     = EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes);
   MapInfo->CryptedAddress    = (UINTN)HostAddress;
 
   //
   // In the switch statement below, we point "MapInfo->PlainTextAddress" to the
   // plaintext buffer, according to Operation. We also set "DecryptionSource".
   //
   MapInfo->PlainTextAddress = MAX_ADDRESS;
   AllocateType = AllocateAnyPages;
   DecryptionSource = (VOID *)(UINTN)MapInfo->CryptedAddress;
   switch (Operation) {
   //
   // For BusMasterRead[64] and BusMasterWrite[64] operations, a bounce buffer
   // is necessary regardless of whether the original (crypted) buffer crosses
   // the 4GB limit or not -- we have to allocate a separate plaintext buffer.
   // The only variable is whether the plaintext buffer should be under 4GB.
   //
   case EdkiiIoMmuOperationBusMasterRead:
   case EdkiiIoMmuOperationBusMasterWrite:
     MapInfo->PlainTextAddress = BASE_4GB - 1;
     AllocateType = AllocateMaxAddress;
     //
     // fall through
     //
   case EdkiiIoMmuOperationBusMasterRead64:
   case EdkiiIoMmuOperationBusMasterWrite64:
     //
     // Allocate the implicit plaintext bounce buffer.
     //
     Status = gBS->AllocatePages (
                     AllocateType,
                     EfiBootServicesData,
                     MapInfo->NumberOfPages,
                     &MapInfo->PlainTextAddress
                     );
     if (EFI_ERROR (Status)) {
       goto FreeMapInfo;
     }
     break;
 
   //
   // For BusMasterCommonBuffer[64] operations, a to-be-plaintext buffer and a
   // stash buffer (for in-place decryption) have been allocated already, with
   // AllocateBuffer(). We only check whether the address of the to-be-plaintext
   // buffer is low enough for the requested operation.
   //
   case EdkiiIoMmuOperationBusMasterCommonBuffer:
     if ((MapInfo->CryptedAddress > BASE_4GB) ||
         (EFI_PAGES_TO_SIZE (MapInfo->NumberOfPages) >
          BASE_4GB - MapInfo->CryptedAddress)) {
       //
       // CommonBuffer operations cannot be remapped. If the common buffer is
       // above 4GB, then it is not possible to generate a mapping, so return an
       // error.
       //
       Status = EFI_UNSUPPORTED;
       goto FreeMapInfo;
     }
     //
     // fall through
     //
   case EdkiiIoMmuOperationBusMasterCommonBuffer64:
     //
     // The buffer at MapInfo->CryptedAddress comes from AllocateBuffer().
     //
     MapInfo->PlainTextAddress = MapInfo->CryptedAddress;
     //
     // Stash the crypted data.
     //
     CommonBufferHeader = (COMMON_BUFFER_HEADER *)(
                            (UINTN)MapInfo->CryptedAddress - EFI_PAGE_SIZE
                            );
     ASSERT (CommonBufferHeader->Signature == COMMON_BUFFER_SIG);
     CopyMem (
       CommonBufferHeader->StashBuffer,
       (VOID *)(UINTN)MapInfo->CryptedAddress,
       MapInfo->NumberOfBytes
       );
     //
     // Point "DecryptionSource" to the stash buffer so that we decrypt
     // it to the original location, after the switch statement.
     //
     DecryptionSource = CommonBufferHeader->StashBuffer;
     break;
 
   default:
     //
     // Operation is invalid
     //
     Status = EFI_INVALID_PARAMETER;
     goto FreeMapInfo;
   }
 
   //
   // Clear the memory encryption mask on the plaintext buffer.
   //
   Status = MemEncryptSevClearPageEncMask (
              0,
              MapInfo->PlainTextAddress,
              MapInfo->NumberOfPages,
              TRUE
              );
-  ASSERT_EFI_ERROR(Status);
+  ASSERT_EFI_ERROR (Status);
+  if (EFI_ERROR (Status)) {
+    CpuDeadLoop ();
+  }
 
   //
   // If this is a read operation from the Bus Master's point of view,
   // then copy the contents of the real buffer into the mapped buffer
   // so the Bus Master can read the contents of the real buffer.
   //
   // For BusMasterCommonBuffer[64] operations, the CopyMem() below will decrypt
   // the original data (from the stash buffer) back to the original location.
   //
   if (Operation == EdkiiIoMmuOperationBusMasterRead ||
       Operation == EdkiiIoMmuOperationBusMasterRead64 ||
       Operation == EdkiiIoMmuOperationBusMasterCommonBuffer ||
       Operation == EdkiiIoMmuOperationBusMasterCommonBuffer64) {
     CopyMem (
       (VOID *) (UINTN) MapInfo->PlainTextAddress,
       DecryptionSource,
       MapInfo->NumberOfBytes
       );
   }
 
   //
   // Populate output parameters.
   //
   *DeviceAddress = MapInfo->PlainTextAddress;
   *Mapping       = MapInfo;
 
   DEBUG ((
     DEBUG_VERBOSE,
     "%a PlainText 0x%Lx Crypted 0x%Lx Pages 0x%Lx Bytes 0x%Lx\n",
     __FUNCTION__,
     MapInfo->PlainTextAddress,
     MapInfo->CryptedAddress,
     (UINT64)MapInfo->NumberOfPages,
     (UINT64)MapInfo->NumberOfBytes
     ));
 
   return EFI_SUCCESS;
@@ -287,129 +290,132 @@ EFIAPI
 IoMmuUnmap (
   IN  EDKII_IOMMU_PROTOCOL                     *This,
   IN  VOID                                     *Mapping
   )
 {
   MAP_INFO                 *MapInfo;
   EFI_STATUS               Status;
   COMMON_BUFFER_HEADER     *CommonBufferHeader;
   VOID                     *EncryptionTarget;
 
   if (Mapping == NULL) {
     return EFI_INVALID_PARAMETER;
   }
 
   MapInfo = (MAP_INFO *)Mapping;
 
   //
   // For BusMasterWrite[64] operations and BusMasterCommonBuffer[64] operations
   // we have to encrypt the results, ultimately to the original place (i.e.,
   // "MapInfo->CryptedAddress").
   //
   // For BusMasterCommonBuffer[64] operations however, this encryption has to
   // land in-place, so divert the encryption to the stash buffer first.
   //
   EncryptionTarget = (VOID *)(UINTN)MapInfo->CryptedAddress;
 
   switch (MapInfo->Operation) {
   case EdkiiIoMmuOperationBusMasterCommonBuffer:
   case EdkiiIoMmuOperationBusMasterCommonBuffer64:
     ASSERT (MapInfo->PlainTextAddress == MapInfo->CryptedAddress);
 
     CommonBufferHeader = (COMMON_BUFFER_HEADER *)(
                            (UINTN)MapInfo->PlainTextAddress - EFI_PAGE_SIZE
                            );
     ASSERT (CommonBufferHeader->Signature == COMMON_BUFFER_SIG);
     EncryptionTarget = CommonBufferHeader->StashBuffer;
     //
     // fall through
     //
 
   case EdkiiIoMmuOperationBusMasterWrite:
   case EdkiiIoMmuOperationBusMasterWrite64:
     CopyMem (
       EncryptionTarget,
       (VOID *) (UINTN) MapInfo->PlainTextAddress,
       MapInfo->NumberOfBytes
       );
     break;
 
   default:
     //
     // nothing to encrypt after BusMasterRead[64] operations
     //
     break;
   }
 
   DEBUG ((
     DEBUG_VERBOSE,
     "%a PlainText 0x%Lx Crypted 0x%Lx Pages 0x%Lx Bytes 0x%Lx\n",
     __FUNCTION__,
     MapInfo->PlainTextAddress,
     MapInfo->CryptedAddress,
     (UINT64)MapInfo->NumberOfPages,
     (UINT64)MapInfo->NumberOfBytes
     ));
 
   //
   // Restore the memory encryption mask on the area we used to hold the
   // plaintext.
   //
   Status = MemEncryptSevSetPageEncMask (
              0,
              MapInfo->PlainTextAddress,
              MapInfo->NumberOfPages,
              TRUE
              );
-  ASSERT_EFI_ERROR(Status);
+  ASSERT_EFI_ERROR (Status);
+  if (EFI_ERROR (Status)) {
+    CpuDeadLoop ();
+  }
 
   //
   // For BusMasterCommonBuffer[64] operations, copy the stashed data to the
   // original (now encrypted) location.
   //
   // For all other operations, fill the late bounce buffer (which existed as
   // plaintext at some point) with zeros, and then release it.
   //
   if (MapInfo->Operation == EdkiiIoMmuOperationBusMasterCommonBuffer ||
       MapInfo->Operation == EdkiiIoMmuOperationBusMasterCommonBuffer64) {
     CopyMem (
       (VOID *)(UINTN)MapInfo->CryptedAddress,
       CommonBufferHeader->StashBuffer,
       MapInfo->NumberOfBytes
       );
   } else {
     ZeroMem (
       (VOID *)(UINTN)MapInfo->PlainTextAddress,
       EFI_PAGES_TO_SIZE (MapInfo->NumberOfPages)
       );
     gBS->FreePages (MapInfo->PlainTextAddress, MapInfo->NumberOfPages);
   }
 
   //
   // Free the MAP_INFO structure.
   //
   FreePool (Mapping);
   return EFI_SUCCESS;
 }
 
 /**
   Allocates pages that are suitable for an OperationBusMasterCommonBuffer or
   OperationBusMasterCommonBuffer64 mapping.
 
   @param  This                  The protocol instance pointer.
   @param  Type                  This parameter is not used and must be ignored.
   @param  MemoryType            The type of memory to allocate,
                                 EfiBootServicesData or EfiRuntimeServicesData.
   @param  Pages                 The number of pages to allocate.
   @param  HostAddress           A pointer to store the base system memory
                                 address of the allocated range.
   @param  Attributes            The requested bit mask of attributes for the
                                 allocated range.
 
   @retval EFI_SUCCESS           The requested memory pages were allocated.
   @retval EFI_UNSUPPORTED       Attributes is unsupported. The only legal
                                 attribute bits are MEMORY_WRITE_COMBINE and
                                 MEMORY_CACHED.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
 
 **/
-- 
2.13.1.3.g8be5a757fa67


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel