Mirocom/EDK2-fork
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

MdeModulePkg NonDiscoverablePciDeviceIo: MMIO Memory XP By Default

Browse Source 
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>
This commit is contained in:
Oliver Smith-Denny
2023-04-26 12:04:37 -07:00
committed by mergify[bot]
parent 01735bbe4a
commit 1169122c6f
1 changed files with 32 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
View File

@@ -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),