From ee80838afa518b2351a28c498bcb31820064b07f Mon Sep 17 00:00:00 2001 From: Kun Qin Date: Thu, 29 Oct 2026 13:20:57 -0700 Subject: [PATCH 1/2] [SQUASH on REBASE] Revert "MdeModulePkg/Core/PiSmmCore: Decouple MM core mailbox data" This reverts commit f498d0f03d252b5fb5890c8b270e7fd171d9b882. Signed-off-by: Kun Qin --- MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 8 ++---- MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 36 ++++-------------------- MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 1 - 3 files changed, 7 insertions(+), 38 deletions(-) diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c index e3ec462bb84..2011db1b389 100644 --- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c +++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c @@ -919,9 +919,7 @@ SmmDispatcher ( // // Cache state of SmmEntryPointRegistered before calling entry point // - // MU_CHANGE: Decouple Core private and IPL mailbox - // PreviousSmmEntryPointRegistered = gSmmCorePrivate->SmmEntryPointRegistered; - PreviousSmmEntryPointRegistered = gSmmCoreMailbox->SmmEntryPointRegistered; + PreviousSmmEntryPointRegistered = gSmmCorePrivate->SmmEntryPointRegistered; // // For each SMM driver, pass NULL as ImageHandle @@ -974,9 +972,7 @@ SmmDispatcher ( sizeof (DriverEntry->ImageHandle) ); - // MU_CHANGE: Decouple Core private and IPL mailbox - // if (!PreviousSmmEntryPointRegistered && gSmmCorePrivate->SmmEntryPointRegistered) { - if (!PreviousSmmEntryPointRegistered && gSmmCoreMailbox->SmmEntryPointRegistered) { + if (!PreviousSmmEntryPointRegistered && gSmmCorePrivate->SmmEntryPointRegistered) { // // Return immediately if the SMM Entry Point was registered by the SMM // Driver that was just dispatched. The SMM IPL will reinvoke the SMM diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c index e75342d6775..100e00697c5 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c @@ -12,7 +12,6 @@ // Physical pointer to private structure shared between SMM IPL and the SMM Core // SMM_CORE_PRIVATE_DATA *gSmmCorePrivate; -SMM_CORE_PRIVATE_DATA *gSmmCoreMailbox; // MU_CHANGE: Decouple Core private and IPL mailbox // // SMM Core global variable for SMM System Table. Only accessed as a physical structure in SMRAM. @@ -713,17 +712,7 @@ SmmEntryPoint ( // // Mark the InSmm flag as TRUE, it will be used by SmmBase2 protocol // - // MU_CHANGE Starts: Decouple Core private and IPL mailbox - // gSmmCorePrivate->InSmm = TRUE; - gSmmCoreMailbox->InSmm = TRUE; - - if (gSmmCorePrivate == NULL) { - DEBUG ((DEBUG_ERROR, "%a Internal Core private data is not allocated. No communication allowed!!!\n", __func__)); - ASSERT (FALSE); - goto AsyncSmi; - } - - CopyMem (gSmmCorePrivate, gSmmCoreMailbox, sizeof (*gSmmCoreMailbox)); + gSmmCorePrivate->InSmm = TRUE; // // Check to see if this is a Synchronous SMI sent through the SMM Communication @@ -738,8 +727,8 @@ SmmEntryPoint ( IsOverlapped = InternalIsBufferOverlapped ( (UINT8 *)CommunicationBuffer, BufferSize, - (UINT8 *)gSmmCoreMailbox, // MU_CHANGE Starts: Decouple Core private and IPL mailbox - sizeof (*gSmmCoreMailbox) + (UINT8 *)gSmmCorePrivate, + sizeof (*gSmmCorePrivate) ); // // Check for over or underflows @@ -816,17 +805,10 @@ SmmEntryPoint ( // If a legacy boot has occurred, then make sure gSmmCorePrivate is not accessed // if (!InLegacyBoot) { - // MU_CHANGE Starts: Decouple Core private and IPL mailbox - if (gSmmCorePrivate != NULL) { - CopyMem (gSmmCoreMailbox, gSmmCorePrivate, sizeof (*gSmmCoreMailbox)); - } - // // Clear the InSmm flag as we are going to leave SMM // - // gSmmCorePrivate->InSmm = FALSE; - gSmmCoreMailbox->InSmm = FALSE; - // MU_CHANGE Ends + gSmmCorePrivate->InSmm = FALSE; } PERF_FUNCTION_END (); @@ -942,12 +924,7 @@ SmmMain ( // // Get SMM Core Private context passed in from SMM IPL in ImageHandle. // - // MU_CHANGE Starts: Decouple Core private and IPL mailbox - // gSmmCorePrivate = (SMM_CORE_PRIVATE_DATA *)ImageHandle; - gSmmCoreMailbox = (SMM_CORE_PRIVATE_DATA *)ImageHandle; - gSmmCorePrivate = AllocateCopyPool (sizeof (*gSmmCoreMailbox), gSmmCoreMailbox); - ASSERT (gSmmCorePrivate != NULL); - // MU_CHANGE Ends + gSmmCorePrivate = (SMM_CORE_PRIVATE_DATA *)ImageHandle; // // Fill in SMRAM physical address for the SMM Services Table and the SMM Entry Point. @@ -1007,8 +984,5 @@ SmmMain ( SmmCoreInitializeSmiHandlerProfile (); - // MU_CHANGE: Decouple Core private and IPL mailbox - CopyMem (gSmmCoreMailbox, gSmmCorePrivate, sizeof (*gSmmCoreMailbox)); - return EFI_SUCCESS; } diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h index f6cc6deaac3..b180f5d3c2a 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h @@ -217,7 +217,6 @@ typedef struct { // SMM Core Global Variables // extern SMM_CORE_PRIVATE_DATA *gSmmCorePrivate; -extern SMM_CORE_PRIVATE_DATA *gSmmCoreMailbox; // MU_CHANGE: Decouple Core private and IPL mailbox extern EFI_SMM_SYSTEM_TABLE2 gSmmCoreSmst; extern LIST_ENTRY gHandleList; extern EFI_PHYSICAL_ADDRESS gLoadModuleAtFixAddressSmramBase; From 5434ba8a12a9b9f0215ecc062c096cbfe151a9f8 Mon Sep 17 00:00:00 2001 From: Kun Qin Date: Wed, 16 Feb 2022 20:10:34 +0000 Subject: [PATCH 2/2] [CHERRY-PICK] MdeModulePkg: PiSmmCore: Decouple MM core mailbox data Current `gSmmCorePrivate` data used in PiSmmCore is a shared data structure initialized and used by PiSmmIpl. Thus the content is exposed to normal DXE environment. Specifically, `SmmDispatcher` actively uses `gSmmCorePrivate` to determine dispatch behavior. As a result, the potential attack surface is exposed from SMM initialization through the SMM Ready-to-Lock event. This change decouple Core private data from and IPL mailbox at the entrypoint of each MMI handle routine. It is a preventative fix to eliminate hot access to gSmmCorePrivate within the SMM core. Signed-off-by: Kun Qin (cherry picked from commit 3b54a25765919289f1bc38261b6b2bddb7b20a21) --- MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 4 +-- MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 38 ++++++++++++++++++++---- MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 1 + 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c index 2011db1b389..62307979c09 100644 --- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c +++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c @@ -919,7 +919,7 @@ SmmDispatcher ( // // Cache state of SmmEntryPointRegistered before calling entry point // - PreviousSmmEntryPointRegistered = gSmmCorePrivate->SmmEntryPointRegistered; + PreviousSmmEntryPointRegistered = gSmmCoreMailbox->SmmEntryPointRegistered; // // For each SMM driver, pass NULL as ImageHandle @@ -972,7 +972,7 @@ SmmDispatcher ( sizeof (DriverEntry->ImageHandle) ); - if (!PreviousSmmEntryPointRegistered && gSmmCorePrivate->SmmEntryPointRegistered) { + if (!PreviousSmmEntryPointRegistered && gSmmCoreMailbox->SmmEntryPointRegistered) { // // Return immediately if the SMM Entry Point was registered by the SMM // Driver that was just dispatched. The SMM IPL will reinvoke the SMM diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c index 100e00697c5..0f68c119a20 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c @@ -11,6 +11,14 @@ // // Physical pointer to private structure shared between SMM IPL and the SMM Core // +// WARNING: The mailbox is exposed to the normal world. Its contents must be +// carefully validated before use. +// +SMM_CORE_PRIVATE_DATA *gSmmCoreMailbox; +// +// Pointer to the SMM Core's private copy of the mailbox in MMRAM. The SMM Core +// uses this copy for internal operations. +// SMM_CORE_PRIVATE_DATA *gSmmCorePrivate; // @@ -712,7 +720,15 @@ SmmEntryPoint ( // // Mark the InSmm flag as TRUE, it will be used by SmmBase2 protocol // - gSmmCorePrivate->InSmm = TRUE; + gSmmCoreMailbox->InSmm = TRUE; + + if (gSmmCorePrivate == NULL) { + DEBUG ((DEBUG_ERROR, "%a Internal Core private data is not allocated. No communication allowed!!!\n", __func__)); + ASSERT (FALSE); + goto AsyncSmi; + } + + CopyMem (gSmmCorePrivate, gSmmCoreMailbox, sizeof (*gSmmCoreMailbox)); // // Check to see if this is a Synchronous SMI sent through the SMM Communication @@ -727,8 +743,8 @@ SmmEntryPoint ( IsOverlapped = InternalIsBufferOverlapped ( (UINT8 *)CommunicationBuffer, BufferSize, - (UINT8 *)gSmmCorePrivate, - sizeof (*gSmmCorePrivate) + (UINT8 *)gSmmCoreMailbox, + sizeof (*gSmmCoreMailbox) ); // // Check for over or underflows @@ -805,10 +821,14 @@ SmmEntryPoint ( // If a legacy boot has occurred, then make sure gSmmCorePrivate is not accessed // if (!InLegacyBoot) { + if (gSmmCorePrivate != NULL) { + CopyMem (gSmmCoreMailbox, gSmmCorePrivate, sizeof (*gSmmCoreMailbox)); + } + // // Clear the InSmm flag as we are going to leave SMM // - gSmmCorePrivate->InSmm = FALSE; + gSmmCoreMailbox->InSmm = FALSE; } PERF_FUNCTION_END (); @@ -924,7 +944,12 @@ SmmMain ( // // Get SMM Core Private context passed in from SMM IPL in ImageHandle. // - gSmmCorePrivate = (SMM_CORE_PRIVATE_DATA *)ImageHandle; + gSmmCoreMailbox = (SMM_CORE_PRIVATE_DATA *)ImageHandle; + gSmmCorePrivate = AllocateCopyPool (sizeof (*gSmmCoreMailbox), gSmmCoreMailbox); + ASSERT (gSmmCorePrivate != NULL); + if (gSmmCorePrivate == NULL) { + return EFI_OUT_OF_RESOURCES; + } // // Fill in SMRAM physical address for the SMM Services Table and the SMM Entry Point. @@ -984,5 +1009,8 @@ SmmMain ( SmmCoreInitializeSmiHandlerProfile (); + gSmmCoreMailbox->Smst = gSmmCorePrivate->Smst; + gSmmCoreMailbox->SmmEntryPoint = gSmmCorePrivate->SmmEntryPoint; + return EFI_SUCCESS; } diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h index b180f5d3c2a..ffacea7626b 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h @@ -217,6 +217,7 @@ typedef struct { // SMM Core Global Variables // extern SMM_CORE_PRIVATE_DATA *gSmmCorePrivate; +extern SMM_CORE_PRIVATE_DATA *gSmmCoreMailbox; extern EFI_SMM_SYSTEM_TABLE2 gSmmCoreSmst; extern LIST_ENTRY gHandleList; extern EFI_PHYSICAL_ADDRESS gLoadModuleAtFixAddressSmramBase;