Skip to content

WHP: releasing file mappings after deleting partition in WhpVm::Drop - #1756

Closed
ludfjig wants to merge 1 commit into
hyperlight-dev:mainfrom
ludfjig:fix-whp-drop-mapping-order
Closed

WHP: releasing file mappings after deleting partition in WhpVm::Drop#1756
ludfjig wants to merge 1 commit into
hyperlight-dev:mainfrom
ludfjig:fix-whp-drop-mapping-order

Conversation

@ludfjig

@ludfjig ludfjig commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Keep mapped-file views alive while WHP may still reference them. With surrogate processes disabled (HYPERLIGHT_MAX_SURROGATES=0), WHvMapGpaRange uses the host view directly. Releasing that view during VM teardown leaves an active GPA mapping backed by an invalid virtual address.

This is a defensive change, I don't know of any current impact

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 21, 2026 21:04
@ludfjig ludfjig added the kind/bugfix For PRs that fix bugs label Aug 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Reorders WHP teardown so mapped-file views remain valid until the partition is deleted.

Changes:

  • Moves mapped-file cleanup after WHvDeletePartition.
  • Prevents invalid host addresses during teardown.
  • Moderate issue: failed partition deletion may still leave active mappings before views are released.
Suppressed comments (1)

src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs:1380

  • This ordering protects the HYPERLIGHT_MAX_SURROGATES=0 path, but the existing no-surrogate test drops only an unmapped VM, while the mapped-file cleanup test runs with the default surrogate mode. Add a Windows regression test that maps a file with surrogates disabled and then drops the VM, so this lifetime requirement is exercised.
        for (handle, view) in self.file_mappings.drain(..) {
            release_file_mapping(view, handle);

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +1379 to +1380
for (handle, view) in self.file_mappings.drain(..) {
release_file_mapping(view, handle);
@ludfjig ludfjig closed this Aug 21, 2026
@ludfjig

ludfjig commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Closing, this was wrong! Whoops

@ludfjig ludfjig reopened this Aug 21, 2026
@ludfjig ludfjig closed this Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bugfix For PRs that fix bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants