Prevent MSRs from leaking across calls to `MultiuseSandbox::restore` by ludfjig · Pull Request #991 · hyperlight-dev/hyperlight

ludfjig · 2025-10-27T19:36:32Z

The goal is to prevent guest state to persist across snapshot-restores.

Kvm: directly disallows read/writes to msrs using KVM_X86_SET_MSR_FILTER. A denied access raises #GP in the guest, surfaces as GuestAborted, and poisons the sandbox.
Hyperv (mshv+whp): The read/writes are allowed (no way to prevent it (intercept exists but doesn't cover all msrs)). Instead restore will reset set msrs using a hardcoded list of MSRs that are known to be writeable. The baseline values of these MSRs are captured on vm-creation. A limitation is that any MSR that is guest-writable and not in this list, has the potential to leak.

Future work:

When scrub-parition hvcall is ready, we will switch to it, which is guaranteed reset all MSRs. (won't exist for kvm though, which is fine).
address Unify mshv and whp partition features #1685
batch calls to read/write msrs in vm creation, to improve perf via decreasing number of hv call. The reason it's all done 1 at at time right now it because on hyperv, read/write multiple msrs is either "all or nothing" so not trivial to batch, since it's expected that some should fail (and we need to know exactly which!). Could also be addressed by above item Unify mshv and whp partition features #1685 to perhaps avoid msr probing entirely, or at least make the number of probed msrs fewer

~~Will mark ready for review once KVM releases new version, which should include newly added KVM_X86_SET_MSR_FILTER vm ioctl that this PR depends on, see rust-vmm/kvm#359~~

Copilot

Pull request overview

This PR prevents x86_64 model-specific register (MSR) state from persisting across snapshot restores by adding explicit MSR capture/reset semantics to snapshots, enforcing an MSR allow-list contract, and (on KVM) denying guest MSR access by default via an MSR filter.

Changes:

Capture MSR reset state into Snapshot (and snapshot-on-disk format), and restore it during MultiUseSandbox::from_snapshot / MultiUseSandbox::restore.
Add SandboxConfiguration::allow_msrs (bounded allow list) and enforce allow-list compatibility on restore (destination must be a superset).
Implement backend-specific MSR handling: KVM deny-by-default filtering + violation reporting; MSHV/WHP MSR read/write support for reset.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/tests/rust_guests/simpleguest/src/main.rs	Adds guest-side RDMSR/WRMSR and SWAPGS helpers for host MSR tests.
src/hyperlight_host/src/sandbox/snapshot/mod.rs	Extends `Snapshot` with optional captured MSRs and snapshot allow list.
src/hyperlight_host/src/sandbox/snapshot/file/mod.rs	Persists/loads MSR fields and validates `msrs`/`allowed_msrs` consistency.
src/hyperlight_host/src/sandbox/snapshot/file/config.rs	Adds MSR fields to OCI snapshot config and serde tests/schema pin updates.
src/hyperlight_host/src/sandbox/snapshot/file_tests.rs	Adds end-to-end disk snapshot tests for MSR capture/compat/allow-list rules.
src/hyperlight_host/src/sandbox/initialized_multi_use.rs	Captures MSRs on snapshot and restores them on `from_snapshot`/`restore` + extensive MSR tests.
src/hyperlight_host/src/sandbox/config.rs	Introduces MSR allow-list storage and capacity-checked `allow_msrs`.
src/hyperlight_host/src/mem/mgr.rs	Threads MSR state through snapshot construction.
src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs	Adds WHP MSR register mapping + get/set MSRs for reset.
src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs	Adds MSHV MSR mapping + get/set MSRs for reset.
src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs	Extends `VirtualMachine` with MSR read/write APIs + validation helpers/errors.
src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs	Implements KVM MSR filter configuration, filtered-exit handling, and KVM MSR get/set.
src/hyperlight_host/src/hypervisor/regs/x86_64/special_regs.rs	Adds test asserting x2APIC stays disabled by default.
src/hyperlight_host/src/hypervisor/regs/x86_64/msrs.rs	New MSR reset table + snapshot validation logic and related constants/tests.
src/hyperlight_host/src/hypervisor/regs/x86_64/mod.rs	Exposes the new MSR module via `pub(crate) use`.
src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs	Captures baseline MSR reset state at VM creation and implements restore logic.
src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs	Wires KVM MSR violations into `RunVmError` and stores `msr_reset` in `HyperlightVm`.
src/hyperlight_host/src/error.rs	Adds `HyperlightError::{MsrReadViolation, MsrWriteViolation}` and marks them poisoning on KVM.
Justfile	Adds ignored host-dependent MSR audit test recipes to `test-isolated`.
docs/msr.md	Documents MSR reset set, backend differences, validation rules, and limitations.
CHANGELOG.md	Adds breaking-change entry for MSR allow-list + restore behavior.
.github/workflows/ValidatePullRequest.yml	Disables matrix `fail-fast` for PR validation workflow.

syntactically

This looks functionally quite good! I left a number of review comments inline, which are mostly about improving clarity / things whose function was not obvious to me.

Beyond what I've explicitly called out in the review comments, I think the high level structure of both the code and the documentation could be a little bit clearer. For the code, I felt like it was not always clear what was HV-specific and what wasn't, and there was just generally a lot of jumping around to follow e.g. the initialisation sequences and compare kvm vs mshv for things like allowlist filtering. For the documentation, there are currently a lot of sections that are all level 2 headings, some of which are related to each other and some of which are not, and it feels like it jumps between different things a few times, making it harder to follow; probably there is some way to use heading structure to organise it a bit more nicely) could be a bit clearer.

syntactically · 2026-07-21T17:28:58Z

+`EFER`, `APIC_BASE`, `FS_BASE`, and `GS_BASE` belong to the special-register
+state.
+
+The two halves are established differently. Resolution checks the


I don't follow this paragraph. Is "Resolution" a specific process? If so, what? What are the "two halves"? Why does "the host-wrtiable half" "hold by construction"? What is mshv doing in here?

I've rewritten msr.md, please take another look

syntactically · 2026-07-21T17:30:30Z

+guest-controlled state and are not reset. Each row below names the guest state
+that persists.
+
+| MSR (index) | Retained guest state |


Do we need to regularly cross-check this table against updates to the hypervisor?

yes, which is far from ideal, and that's why I'd like to switch to scrub hv call asap

syntactically · 2026-07-21T17:33:19Z

+
+* The snapshot's allow list must be a subset of the destination's. A
+  destination that allows at least as much accepts the snapshot.
+* Every supplied index must belong to the destination reset set.


On mshv, are the "supplied index"es in the snapshot "every msr that mshv intercepts" or "just the ones specifically allowed"? If the former, isn't this overly-restrictive? If the latter, isn't this enforced by the previous bullet as well? I assume it's an invariant (independent of snapshotting) that every allowlisted msr is in the reset set.

The supplied indices are every MSR in the source reset set. Yes it is restrictive but without it I think captured MSR state could be lost. If the source snapshot contains an MSR that cannot be restored on the destination, silently omitting it could cause incorrect behavior if the guest depends on that state

I thought the point of the allowed set here is that only the MSRs in the allowed set would be persisted across save/restore because the sandbox ought not to be relying on the behaviour of any MSR that it has not declared to need to use?

syntactically · 2026-07-21T17:33:50Z

+capturing sandbox's allow list. `validate_snapshot` enforces two rules against
+the destination VM's reset set:
+
+* The snapshot's allow list must be a subset of the destination's. A


Why not reinitialise the allow list from the one in the snapshot (changing the filters if necessary)?

My idea is that the allow list is a property/policy of a sandbox, not the snapshot, and restoring a snapshot should not change the MSR policy of the sandbox. To use a different compatible policy, you'd need to create a new sandbox with that policy. The allow list in the snapshot is only used to check that the new sandbox provides the permissions the snapshot expects.

That makes sense; I can see the argument for there being a benefit to the host being able to know that snapshots won't use more msrs than it has declared OK.

syntactically · 2026-07-21T17:36:52Z

+
+## KVM
+
+KVM installs a deny filter over the full MSR space. Allowed indices form the


You probably already checked this but I just wanted to call out the need to be sure that KVM doesn't have any special handling for certain things (I could imagine syscall vectors or something) that bypasses the filter.

yes I checked, everything except 0x800..=0x8ff is correctly denied by the filter filtered

syntactically · 2026-07-21T19:06:31Z

        {
            use mshv_bindings::MSHV_PT_BIT_LAPIC;
-            pr.pt_flags = 1u64 << MSHV_PT_BIT_LAPIC;
+            pr.pt_flags |= 1u64 << MSHV_PT_BIT_LAPIC;


Is this change preserving any specific other flag?

syntactically · 2026-07-21T19:14:18Z

+// PRED_CMD (0x49) and FLUSH_CMD (0x10B) are intentionally absent: they are
+// write-only commands with no retained state, so they cannot be reset and
+// therefore cannot be allowed.
+const MSR_TABLE: &[u32] = &[


Does this need to include the x2APIC accesses for mshv guests which I presume can write APIC_BASE?

No, x2apic requires opt-in partition feature, so any writes there faults. APIC_BASE is restored though

syntactically · 2026-07-21T19:17:27Z

                })?;
+
+            // Restore captured MSR state.
+            #[cfg(target_arch = "x86_64")]


It would be nice if most of this x86_64-specific stuff could be factored out into e.g. arch::handle_arch_specific_state.

I agree, but I'll skip it for this pr if you don't mind

We will eventually need this on ARM as well?

@jsturtevant In re system registers specifically: I think on kvm and hvf both we use delete+recreate vcpu right now, although I haven't tested the scalability limitations or benchmarked it. If we switch away from doing that, then we will need something similar (though a bit different).

However, I would be unsurprised in general if there was some other kind of architecture-specific state we wanted at some point as well. I also would just like to get all the x86 specific stuff off in its corner not muddying up the core abstractions, to the degree that that's possible.

syntactically · 2026-07-21T19:23:55Z

+                } else if kernel_gs_uses_instruction_side_effect && index == KERNEL_GS_BASE {
+                    // Direct WRMSR is denied. The dedicated SWAPGS test proves
+                    // the instruction-side mutation is restored.
+                } else if (index == 0x10 && !kernel_gs_uses_instruction_side_effect)


Why is kernel_gs_uses_instruction_side_effect relevant to TSC?

Also, for the MSRs which have named constants in regs/x86_64/msrs.rs, it would be nice to use the constants in the tests?

whops it's not. Fixed

syntactically · 2026-07-21T19:25:05Z

+                    || matches!(index, 0xE7 | 0xE8)
+                {
+                    assert_guest_counter_is_writable_and_restored(&mut sbox, index);
+                } else if let Some(sentinel) = positive_write_sentinel(index) {


What does "positive_write_sentinel" mean?

it was just some msrs with specific values that should be accepeted. Renamed

ludfjig

I think I addressed your comments. I moved the hyperv specific code to be called from respective backend (whp/mshv) but since it's duplicate code the code is still located outside the backends. Would you prefer the duplicate code appraoch inside each backend isntead?

ludfjig · 2026-07-23T03:38:58Z

+capturing sandbox's allow list. `validate_snapshot` enforces two rules against
+the destination VM's reset set:
+
+* The snapshot's allow list must be a subset of the destination's. A


My idea is that the allow list is a property/policy of a sandbox, not the snapshot, and restoring a snapshot should not change the MSR policy of the sandbox. To use a different compatible policy, you'd need to create a new sandbox with that policy. The allow list in the snapshot is only used to check that the new sandbox provides the permissions the snapshot expects.

ludfjig · 2026-07-23T03:39:34Z

+guest-controlled state and are not reset. Each row below names the guest state
+that persists.
+
+| MSR (index) | Retained guest state |


yes, which is far from ideal, and that's why I'd like to switch to scrub hv call asap

ludfjig · 2026-07-23T03:39:44Z

+`EFER`, `APIC_BASE`, `FS_BASE`, and `GS_BASE` belong to the special-register
+state.
+
+The two halves are established differently. Resolution checks the


I've rewritten msr.md, please take another look

ludfjig · 2026-07-23T04:20:48Z

+
+* The snapshot's allow list must be a subset of the destination's. A
+  destination that allows at least as much accepts the snapshot.
+* Every supplied index must belong to the destination reset set.


The supplied indices are every MSR in the source reset set. Yes it is restrictive but without it I think captured MSR state could be lost. If the source snapshot contains an MSR that cannot be restored on the destination, silently omitting it could cause incorrect behavior if the guest depends on that state

ludfjig · 2026-07-23T04:29:25Z

+
+## KVM
+
+KVM installs a deny filter over the full MSR space. Allowed indices form the


yes I checked, everything except 0x800..=0x8ff is correctly denied by the filter filtered

ludfjig · 2026-07-23T05:50:11Z

+
+    /// The allow list of the sandbox that captured this snapshot,
+    /// sorted and deduplicated. Present exactly when `msrs` is present.
+    /// A restore requires the destination allow list to be a superset.


ludfjig · 2026-07-23T05:50:58Z

        // 8. Build entrypoint + sregs back from the config.
        let entrypoint = NextAction::Call(cfg.entrypoint_addr);

+        // `msrs` and `allowed_msrs` travel together. A config with one but


ludfjig · 2026-07-23T06:03:37Z

                })?;
