Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Enhanced Memory Protections #912

Merged
merged 36 commits into from
Jul 19, 2024

Conversation

os-d
Copy link
Contributor

@os-d os-d commented Jun 13, 2024

Description

This adds enhanced memory protections to Project Mu. This expands the set of memory protections available in edk2 and makes them configurable via a HOB so that memory protections can be dynamically updated for compatibility. See https://microsoft.github.io/mu/WhatAndWhy/enhancedmemoryprotection/ for more details.

It consists of the 2311 commits (greatly squashed, split by package, dropped when not required):

45cb26c
8fb7503
b152f68
ebcd9ab
9ce11dc
9ee166f
27f20fc
9345b79
144e3fe
2a76908
afb07e5
f2e3baf
1983fbd
f1d9992
4ab1191
e689cc2
9a124ec
822b984
22f77a8
f2b4795
aead3c5
272d80d
93e8fab
ae58d20
3ca926d
a462742
eed7092
001fc56
ab4e303
6d3f67c
95f57f3
f97224e
01a3f55
5dce0eb
9c1cc77
3f48112
f54f81f
ad55442
d7f992d
aa159cd
e084763
38601c5
8b96862
ac74e7e
f281c1b
e8d23a5
98c1649
6edaa5c
c00bb26
37fbe8b
6c80787

For each item, place an "x" in between [ and ] if true. Example: [x].
(you can also check items in the GitHub UI)

  • 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

Moving from release/202311 branch.

Integration Instructions

See https://microsoft.github.io/mu/WhatAndWhy/enhancedmemoryprotection/.

@os-d os-d force-pushed the osde/2405_memoryprotections_slim branch from 18978e1 to 3a5a6a1 Compare June 25, 2024 20:04
@github-actions github-actions bot added impact:breaking-change Requires integration attention impact:security Has a security impact impact:testing Affects testing type:documentation Improvements or additions to documentation labels Jun 25, 2024
@os-d os-d force-pushed the osde/2405_memoryprotections_slim branch 14 times, most recently from 6ed2547 to c8b7aad Compare June 28, 2024 23:27
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2024

Codecov Report

Attention: Patch coverage is 0% with 843 lines in your changes missing coverage. Please review.

Please upload report for BASE (release/202405@ab3d90f). Learn more about missing BASE report.

Files Patch % Lines
...MemoryProtectionHobLib/DxeMemoryProtectionHobLib.c 0.00% 156 Missing ⚠️
MdeModulePkg/Core/Dxe/Mem/HeapGuard.c 0.00% 101 Missing ⚠️
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 0.00% 98 Missing ⚠️
MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c 0.00% 88 Missing ⚠️
...yProtectionHobLib/MmCommonMemoryProtectionHobLib.c 0.00% 87 Missing ⚠️
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 0.00% 57 Missing ⚠️
MdeModulePkg/Core/Dxe/Image/Image.c 0.00% 42 Missing ⚠️
MdeModulePkg/Core/PiSmmCore/HeapGuard.c 0.00% 38 Missing ⚠️
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c 0.00% 24 Missing ⚠️
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c 0.00% 24 Missing ⚠️
... and 18 more
Additional details and impacted files
@@                Coverage Diff                @@
##             release/202405     #912   +/-   ##
=================================================
  Coverage                  ?    1.06%           
=================================================
  Files                     ?     1434           
  Lines                     ?   357757           
  Branches                  ?     5350           
=================================================
  Hits                      ?     3823           
  Misses                    ?   353178           
  Partials                  ?      756           
