Skip to content

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

Merged
merged 5 commits into from
Jul 8, 2025
Merged

Don't Allocate Page 0 #11204

merged 5 commits into from
Jul 8, 2025

Conversation

os-d
Copy link
Contributor

@os-d os-d commented Jun 17, 2025

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.

  • 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 booting Ovmf and ArmVirt to shell.

Integration Instructions

Any modules attempting to allocate page 0 by address must change what address they are allocating.

os-d added 3 commits June 17, 2025 15:26
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]>
@lgao4
Copy link
Contributor

lgao4 commented Jun 23, 2025

I know X86 Legacy Boot needs to allocate zero address. This change will not allow this boot mode. Right?

@kraxel
Copy link
Member

kraxel commented Jun 23, 2025

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.
CSM support was removed in 2023, VbeShim (which needed the zero page too for int10h hacks) followed in 2024.

@olegilyasov
Copy link
Contributor

I know X86 Legacy Boot needs to allocate zero address. This change will not allow this boot mode. Right?

Yes, indeed. Legacy boot will be broken after this change.
In general, NULL pointer detection is a debugging feature that should not be generic. NULL pointer related problems are as "good" as any other memory corruptions.

@os-d
Copy link
Contributor Author

os-d commented Jun 27, 2025

I know X86 Legacy Boot needs to allocate zero address. This change will not allow this boot mode. Right?

Yes, indeed. Legacy boot will be broken after this change. In general, NULL pointer detection is a debugging feature that should not be generic. NULL pointer related problems are as "good" as any other memory corruptions.

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.

@olegilyasov
Copy link
Contributor

I know X86 Legacy Boot needs to allocate zero address. This change will not allow this boot mode. Right?

Yes, indeed. Legacy boot will be broken after this change. In general, NULL pointer detection is a debugging feature that should not be generic. NULL pointer related problems are as "good" as any other memory corruptions.

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?

@os-d
Copy link
Contributor Author

os-d commented Jun 27, 2025

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

//
// Add EFI_MEMORY_RP attribute for page 0 if NULL pointer detection is
// enabled.
//
if ((MemoryMapEntry->PhysicalStart == 0) &&
(PcdGet8 (PcdNullPointerDetectionPropertyMask) != 0))
{
ASSERT (MemoryMapEntry->NumberOfPages > 0);
SetUefiImageMemoryAttributes (
0,
EFI_PAGES_TO_SIZE (1),
EFI_MEMORY_RP | Attributes
);
}
.

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.

@olegilyasov
Copy link
Contributor

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

//
// Add EFI_MEMORY_RP attribute for page 0 if NULL pointer detection is
// enabled.
//
if ((MemoryMapEntry->PhysicalStart == 0) &&
(PcdGet8 (PcdNullPointerDetectionPropertyMask) != 0))
{
ASSERT (MemoryMapEntry->NumberOfPages > 0);
SetUefiImageMemoryAttributes (
0,
EFI_PAGES_TO_SIZE (1),
EFI_MEMORY_RP | Attributes
);
}

.

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.

@os-d
Copy link
Contributor Author

os-d commented Jun 30, 2025

@lgao4 the open conversations have been resolved, please re-review.

@lgao4
Copy link
Contributor

lgao4 commented Jul 1, 2025

@lgao4 the open conversations have been resolved, please re-review.

Thanks for your detail. I have no other comments.

@os-d
Copy link
Contributor Author

os-d commented Jul 1, 2025

@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.

@ardbiesheuvel ardbiesheuvel added the push Auto push patch series in PR if all checks pass label Jul 8, 2025
@mergify mergify bot merged commit 91bb5ce into tianocore:master Jul 8, 2025
130 checks passed
@os-d os-d deleted the page_0_hob branch July 8, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
push Auto push patch series in PR if all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants