From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Brijesh Singh <brijesh.singh@amd.com>,
Jordan Justen <jordan.l.justen@intel.com>,
Tom Lendacky <thomas.lendacky@amd.com>
Subject: [PATCH 11/12] OvmfPkg/IoMmuDxe: abort harder on memory encryption mask failures
Date: Wed, 2 Aug 2017 23:24:52 +0200 [thread overview]
Message-ID: <20170802212453.19221-12-lersek@redhat.com> (raw)
In-Reply-To: <20170802212453.19221-1-lersek@redhat.com>
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
next prev parent reply other threads:[~2017-08-02 21:23 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-02 21:24 [PATCH 00/12] OvmfPkg/IoMmuDxe: cleanups and fixes Laszlo Ersek
2017-08-02 21:24 ` [PATCH 01/12] OvmfPkg/IoMmuDxe: rewrap source code to 79 characters Laszlo Ersek
2017-08-02 21:24 ` [PATCH 02/12] OvmfPkg/IoMmuDxe: rename DeviceAddress to PlainTextAddress in MAP_INFO Laszlo Ersek
2017-08-02 21:24 ` [PATCH 03/12] OvmfPkg/IoMmuDxe: rename HostAddress to CryptedAddress " Laszlo Ersek
2017-08-02 21:24 ` [PATCH 04/12] OvmfPkg/IoMmuDxe: convert UINTN arguments to UINT64 for the %Lx fmt spec Laszlo Ersek
2017-08-02 21:24 ` [PATCH 05/12] OvmfPkg/IoMmuDxe: don't initialize local variables Laszlo Ersek
2017-08-02 21:24 ` [PATCH 06/12] OvmfPkg/IoMmuDxe: propagate errors from AmdSevInstallIoMmuProtocol() Laszlo Ersek
2017-08-02 21:24 ` [PATCH 07/12] OvmfPkg/IoMmuDxe: clean up used library classes Laszlo Ersek
2017-08-02 21:24 ` [PATCH 08/12] OvmfPkg/IoMmuDxe: zero out pages before releasing them Laszlo Ersek
2017-08-02 21:24 ` [PATCH 09/12] OvmfPkg/IoMmuDxe: rework setup of "MapInfo->PlainTextAddress" in Map() Laszlo Ersek
2017-08-02 21:24 ` [PATCH 10/12] OvmfPkg/IoMmuDxe: implement in-place decryption/encryption for Map/Unmap Laszlo Ersek
2017-08-02 23:01 ` Brijesh Singh
2017-08-03 0:13 ` Laszlo Ersek
2017-08-03 1:09 ` Brijesh Singh
2017-08-03 14:35 ` Brijesh Singh
2017-08-03 14:40 ` Laszlo Ersek
2017-08-02 21:24 ` Laszlo Ersek [this message]
2017-08-02 21:24 ` [PATCH 12/12] OvmfPkg/IoMmuDxe: Unmap(): recycle MAP_INFO after BusMasterCommonBuffer[64] Laszlo Ersek
2017-08-02 21:31 ` [PATCH 00/12] OvmfPkg/IoMmuDxe: cleanups and fixes Laszlo Ersek
2017-08-03 14:10 ` Brijesh Singh
2017-08-03 14:15 ` Laszlo Ersek
2017-08-05 1:25 ` Laszlo Ersek
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=20170802212453.19221-12-lersek@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