Skip to content

Commit

Permalink
MdeModulePkg\DxeCore: BS memory allocated from SMM Entry points is st…
Browse files Browse the repository at this point in the history
…ill 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
  • Loading branch information
apop5 authored May 24, 2024
1 parent 13dff4f commit 45cb26c
Showing 1 changed file with 36 additions and 25 deletions.
61 changes: 36 additions & 25 deletions MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 45cb26c

Please sign in to comment.