Flag Coverage Δ
MdeModulePkg 0.60% <0.00%> (?)
MdePkg 3.30% <0.00%> (?)
NetworkPkg 0.55% <ø> (?)
UefiCpuPkg 2.87% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@os-d os-d force-pushed the osde/2405_memoryprotections_slim branch 7 times, most recently from 7469ebc to 8589ef4 Compare July 2, 2024 22:05
@os-d os-d requested a review from apop5 July 2, 2024 22:06
@os-d os-d force-pushed the osde/2405_memoryprotections_slim branch 2 times, most recently from 7ddfa2d to 5aeca6a Compare July 3, 2024 16:17
TaylorBeebe and others added 26 commits July 18, 2024 16:33
Consume SMM memory protections from the MmMemoryProtectionHobLib
instead of the PCDs.
This patch adds MemoryProtectionSupport.c which defines helper
functions for managing memory protections in MemoryProtection.c.
In a later patch, MemoryProtection.c will use this functionality.
This patch has MemoryProtection.c use the enhanced memory protections
provided by MemoryProtectionSupport.c as well as use the
MemoryProtectionHobLib instead of the PCDs.
This updates Image.c to get a return status from ProtectUefiImage.
Update PciHostBridgeDxe to map MMIO memory as XP if the
Dxe memory protection HOB specifies that protection.
Move UefiCpuPkg to use the HOB based memory protections instead
of the PCDs.
In preparation for removing them from edk2, comment out memory
protection PCDs.
This adds the ability for a platform to specify special regions
that do not follow the standard memory protections that are enabled
in the memory protection HOB. These regions are instead expected to
have the memory attributes the platform defines.
Add support for the PE/COFF NX_COMPAT flag that may be set in
the optional header. This flag is used to indicate a given image
supports NX memory protections being set on its data sections.
This patch adds support to protect NX compatible images that are
loaded by marking their data sections NX.
MSVC built binaries had section alignment set to 0x20 instead of
0x1000, which is incompatible with the NX_COMPAT flag being set
on the binary and memory protections applied to the sections.
This adds an implementation of the memory attributes protocol to
UefiCpuPkg as defined in the UEFI spec. Support is provided to
get, set, and clear memory attributes.
…Code

With the availability of the memory attribute protocol in x86 and AARCH64,
use it in DxeCore to catch an edge case where the attributes are not
consistent across a range.
This patch adds a protocol to get debug and testing information
from the memory subsytem to validate memory protections. The intent
is to be used from a test application to verify a platform has the
expected memory protections.
This patch adds a definition for a nonstop protocol for memory
protections, which adds the ability to clear page faults that
occur during a boot. This is intended to be a debugging and
testing tool, platforms are expecting to not have this mode
enabled for regular boot.
This patch provides an x86 implementation of the memory nonstop protocol
to clear page faults when they occur to ease debugging and testing of
memory attributes.
This patch adds the ability to mark MP wakeup buffers as RO, to
protect their contents from being overwritten.
This patch adds a log to mark when the GCD sync occurs as this is
a source of major bugs in the memory subsystem, so debugging from
a log is greatly enhanced by knowing when this occurs.
This patch skips looping through the GCD if DEBUG_GCD is not set,
as this is a performance hit to touch every GCD entry just to not
print it.

It also lowers the print level of SetUefiImageMemoryAttributes to
verbose as it is quite a noisy log. While there, it also ensures
that gCpu is available in that function before it is dereferenced.
Linux shim currently incorrectly uses the UEFI memory attribute protocol
causing a page fault. The broken shim does not have the NXCOMPAT
flag, so compatibility mode can be used to uninstall the protocol
when it is loaded. For flexibility, this patch adds a policy
configuration option to allow platforms to choose not to install the
protocol.
This patch uninstalls the memory attributes protocol if the memory
protection settings dictate, to allow booting broken versions of the
Linux shim.
…tion

As described in the enhanced memory protection document, entering
compatibility mode maps page zero, no longer applies attributes to
loaded images, allocates memory as RWX, and uninstalls the memory
attribute protocol. This patch implements compatibility mode in the
DXE Core as it is currently defined. Compatiblity mode is triggered
when a non-NXCOMPAT image is loaded.

This includes a later 202311 addition to Compatibility Mode:

