microsoft / mu_basecore

Project Mu BaseCore
https://microsoft.github.io/mu/
Other
239 stars 122 forks source link

MdeModulePkg: Compatibility Mode: Only Remap System Memory Regions #1030

Closed os-d closed 1 month ago

os-d commented 2 months ago

Description

When we enter memory protections compatibility mode, we attempt to disable null protection and remap 0 - 0xA0000 as RWX. This was done for x86 systems with broken shim/grubs on Linux that would attempt to use those regions. This resolved that issue and we could boot non-memory protection safe Linux images on x86 HW. However, this approach did not take into account systems that do not have that range marked as system memory, for example ARM64 systems do not have this requirement. As such, this would inappropriately map these regions as RWX when they were not system memory.

This patch updates the remapping to only remap and disable null protection if these ranges are marked as system memory, otherwise it will leave them alone.

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

How This Was Tested

Tested on an ARM64 platform that does not have 0 - 0xA0000 as system memory, as well as an X86 system that does have that range as system memory, booting a Linux image on both that forces us to enter compatibility mode.

Integration Instructions

N/A.

codecov-commenter commented 2 months ago

Codecov Report

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

Project coverage is 1.35%. Comparing base (90adf2b) to head (3f3adb8).

Files Patch % Lines
...eModulePkg/Core/Dxe/Misc/MemoryProtectionSupport.c 0.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## release/202311 #1030 +/- ## ================================================== - Coverage 1.35% 1.35% -0.01% ================================================== Files 1304 1304 Lines 333790 333854 +64 Branches 5103 5103 ================================================== Hits 4512 4512 - Misses 329195 329259 +64 Partials 83 83 ``` | [Flag](https://app.codecov.io/gh/microsoft/mu_basecore/pull/1030/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=microsoft) | Coverage Δ | | |---|---|---| | [MdeModulePkg](https://app.codecov.io/gh/microsoft/mu_basecore/pull/1030/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=microsoft) | `0.68% <0.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=microsoft#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

os-d commented 1 month ago

@spbrogan, refactored to use SafeIntLib and to only use my own fields as the basis for further checks.