diff options
| author | Oliver Smith-Denny <osde@microsoft.com> | 2023-04-26 12:04:37 -0700 |
|---|---|---|
| committer | mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> | 2024-08-29 16:11:40 +0000 |
| commit | 1169122c6f22d4db3e44b7b720480522b6933a62 (patch) | |
| tree | dadafdb0029f2cc1adbc6c405dc4421895f2e63d /MdeModulePkg | |
| parent | 01735bbe4a46a6fb7d5d739d0fc5a14897ad18da (diff) | |
| download | edk2-1169122c6f22d4db3e44b7b720480522b6933a62.tar.gz edk2-1169122c6f22d4db3e44b7b720480522b6933a62.tar.bz2 edk2-1169122c6f22d4db3e44b7b720480522b6933a62.zip | |
MdeModulePkg NonDiscoverablePciDeviceIo: MMIO Memory XP By Default
When allocating memory for a non-discoverable PCI device's IO, the
current core code removes the XP attribute, allowing code to execute
from that region. This is a security vulnerability and unneeded. This
change updates to mark the region as XP when allocating memory for the
non-discoverable PCI device.
These allocations in this function are limited to `EfiBootServicesData`
and `EfiRuntimeServicesData`, which we expect to be XP.
Signed-off-by: Aaron Pop <aaronpop@microsoft.com>
Diffstat (limited to 'MdeModulePkg')
| -rw-r--r-- | MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c | 34 |
1 files changed, 32 insertions, 2 deletions
diff --git a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c index e31c38deed..4daf51761b 100644 --- a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c +++ b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c @@ -1111,6 +1111,8 @@ NonCoherentPciIoAllocateBuffer ( NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *Alloc;
VOID *AllocAddress;
+ MemType = EFI_MEMORY_XP;
+
if (HostAddress == NULL) {
return EFI_INVALID_PARAMETER;
}
@@ -1152,9 +1154,9 @@ NonCoherentPciIoAllocateBuffer ( // Use write combining if it was requested, or if it is the only
// type supported by the region.
//
- MemType = EFI_MEMORY_WC;
+ MemType |= EFI_MEMORY_WC;
} else {
- MemType = EFI_MEMORY_UC;
+ MemType |= EFI_MEMORY_UC;
}
Alloc = AllocatePool (sizeof *Alloc);
@@ -1172,6 +1174,34 @@ NonCoherentPciIoAllocateBuffer ( //
InsertHeadList (&Dev->UncachedAllocationList, &Alloc->List);
+ //
+ // Ensure that EFI_MEMORY_XP is in the capability set
+ //
+ if ((GcdDescriptor.Capabilities & EFI_MEMORY_XP) != EFI_MEMORY_XP) {
+ Status = gDS->SetMemorySpaceCapabilities (
+ (PHYSICAL_ADDRESS)(UINTN)AllocAddress,
+ EFI_PAGES_TO_SIZE (Pages),
+ GcdDescriptor.Capabilities | EFI_MEMORY_XP
+ );
+
+ // if we were to fail setting the capability, this would indicate an internal failure of the GCD code. We should
+ // assert here to let a platform know something went crazy, but for a release build we can let the allocation occur
+ // without the EFI_MEMORY_XP bit set, as that was the existing behavior
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a failed to set EFI_MEMORY_XP capability on 0x%llx for length 0x%llx. Attempting to allocate without XP set.\n",
+ __func__,
+ AllocAddress,
+ EFI_PAGES_TO_SIZE (Pages)
+ ));
+
+ ASSERT_EFI_ERROR (Status);
+
+ MemType &= ~EFI_MEMORY_XP;
+ }
+ }
+
Status = gDS->SetMemorySpaceAttributes (
(EFI_PHYSICAL_ADDRESS)(UINTN)AllocAddress,
EFI_PAGES_TO_SIZE (Pages),
|