From bb248a95091ab542440053d9c289a97e80eb6630 Mon Sep 17 00:00:00 2001 From: Oliver Smith-Denny Date: Mon, 26 Aug 2024 10:23:14 -0700 Subject: [PATCH] MdeModulePkg: MAT Set RO/XP on Code/Data Sections Outside Image Memory The Memory Attributes Table is generated by fetching the EFI memory map and splitting entries which contain loaded images so DATA and CODE sections have separate descriptors. The splitting is done via a call to SplitTable() which marks image DATA sections with the EFI_MEMORY_XP attribute and CODE sections with the EFI_MEMORY_RO attribute when splitting. After this process, there may still be EfiRuntimeServicesCode regions which did not have their attributes set because they are not part of loaded images. This patch updates the MAT EnforceMemoryMapAttribute logic to set the access attributes of runtime memory regions which are not part of loaded images (have not had their access attributes set). The attributes of the code regions will be read-only and no-execute because the UEFI spec dictates that runtime code regions should only contain loaded EFI modules. BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4832 Refs: 1. https://edk2.groups.io/g/devel/topic/patch_v1_mdemodulepkg/105570114?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,105570114 2. https://edk2.groups.io/g/devel/topic/mdemodulepkg_fix_mat/105477564?p=,,,20,0,0,0::recentpostdate/sticky,,,20,2,0,105477564 Signed-off-by: Oliver Smith-Denny --- .../Core/Dxe/Misc/MemoryAttributesTable.c | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c index 5fe285c48b..58b947423a 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c @@ -447,16 +447,23 @@ EnforceMemoryMapAttribute ( MemoryMapEntry = MemoryMap; MemoryMapEnd = (EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + MemoryMapSize); while ((UINTN)MemoryMapEntry < (UINTN)MemoryMapEnd) { - switch (MemoryMapEntry->Type) { - case EfiRuntimeServicesCode: - // do nothing - break; - case EfiRuntimeServicesData: - MemoryMapEntry->Attribute |= EFI_MEMORY_XP; - break; - case EfiReservedMemoryType: - case EfiACPIMemoryNVS: - break; + if ((MemoryMapEntry->Attribute & EFI_MEMORY_ACCESS_MASK) == 0) { + switch (MemoryMapEntry->Type) { + case EfiRuntimeServicesCode: + // If at this point the attributes have not been set on an EfiRuntimeServicesCode + // region, the memory range must not contain a loaded image. It's possible these + // non-image EfiRuntimeServicesCode regions are part of the unused memory bucket. + // It could also be that this region was explicitly allocated outside of the PE + // loader but the UEFI spec requires that all EfiRuntimeServicesCode regions contain + // EFI modules. In either case, set the attributes to RO and XP. + MemoryMapEntry->Attribute |= (EFI_MEMORY_RO | EFI_MEMORY_XP); + break; + case EfiRuntimeServicesData: + MemoryMapEntry->Attribute |= EFI_MEMORY_XP; + break; + default: + break; + } } MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);