From 1034d223f8cc6bf8b9b86c57e564753cdad46f88 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Wed, 8 Feb 2023 16:34:33 +0100 Subject: ArmPkg/CpuDxe: Perform preliminary NX remap of free memory The DXE core implementation of PcdDxeNxMemoryProtectionPolicy already contains an assertion that EfiConventionalMemory and EfiBootServicesData are subjected to the same policy when it comes to the use of NX permissions. The reason for this is that we may otherwise end up with unbounded recursion in the page table code, given that allocating a page table would then involve a permission attribute change, and this could result in the need for a block entry to be split, which would trigger the allocation of a page table recursively. For the same reason, a shortcut exists in ApplyMemoryProtectionPolicy() where, instead of setting the memory attributes unconditionally, we compare the NX policies and avoid touching the page tables if they are the same for the old and the new memory types. Without this shortcut, we may end up in a situation where, as the CPU arch protocol DXE driver is ramping up, the same unbounded recursion is triggered, due to the fact that the NX policy for EfiConventionalMemory has not been applied yet. To break this cycle, let's remap all EfiConventionalMemory regions according to the NX policy for EfiBootServicesData before exposing the CPU arch protocol to the DXE core and other drivers. This ensures that creating EfiBootServicesData allocations does not result in memory attribute changes, and therefore no recursion. Signed-off-by: Ard Biesheuvel Reviewed-by: Leif Lindholm --- ArmPkg/ArmPkg.dec | 5 +++ ArmPkg/Drivers/CpuDxe/CpuDxe.c | 85 ++++++++++++++++++++++++++++++++++++++++ ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 3 ++ 3 files changed, 93 insertions(+) diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec index 2444457ae5..1a16d044c9 100644 --- a/ArmPkg/ArmPkg.dec +++ b/ArmPkg/ArmPkg.dec @@ -144,6 +144,11 @@ # If PcdMonitorConduitHvc = TRUE, conduit = HVC gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047 + # Whether to remap all unused memory NX before installing the CPU arch + # protocol driver. This is needed on platforms that map all DRAM with RWX + # attributes initially, and can be disabled otherwise. + gArmTokenSpaceGuid.PcdRemapUnusedMemoryNx|TRUE|BOOLEAN|0x00000048 + [PcdsFeatureFlag.ARM] # Whether to map normal memory as non-shareable. FALSE is the safe choice, but # TRUE may be appropriate to fix performance problems if you don't care about diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c index d04958e79e..f820f3f621 100644 --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c @@ -11,6 +11,8 @@ #include +#include + BOOLEAN mIsFlushingGCD; /** @@ -227,6 +229,75 @@ InitializeDma ( CpuArchProtocol->DmaBufferAlignment = ArmCacheWritebackGranule (); } +/** + Map all EfiConventionalMemory regions in the memory map with NX + attributes so that allocating or freeing EfiBootServicesData regions + does not result in changes to memory permission attributes. + +**/ +STATIC +VOID +RemapUnusedMemoryNx ( + VOID + ) +{ + UINT64 TestBit; + UINTN MemoryMapSize; + UINTN MapKey; + UINTN DescriptorSize; + UINT32 DescriptorVersion; + EFI_MEMORY_DESCRIPTOR *MemoryMap; + EFI_MEMORY_DESCRIPTOR *MemoryMapEntry; + EFI_MEMORY_DESCRIPTOR *MemoryMapEnd; + EFI_STATUS Status; + + TestBit = LShiftU64 (1, EfiBootServicesData); + if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & TestBit) == 0) { + return; + } + + MemoryMapSize = 0; + MemoryMap = NULL; + + Status = gBS->GetMemoryMap ( + &MemoryMapSize, + MemoryMap, + &MapKey, + &DescriptorSize, + &DescriptorVersion + ); + ASSERT (Status == EFI_BUFFER_TOO_SMALL); + do { + MemoryMap = (EFI_MEMORY_DESCRIPTOR *)AllocatePool (MemoryMapSize); + ASSERT (MemoryMap != NULL); + Status = gBS->GetMemoryMap ( + &MemoryMapSize, + MemoryMap, + &MapKey, + &DescriptorSize, + &DescriptorVersion + ); + if (EFI_ERROR (Status)) { + FreePool (MemoryMap); + } + } while (Status == EFI_BUFFER_TOO_SMALL); + + ASSERT_EFI_ERROR (Status); + + MemoryMapEntry = MemoryMap; + MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + MemoryMapSize); + while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) { + if (MemoryMapEntry->Type == EfiConventionalMemory) { + ArmSetMemoryRegionNoExec ( + MemoryMapEntry->PhysicalStart, + EFI_PAGES_TO_SIZE (MemoryMapEntry->NumberOfPages) + ); + } + + MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize); + } +} + EFI_STATUS CpuDxeInitialize ( IN EFI_HANDLE ImageHandle, @@ -240,6 +311,20 @@ CpuDxeInitialize ( InitializeDma (&mCpu); + // + // Once we install the CPU arch protocol, the DXE core's memory + // protection routines will invoke them to manage the permissions of page + // allocations as they are created. Given that this includes pages + // allocated for page tables by this driver, we must ensure that unused + // memory is mapped with the same permissions as boot services data + // regions. Otherwise, we may end up with unbounded recursion, due to the + // fact that updating permissions on a newly allocated page table may trigger + // a block entry split, which triggers a page table allocation, etc etc + // + if (FeaturePcdGet (PcdRemapUnusedMemoryNx)) { + RemapUnusedMemoryNx (); + } + Status = gBS->InstallMultipleProtocolInterfaces ( &mCpuHandle, &gEfiCpuArchProtocolGuid, diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf index e732e21cb9..7d8132200e 100644 --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf @@ -48,6 +48,7 @@ DefaultExceptionHandlerLib DxeServicesTableLib HobLib + MemoryAllocationLib PeCoffGetEntryPointLib UefiDriverEntryPoint UefiLib @@ -64,9 +65,11 @@ [Pcd.common] gArmTokenSpaceGuid.PcdVFPEnabled + gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy [FeaturePcd.common] gArmTokenSpaceGuid.PcdDebuggerExceptionSupport + gArmTokenSpaceGuid.PcdRemapUnusedMemoryNx [Depex] gHardwareInterruptProtocolGuid OR gHardwareInterrupt2ProtocolGuid -- cgit v1.2.3