-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Don't Allocate Page 0 #11204
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
Don't Allocate Page 0 #11204
Conversation
UefiPayloadPkg has copied the MdeModulePkg DxeIpl behavior to create a memory allocation HOB for page 0. That is being changed (see that commit for details), so also remove it here. Signed-off-by: Oliver Smith-Denny <[email protected]>
OvmfPkg has copied the MdeModulePkg DxeIpl behavior to create a memory allocation HOB for page 0. That is being changed (see that commit for details), so also remove it here. Signed-off-by: Oliver Smith-Denny <[email protected]>
Currently DxeIpl attempts to set page 0 to all 0's and to create a memory allocation HOB for it. However, DxeIpl will also unmap the page when mapping page tables and if null detection is not enabled, DxeCore will set the page to 0, regardless of allocation status. Because no consumers are using the memory allocation HOB for page 0, drop it. Instead, ensure that PeiCore and DxeCore do not allow allocating page 0; it should always be reserved for null pointer detection. It also complicates the story for platforms that are attempting to audit the system and ensure that no modules are using page 0. With these memory allocation HOBs in place, it is difficult to tell if it is simply DxeIpl who has allocated the memory or another module. This commit drops the memory allocation HOB publishing and ensures that DxeCore and PeiCore do not allocate page 0. DxeCore already will not allocate page 0 to callers of AllocatePages who call with a type other than AllocateAddress, this just changes so that AllocateAddress cannot allocate at page 0 (which if null detection is enabled will cause a page fault). PeiCore does not have AllocateAddress and so this ensures standard allocations do not receive page 0. Signed-off-by: Oliver Smith-Denny <[email protected]>
I know X86 Legacy Boot needs to allocate zero address. This change will not allow this boot mode. Right? |
That is correct. Legacy boot support has been removed from the edk2 repo though. |
Yes, indeed. Legacy boot will be broken after this change. |
As Gerd noted, edk2 no longer supports legacy boot. However, this change doesn’t break legacy boot. Legacy boot can still use page 0. It just can’t allocate it. But that’s fine, nothing else can either, so it is free for legacy boot code to use. I agree all memory corruption is important to catch and I am mid upstreaming more memory subsystem fixes, but that also means we should catch the most common kind of corruption, null pointer usage. This is an age old mitigation and we should support it always. That being said, this PR doesn’t do that. It just makes page 0 non-allocatable. |
Null pointer detection implies the CPU exception handler that catches it. If legacy code starts using page 0 without allocating it, it will need its own exception handler for this, right? |
This PR does not change null pointer detection. That is still controlled through the PCD edk2/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c Lines 786 to 799 in 51d273d
Legacy boot platforms would set this PCD to 0 and not enable null pointer detection. Then it is mapped RW and it can be written to, no exception handlers. All this PR does is not let page 0 be allocated, which again is not a prerequisite to writing to it. |
Got it, thank you. |
@lgao4 the open conversations have been resolved, please re-review. |
Thanks for your detail. I have no other comments. |
@benjamindoron @ChaselChiu @gdong1 @gguo11837463 @jameslu8 @LinusvPelt @Sean-StarLabs @shuoliu0 any review/comment from the UefiPayloadPkg side? The MdeModulePkg and OvmfPkg maintainers have signed off and the UefiPayloadPkg change is just updating the copied driver from those packages, so hopefully should not be controversial. |
Description
Currently DxeIpl attempts to set page 0 to all 0's and to create a memory allocation HOB for it. However, DxeIpl will also unmap the page when mapping page tables and if null detection is not enabled, DxeCore will set the page to 0, regardless of allocation status.
Because no consumers are using the memory allocation HOB for page 0, drop it. Instead, ensure that PeiCore and DxeCore do not allow allocating page 0; it should always be reserved for null pointer detection. It also complicates the story for platforms that are attempting to audit the system and ensure that no modules are using page 0. With these memory allocation HOBs in place, it is difficult to tell if it is simply DxeIpl who has allocated the memory or another module.
This commit drops the memory allocation HOB publishing and ensures that DxeCore and PeiCore do not allocate page 0. DxeCore already will not allocate page 0 to callers of AllocatePages who call with a type other than AllocateAddress, this just changes so that AllocateAddress cannot allocate at page 0 (which if null detection is enabled will cause a page fault). PeiCore does not have AllocateAddress and so this ensures standard allocations do not receive page 0.
How This Was Tested
Tested booting Ovmf and ArmVirt to shell.
Integration Instructions
Any modules attempting to allocate page 0 by address must change what address they are allocating.