+
+            // Restore captured MSR state.
+            #[cfg(target_arch = "x86_64")]


I agree, but I'll skip it for this pr if you don't mind

ludfjig · 2026-07-23T06:06:38Z

+                } else if kernel_gs_uses_instruction_side_effect && index == KERNEL_GS_BASE {
+                    // Direct WRMSR is denied. The dedicated SWAPGS test proves
+                    // the instruction-side mutation is restored.
+                } else if (index == 0x10 && !kernel_gs_uses_instruction_side_effect)


whops it's not. Fixed

ludfjig · 2026-07-23T06:08:10Z

+                    || matches!(index, 0xE7 | 0xE8)
+                {
+                    assert_guest_counter_is_writable_and_restored(&mut sbox, index);
+                } else if let Some(sentinel) = positive_write_sentinel(index) {


it was just some msrs with specific values that should be accepeted. Renamed

KVM denies guest MSR access by default. SandboxConfiguration::allow_msrs permits selected MSRs. MSHV and WHP have no per-MSR filter. Hyperlight captures exposed retained MSR state at VM creation and resets it on restore. Captured MSR state persists in OCI snapshots. KVM denials report the MSR index. Unsupported accesses on MSHV and WHP raise a guest general protection fault. Both failures poison the sandbox. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

Active SSP is guest-writable state exposed through the Hyper-V VP register API, not an architectural MSR. Add it to the Hyper-V reset candidates and the WHP register map so restore clears it, while keeping it out of the allow-list surface. Guest tests read SSP and TSC to confirm neither leaks across restore. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

KVM cannot reset active SSP across restore. Removing CET (CPUID leaf 7 SHSTK/IBT) from the guest stops it enabling shadow stacks, so active SSP never changes and the gap is unreachable. IA32_S_CET is then unreadable host-side and rejected from allow_msrs at creation. MSHV and WHP expose CET and reset active SSP instead. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

syntactically · 2026-07-23T20:55:49Z

I didn't have a chance to actually read this yet. A couple of immediate reactions from the comment replies you wrote to me:

I am still not sure I completely understand the semantics of the allow set; I left an inline reply on that
I also had a couple of quick follow-up questions about what testing you did for the vmexit/vmenter emulation

Would you prefer the duplicate code appraoch inside each backend isntead?

Duplicating the code does indeed not seem great. I might go for a mshv_shared module at the same level as kvm/mshv/hvf/whp that has any shared logic for the mshv/whp stuff? This is getting into the weeds of taste now, so feel free to ignore.

Save only the declared guest_msrs plus a fixed core into a snapshot and scrub the rest to the destination baseline on restore. Renames allow_msrs to guest_msrs and stores snapshot MSRs as a flat Vec<MsrEntry>. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

ludfjig force-pushed the reset2 branch 7 times, most recently from aeca04d to 968f2f4 Compare October 31, 2025 00:29

ludfjig force-pushed the reset2 branch from 968f2f4 to 91ba9e0 Compare December 2, 2025 00:27

ludfjig added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Dec 2, 2025

ludfjig force-pushed the reset2 branch 3 times, most recently from e859ff9 to e793129 Compare December 8, 2025 18:42

ludfjig changed the title ~~Reset more state when restoring snapshot~~ Reset MSRs when restoring snapshot Dec 18, 2025

ludfjig force-pushed the reset2 branch from e793129 to 68751e4 Compare February 10, 2026 22:17

ludfjig changed the title ~~Reset MSRs when restoring snapshot~~ Disallow MSR read/writes Feb 10, 2026

ludfjig force-pushed the reset2 branch 5 times, most recently from 8f55249 to 3bb2294 Compare February 13, 2026 23:16

jsturtevant mentioned this pull request Feb 18, 2026

Ensure that system registers don't leak information between uses of a sandbox #791

Open

ludfjig force-pushed the reset2 branch 2 times, most recently from 032168d to c0383dd Compare February 18, 2026 19:27

ludfjig force-pushed the reset2 branch from c0383dd to a84fa74 Compare March 16, 2026 23:04

ludfjig force-pushed the reset2 branch 6 times, most recently from 2e98073 to 089227c Compare July 11, 2026 07:10

ludfjig requested review from andreiltd, dblnz, jprendes, jsturtevant, simongdavies, squillace and syntactically as code owners July 17, 2026 04:12

ludfjig force-pushed the reset2 branch from a4cf1de to ca728b5 Compare July 17, 2026 04:13

Copilot started reviewing on behalf of ludfjig July 17, 2026 04:15 View session

Copilot AI reviewed Jul 17, 2026

View reviewed changes

Comment thread src/hyperlight_host/src/sandbox/snapshot/mod.rs Outdated

Comment thread docs/msr.md Outdated

Comment thread CHANGELOG.md Outdated

Comment thread src/hyperlight_host/src/sandbox/snapshot/file/config.rs

Comment thread src/hyperlight_host/src/sandbox/snapshot/file/config.rs

ludfjig force-pushed the reset2 branch 2 times, most recently from c079263 to 50b829f Compare July 17, 2026 16:12

ludfjig added the ready-for-review PR is ready for (re-)review label Jul 17, 2026

syntactically reviewed Jul 21, 2026

View reviewed changes

ludfjig force-pushed the reset2 branch 5 times, most recently from 87d696b to 3e609eb Compare July 23, 2026 18:52

ludfjig commented Jul 23, 2026

View reviewed changes

ludfjig added 4 commits July 23, 2026 13:50

Pr feedback

0bd2a0a

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

jsturtevant reviewed Jul 23, 2026

View reviewed changes

Comment thread src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs Outdated

jsturtevant reviewed Jul 23, 2026

View reviewed changes

Comment thread src/hyperlight_host/src/hypervisor/virtual_machine/whp.rs Outdated

jsturtevant reviewed Jul 24, 2026

View reviewed changes

Comment thread src/hyperlight_host/src/sandbox/config.rs Outdated

jsturtevant previously approved these changes Jul 24, 2026

View reviewed changes


		## KVM

		KVM installs a deny filter over the full MSR space. Allowed indices form the

Uh oh!

Conversation

ludfjig commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

syntactically left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ludfjig left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ludfjig commented Oct 27, 2025 •

edited

Loading