edk2
edk2 copied to clipboard
MdeModulePkg: Leak Memory if Not RW on FreePages
Description
Currently, if the DebugClearMemory bit is set in the PcdDebugPropertyMask, CoreConvertPagesEx will attempt to write a pattern to the pages being freed. However, it does not check that the page is writeable, which will cause a page fault if not. Furthermore, if NX protections are not enabled, the core does not ensure that any freed pages are RW, which is the state expected when they are allocated next. If they are not RW, the allocating driver will crash trying to use them.
This patch updates the page freeing code to query the memory attributes protocol, if present, for the attributes. If this call fails or the attributes are not RW at a minimum, the core leaks the memory (returning success to the caller). If the memory attribute protocol is not present (either because a platform doesn't produce it or it is before the protocol has been produced, the core continues with freeing memory. This is either before the CPU Arch protocol is available (so drivers can't change memory attributes) or otherwise matches existing behavior. This was deemed the best approach to let memory that can't be guaranteed to be RW leak instead of letting a driver crash when allocating it. It was deemed less brittle to simply leak the memory instead of attempting to change the attributes.
Closes #11074.
- [ ] Breaking change?
- Breaking change - Does this PR cause a break in build or boot behavior?
- Examples: Does it add a new library class or move a module to a different repo.
- [ ] Impacts security?
- Security - Does this PR have a direct security impact?
- Examples: Crypto algorithm change or buffer overflow fix.
- [ ] Includes tests?
- Tests - Does this PR include any explicit test code?
- Examples: Unit tests or integration tests.
How This Was Tested
Tested by the original reproer using Linux shim v16 and attempting to FreePages that had attributes changed and confirmed that this path now works. Also tested on OVMF booting to shell.
Integration Instructions
N/A.
@lgao4 and @mdkinney please consider merging this for edk2-stable202505. This addresses a crash for the Linux shim v16 on debug builds. When DebugClearMemory is set, DXE Core will currently attempt to write to all freed pages, even if they are not writeable. This patch ensures that DXE Core will change the memory attributes of the free pages before writing to them, or will not write to them if it fails to.
Without this, users of shim v16 will experience failures when using builds of edk2 that have DebugClearMemory set when loading certain Linux distributions.
@ardbiesheuvel @leiflindholm @makubacki @ajfish Please review for edk2-stable202505
@mdkinney @lgao4 can you please re-review this PR? Happy to have greater discussion on direction if we want.
@mdkinney @lersek @lgao4 I have updated this PR (and title/description) per offline discussion. Can you please review? This now simply queries the memory attributes and leaks the memory if it is not RW at a minimum. It does not attempt to set the attributes correctly.
@bluca can you please confirm that this resolves your repro? It boots to OVMF shell for me.
@mdkinney @lgao4 , I need to change the wording in the comment, but please review the functional changes.
@mdkinney @lgao4 , I need to change the wording in the comment, but please review the functional changes.
The change is good to me.