diff options
Diffstat (limited to 'MdeModulePkg')
| -rw-r--r-- | MdeModulePkg/Core/Dxe/DxeMain.h | 2 | ||||
| -rw-r--r-- | MdeModulePkg/Core/Dxe/DxeMain.inf | 1 | ||||
| -rw-r--r-- | MdeModulePkg/Core/Dxe/Mem/Page.c | 32 | ||||
| -rw-r--r-- | MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 63 |
4 files changed, 98 insertions, 0 deletions
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h index cd3940d..6f08554 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.h +++ b/MdeModulePkg/Core/Dxe/DxeMain.h @@ -45,6 +45,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include <Protocol/HiiPackageList.h>
#include <Protocol/SmmBase2.h>
#include <Protocol/PeCoffImageEmulator.h>
+#include <Protocol/MemoryAttribute.h>
#include <Guid/MemoryTypeInformation.h>
#include <Guid/FirmwareFileSystem2.h>
#include <Guid/FirmwareFileSystem3.h>
@@ -250,6 +251,7 @@ extern EFI_SECURITY_ARCH_PROTOCOL *gSecurity; extern EFI_SECURITY2_ARCH_PROTOCOL *gSecurity2;
extern EFI_BDS_ARCH_PROTOCOL *gBds;
extern EFI_SMM_BASE2_PROTOCOL *gSmmBase2;
+extern EFI_MEMORY_ATTRIBUTE_PROTOCOL *gMemoryAttributeProtocol;
extern EFI_TPL gEfiCurrentTpl;
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf index cc315ac..40e7dc8 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.inf +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf @@ -156,6 +156,7 @@ gEfiHiiPackageListProtocolGuid ## SOMETIMES_PRODUCES
gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES
gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiMemoryAttributeProtocolGuid ## CONSUMES
# Arch Protocols
gEfiBdsArchProtocolGuid ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c index 9df64cc..27c2788 100644 --- a/MdeModulePkg/Core/Dxe/Mem/Page.c +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c @@ -1746,6 +1746,38 @@ CoreFreePages ( {
EFI_STATUS Status;
EFI_MEMORY_TYPE MemoryType;
+ UINT64 Attributes;
+
+ // check if this memory is returned to the core as RW at a minimum. If the memory attribute protocol is not installed,
+ // then we assume that the memory is RW by default and continue to free it.
+ if (gMemoryAttributeProtocol != NULL) {
+ Status = gMemoryAttributeProtocol->GetMemoryAttributes (
+ gMemoryAttributeProtocol,
+ Memory,
+ EFI_PAGES_TO_SIZE (NumberOfPages),
+ &Attributes
+ );
+
+ // if we failed to get the attributes, or if the memory is read-only or read-protected,
+ // then we leak the memory and return success. This is done because the UEFI spec does not specify whether pages
+ // should be freed with any specific permission attributes. As such, there exist bootloaders in the wild that will
+ // free memory that is marked RO, which can crash the core if DebugClearMemory is enabled or can be passed out to a
+ // driver in the next AllocatePages() call, which can cause a crash later on. It is deemed lower risk to leak the
+ // memory than to attempt to fix up the attributes as that requires syncing the GCD and the page table.
+ if (EFI_ERROR (Status) || (Attributes & EFI_MEMORY_RO) || (Attributes & EFI_MEMORY_RP)) {
+ DEBUG ((
+ DEBUG_WARN,
+ "%a: Memory %llx for %llx Pages failed to get attributes with status %r or it is read-only or read-protected. "
+ "Attributes: %llx. Leaking memory!\n",
+ __func__,
+ Memory,
+ NumberOfPages,
+ Status,
+ Attributes
+ ));
+ return EFI_SUCCESS;
+ }
+ }
Status = CoreInternalFreePages (Memory, NumberOfPages, &MemoryType);
if (!EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index 33357eb..69d26cb 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -38,6 +38,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include <Guid/MemoryAttributesTable.h>
#include <Protocol/FirmwareVolume2.h>
+#include <Protocol/MemoryAttribute.h>
#include <Protocol/SimpleFileSystem.h>
#include "DxeMain.h"
@@ -67,6 +68,8 @@ extern LIST_ENTRY mGcdMemorySpaceMap; STATIC LIST_ENTRY mProtectedImageRecordList;
+EFI_MEMORY_ATTRIBUTE_PROTOCOL *gMemoryAttributeProtocol;
+
/**
Get the image type.
@@ -1034,6 +1037,37 @@ DisableNullDetectionAtTheEndOfDxe ( }
/**
+ A notification for the Memory Attribute Protocol Installation.
+
+ @param[in] Event Event whose notification function is being invoked.
+ @param[in] Context Pointer to the notification function's context,
+ which is implementation-dependent.
+
+**/
+VOID
+EFIAPI
+MemoryAttributeProtocolNotify (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_STATUS Status;
+
+ Status = gBS->LocateProtocol (&gEfiMemoryAttributeProtocolGuid, NULL, (VOID **)&gMemoryAttributeProtocol);
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_INFO,
+ "%a - Unable to locate the memory attribute protocol! Status = %r\n",
+ __func__,
+ Status
+ ));
+ }
+
+ CoreCloseEvent (Event);
+}
+
+/**
Initialize Memory Protection support.
**/
VOID
@@ -1084,6 +1118,35 @@ CoreInitializeMemoryProtection ( );
ASSERT_EFI_ERROR (Status);
+ // Register an event to populate the memory attribute protocol
+ Status = CoreCreateEvent (
+ EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+ MemoryAttributeProtocolNotify,
+ NULL,
+ &Event
+ );
+
+ // if we fail to create the event or the protocol notify, we should still continue, we won't be able to query the
+ // memory attributes on FreePages(), so we may encounter a driver or bootloader that has not set attributes back to
+ // RW, but this matches the state of the world before this protocol was introduced, so it is not a regression.
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a - Failed to create event for the Memory Attribute Protocol notification: %r\n", __func__, Status));
+ ASSERT_EFI_ERROR (Status);
+ }
+
+ // Register for protocol notification
+ Status = CoreRegisterProtocolNotify (
+ &gEfiMemoryAttributeProtocolGuid,
+ Event,
+ &Registration
+ );
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a - Failed to register for the Memory Attribute Protocol notification: %r\n", __func__, Status));
+ ASSERT_EFI_ERROR (Status);
+ }
+
//
// Register a callback to disable NULL pointer detection at EndOfDxe
//
|