From 45cb26c60568b85395be1c3adf2ebcd040a0cfd6 Mon Sep 17 00:00:00 2001 From: Aaron <105021049+apop5@users.noreply.github.com> Date: Fri, 24 May 2024 14:26:36 -0700 Subject: [PATCH] MdeModulePkg\DxeCore: BS memory allocated from SMM Entry points is still RP (#861) ## Description With the completion of #768, memory is by default read protected. If an SMM driver, in its entry point, allocates boot services memory, the existing code would not attempt to apply memory protections to the memory. This resulted in the original permissions continuing to exist. (RP) Removing the check that prevents memory attributes to be applied to memory allocated from within a SMM driver. For each item, place an "x" in between `[` and `]` if true. Example: `[x]`. _(you can also check items in the GitHub UI)_ - [X] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... ## How This Was Tested Verified that after this change, silicon vendor code was still able to allocate boot services memory and access the memory ## Integration Instructions N/A --- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 61 +++++++++++-------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index acdefc4e0b..8535a5e7da 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -43,7 +43,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "DxeMain.h" #include "Mem/HeapGuard.h" -#include "MemoryProtectionSupport.h" +#include "MemoryProtectionSupport.h" // MU_CHANGE // // Image type definitions @@ -172,7 +172,7 @@ GetUefiImageProtectionPolicy ( BOOLEAN InSmm; UINT32 ImageType; UINT32 ProtectionPolicy; - DXE_MEMORY_PROTECTION_SETTINGS *Settings = NULL; + DXE_MEMORY_PROTECTION_SETTINGS *Settings = NULL; // MU_CHANGE // // Check SMM @@ -1109,24 +1109,30 @@ DisableNullDetectionAtTheEndOfDxe ( // MU_CHANGE END +// MU_CHANGE START +// With memory being marked as RP, if a SMM driver makes a BS allocation (from within the +// SMM driver's entry point) the memory will need to have its protection policy +// updated appropiately. + /** Returns whether we are currently executing in SMM mode. **/ -STATIC -BOOLEAN -IsInSmm ( - VOID - ) -{ - BOOLEAN InSmm; - - InSmm = FALSE; - if (gSmmBase2 != NULL) { - gSmmBase2->InSmm (gSmmBase2, &InSmm); - } - - return InSmm; -} +// STATIC +// BOOLEAN +// IsInSmm ( +// VOID +// ) +// { +// BOOLEAN InSmm; +// +// InSmm = FALSE; +// if (gSmmBase2 != NULL) { +// gSmmBase2->InSmm (gSmmBase2, &InSmm); +// } +// +// return InSmm; +// } +// MU_CHANGE END /** Manage memory permission attributes on a memory range, according to the @@ -1157,14 +1163,19 @@ ApplyMemoryProtectionPolicy ( UINT64 OldAttributes; UINT64 NewAttributes; - // - // The policy configured in Dxe NX Protection Policy // MU_CHANGE - // does not apply to allocations performed in SMM mode. - // - if (IsInSmm ()) { - return EFI_SUCCESS; - } - + // MU_CHANGE START + // With memory being marked as RP, if a SMM driver makes a BS allocation (from within the + // SMM driver's entry point) the memory will need to have its protection policy + // updated appropiately based upon the dxe memory protection policy, not the smm + // policy. + // // + // // The policy configured in PcdDxeNxMemoryProtectionPolicy + // // does not apply to allocations performed in SMM mode. + // // + // if (IsInSmm ()) { + // return EFI_SUCCESS; + // } + // MU_CHANGE END // // If the CPU arch protocol is not installed yet, we cannot manage memory // permission attributes, and it is the job of the driver that installs this