Map Legacy BIOS Memory RWX When Entering Compatibility Mode (microsoft#794)

The Linux distro used in Mariner 2 has logic [directly carves memory
from the legacy BIOS
region](https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2FCBL-Mariner-Linux-Kernel%2Fblob%2Fedab6ad780cfa0be041a08a79b600443fde10c7f%2Farch%2Fx86%2Fboot%2Fcompressed%2Fpgtable_64.c%23L38&data=05%7C02%7CTaylor.Beebe%40microsoft.com%7Cd6a0f39431794d716fc608dc55c9f96b%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638479573168984897%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=2YR1egvp4z%2BCmMu5aeQnZvZegQTRhdVO0vTzIF5cBAc%3D&reserved=0)
and copies the trampoline code into it for execution. Mariner 2 will
trigger memory protection compatibility mode due to the absence of the
NX_COMPAT flag in Shim, but because the UEFI allocator is not used for
this allocation, the memory attributes won't be updated to make the
region executable. To resolve this, compatibility mode will now map the
writable legacy BIOS region (0x0-0xa0000) as RWX to resolve this issue
with Mariner 2 and other Linux distros older than a couple of years.

- [x] Impacts functionality?
  - **Functionality** - Does the change ultimately impact how firmware functions?
  - Examples: Add a new library, publish a new PPI, update an algorithm, ...
- [x] 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

Tested on Q35 booting to Windows and SBSA booting to Ubuntu

Integration Instructions

N/A

- [x] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [x] 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, ...

Tested on Q35 by booting with a recent a Mariner 2 image.

N/A
…Loaded Image Memory (microsoft#822)

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.

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

- [x] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [x] 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, ...
- [x] 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, ...

Tested by Intel EDK2 consumers and on Q35

Project Mu consumers which allocate EfiRuntimeServicesCode regions
outside of the PE loader may experience a break. If runtime executable
code is necessary, this should be done via a loaded EFI module and not a
random allocated buffer. If the EfiRuntimeServicesCode buffer only needs
to be writable, then a buffer of type EfiRuntimeServicesData should be
used instead.
Description

This PR makes the necessary changes to apply EFI_MEMORY_RP on EfiConventionalMemory
and adds a memory protection policy to configure the setting. This replaces the
broken Freed Memory Guard with a simpler policy to set free memory to read protected.
This policy is intended to be used to catch use-after-free scenarios primarily.

This also combines this commit:

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.

- [x] Impacts functionality?
  - **Functionality** - Does the change ultimately impact how firmware functions?
  - Examples: Add a new library, publish a new PPI, update an algorithm, ...
- [x] 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, ...
- [x] 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

Tested by running the DXE Paging Audit on Q35 and SBSA with various memory protection profiles.

Integration Instructions

Platforms which use pre-built binaries of Mu repos will need to rebuild them to sync the memory protection policy between all modules.
Description

Many changes have been made to CoreInitializeMemoryProtection()
to improve the security state. It's gotten to the point that
the code has been essentially rewritten so to improve
readability and maintainability, consolidate the changes
to a separate part of the function to ensure the logic makes sense.

- [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

Tested by booting to Windows on Q35 and and booting to shell
on SBSA

Integration Instructions

N/A
…mory Map

Description

This PR updates a set of ASSERTs checking if memory was successfully
allocated to branch if the allocation failed in addition to ASSERTing.

- [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

Tested by booting Q35 to shell

Integration Instructions

N/A
@os-d os-d force-pushed the osde/2405_memoryprotections_slim branch from 99a5e0d to 52a46f2 Compare July 18, 2024 23:33
Update the feature_memory_protection readme to add a debugging section
and recent changes made.

- [ ] 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, ...
- [x] 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, ...

N/A

N/A
@os-d os-d force-pushed the osde/2405_memoryprotections_slim branch from 52a46f2 to 2ba81bc Compare July 19, 2024 16:54
@os-d os-d merged commit 177912c into microsoft:release/202405 Jul 19, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:breaking-change Requires integration attention impact:security Has a security impact impact:testing Affects testing type:documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants