loader: fix page table relocation identity mapping - #4380
Closed
Ben Hillis (benhillis) wants to merge 1 commit into
Closed
loader: fix page table relocation identity mapping#4380Ben Hillis (benhillis) wants to merge 1 commit into
Ben Hillis (benhillis) wants to merge 1 commit into
Conversation
The paravisor page table region is placed immediately after the relocation region and is deliberately excluded from it, but the boot identity map uses large pages. A loader performing relocation only fixes up a leaf entry when the region it maps overlaps the relocation region (igvm PageTableRelocationBuilder::recurse_fixup uses RangeMap::get_range, which matches on overlap). When the page table region happens to begin exactly on a large page boundary, the leaf entry mapping it does not overlap the relocation region at all and is left identity mapped at its pre-relocation address. cr3 and the page table pages themselves are relocated, so the relocated cr3 has no mapping and the first access to the page tables through the identity map faults with no IDT loaded, triple faulting the VP during boot shim startup. This has been latent and only reproduces when unrelated image growth lands the region on the boundary. Pad by a page so the page table region always shares a large page with the relocation region. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The change affects early boot/relocation behavior where failures are severe (triple faults) and should receive final human review despite the small diff.
Pull request overview
This PR backports a boot-shim relocation fix into the 1.8 release branch by ensuring the page-table region does not begin exactly on a large-page (2 MiB) boundary, preventing a relocated cr3 from becoming unmapped under large-page identity mapping.
Changes:
- Add a conditional 4 KiB pad in the x64 loader when the post-relocation offset lands on a 2 MiB boundary, so the page-table region shares the same 2 MiB leaf mapping as the relocation region.
- Apply the same conditional padding logic in the arm64 loader (using
Arm64PageSize::Largeas the large-page size). - Add detailed in-code rationale documenting the relocation/identity-map interaction that causes the triple fault.
File summaries
| File | Description |
|---|---|
| vm/loader/src/paravisor.rs | Adds conditional 4 KiB padding before the page-table region for x64 and arm64 to avoid large-page-boundary mis-relocation and prevent early triple faults. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Member
Author
|
Main PR: #4382 |
Member
Author
|
Superseded by #4384, which carries both the loader fix and the image-side mitigation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of the page table relocation fix to
release/1.8.2607. Opened ahead of the main PR because 1.8 is currently red.A relocating loader must keep the page table region identity mapped (VA = PA) after it moves it. Both the OpenVMM loader and the Hyper-V loader only fixed up identity map entries against the
IGVM_VHS_RELOCATABLE_REGIONrange, so a page table region starting exactly on a large page boundary kept its pre-relocation VA, leaving the relocated root unmapped:Latent — the region start is the running total of everything loaded before it, so unrelated image growth decides whether it lands on the boundary. 1.8 landed on
0xca00000and every test that relocates VTL2 failed.Two commits:
The upstream crate fix is microsoft/igvm#135, which fixes this for all consumers of
PageTableRelocationBuilder; the Hyper-V loader needs an equivalent fix separately.