You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
openvmm: an excessive ReadMemory RPC size crashes the VM worker #1167
A management ReadMemory request can panic or abort the VM worker by requesting an impossible buffer size. The handler allocates before checking whether the guest-memory range exists.
In openvmm/openvmm_core/src/worker/dispatch.rs, the LoadedVm::run RPC dispatch does:
Although read_at can return a range error, it runs only after vec! has allocated and initialized size bytes. Capacity overflow panics, and allocator failure can abort the process instead of becoming an RPC error.
Trigger
Send ReadMemory at GPA 0 with size = usize::MAX. A size larger than the guest's RAM also forces an unnecessary allocation before rejection; for a 128 MiB guest, 1 GiB or 1 TiB requests exercise that ordering.
Fix
fix.patch checks address arithmetic and reads the range's final byte before allocating. It then uses try_reserve_exact, initializes the buffer, and performs the complete read. Invalid endpoints and rejected allocations return RPC errors; empty reads retain their existing behavior.
This does not add a general request-size limit. A sparse range with valid endpoints can still allocate before an inaccessible interior byte is detected, and host overcommit can still cause OOM when a very large valid buffer is touched.
Patch against OpenVMM 998b9f2402050cd16d1faa897bc831ef1434be48. Save the contents below as fix.patch and run git apply fix.patch.
fix.patch
diff --git a/openvmm/openvmm_core/src/worker/dispatch.rs b/openvmm/openvmm_core/src/worker/dispatch.rs--- a/openvmm/openvmm_core/src/worker/dispatch.rs+++ b/openvmm/openvmm_core/src/worker/dispatch.rs@@ -4311,11 +4311,23 @@
}
VmRpc::ReadMemory(rpc) => {
rpc.handle_failable_sync(|(gpa, size)| {
- let mut bytes = vec![0u8; size];- self.inner- .gm- .read_at(gpa, bytes.as_mut_slice())- .map(|_| bytes)+ // Read the range's last byte first, so that a range+ // ending outside guest memory fails before its+ // buffer is allocated; then allocate fallibly, so+ // that a size no buffer can have, or the host+ // cannot allocate, fails the request instead of+ // crashing the worker.+ if let Some(last) = size.checked_sub(1) {+ let last_gpa = gpa+ .checked_add(last as u64)+ .context("guest memory read range overflows")?;+ self.inner.gm.read_at(last_gpa, &mut [0])?;+ }+ let mut bytes = Vec::new();+ bytes.try_reserve_exact(size)?;+ bytes.resize(size, 0);+ self.inner.gm.read_at(gpa, bytes.as_mut_slice())?;+ anyhow::Ok(bytes)
});
}
VmRpc::WriteMemory(rpc) => rpc.handle_failable_sync(|(gpa, bytes)| {
diff --git a/openvmm/openvmm_entry/src/vm_controller/microvm/kvm_tests.rs b/openvmm/openvmm_entry/src/vm_controller/microvm/kvm_tests.rs
new file mode 100644
--- /dev/null+++ b/openvmm/openvmm_entry/src/vm_controller/microvm/kvm_tests.rs@@ -0,0 +1,313 @@+// Copyright (c) Microsoft Corporation.+// Licensed under the MIT License.++//! KVM tests of the VM worker's side of a guest-requested microVM snapshot.+//!+//! Each test builds the VM configuration from an `openvmm` command line,+//! launches the VM worker in this process (as `--single-process` does), and+//! takes the controller's place: it receives the guest's snapshot request and+//! sends the snapshot requests itself, in orders the controller does not.+//!+//! The tests need `/dev/kvm` and OpenVMM's Linux direct test kernel and+//! initrd (`.packages/underhill-deps-private/x64/{vmlinux,initrd}` after+//! `cargo xflowey restore-packages`; `OPENVMM_TEST_LINUX_KERNEL` and+//! `OPENVMM_TEST_LINUX_INITRD` select others), so they are ignored by default.+//! Run them with+//!+//! ```text+//! cargo nextest run -p openvmm_entry --run-ignored only -E 'test(/kvm_tests::/)'+//! ```++// The VM worker in this process needs what the `openvmm` binary links.+extern crate openvmm_hypervisors as _;+extern crate openvmm_resources as _;++use crate::Options;+use crate::VmResources;+use crate::meshworker::VmmMesh;+use crate::microvm::MicrovmLaunch;+use chipset_resources::microvm::MicrovmSnapshotScratchPolicy;+use clap::Parser;+use futures::StreamExt;+use mesh::CancelContext;+use mesh::rpc::RpcSend;+use mesh_worker::WorkerEvent;+use mesh_worker::WorkerHandle;+use openvmm_defs::rpc::SnapshotQuiesceError;+use openvmm_defs::rpc::VmRpc;+use openvmm_defs::worker::VM_WORKER;+use openvmm_defs::worker::VmWorkerParameters;+use pal_async::DefaultDriver;+use pal_async::DefaultPool;+use std::path::Path;+use std::path::PathBuf;+use std::time::Duration;+use test_with_tracing::test;+use vmm_core_defs::HaltReason;++/// The guest's init: requests a fresh-scratch snapshot (a write to port+/// 0x605), and once the request returns, powers off with status 7 (port+/// 0x604).+const GUEST_INIT: &str = r#"#!/bin/busybox sh+mount -t devtmpfs devtmpfs /dev 2>/dev/null || true+printf '\000' | dd of=/dev/port bs=1 seek=1541 count=1 conv=notrunc 2>/dev/null+printf '\007' | dd of=/dev/port bs=1 seek=1540 count=1 conv=notrunc 2>/dev/null+while :; do sleep 1; done+"#;+/// A guest's init that never requests a snapshot.+const GUEST_IDLE_INIT: &str = r#"#!/bin/busybox sh+mount -t devtmpfs devtmpfs /dev 2>/dev/null || true+while :; do sleep 1; done+"#;+const GUEST_INIT_PATH: &str = "kvm-test-init";+const GUEST_STATUS: u8 = 7;++/// Time for the guest to boot and request its snapshot.+const BOOT_TIMEOUT: Duration = Duration::from_secs(120);+/// Time for the guest to power off once it runs again.+const GUEST_TIMEOUT: Duration = Duration::from_secs(60);+/// How long a VM that must stay stopped is watched.+const STOPPED_WINDOW: Duration = Duration::from_secs(5);+/// Time for the VM worker to end and tear the VM down.+const EXIT_TIMEOUT: Duration = Duration::from_secs(60);++/// How the VM worker ended, as its host reports it.+#[derive(Debug, PartialEq)]+enum WorkerExit {+ /// It returned normally (`WorkerEvent::Stopped`).+ Stopped,+ /// It returned an error.+ Failed(String),+ /// Its events ended with no final event: the worker's thread unwound (a+ /// panic).+ Vanished,+ /// It had not ended when the wait timed out.+ StillRunning,+}++/// A microVM whose worker runs in this process, with the test as its+/// controller.+struct TestVm {+ rpc: mesh::Sender<VmRpc>,+ halts: mesh::Receiver<HaltReason>,+ snapshot_requests: mesh::Receiver<MicrovmSnapshotScratchPolicy>,+ quiesce_timeout: Duration,+ worker: WorkerHandle,+ mesh: VmmMesh,+ _microvm: MicrovmLaunch,+ _resources: VmResources,+}++impl TestVm {+ /// Launches a 1-VP microVM on KVM that runs [`GUEST_INIT`], with RAM in a+ /// snapshot destination's automatic backing under `dir`, and starts it.+ async fn start(driver: &DefaultDriver, dir: &Path) -> anyhow::Result<Self> {+ Self::start_with_init(driver, dir, GUEST_INIT).await+ }++ /// As [`Self::start`], with the guest running `init` instead.+ async fn start_with_init(+ driver: &DefaultDriver,+ dir: &Path,+ init: &str,+ ) -> anyhow::Result<Self> {+ let vm = Self::launch(driver, dir, init, None).await?;+ anyhow::ensure!(+ vm.rpc.call_failable(VmRpc::Resume, ()).await?,+ "the new VM was already running"+ );+ Ok(vm)+ }++ /// As [`Self::start_with_init`], without starting the VM, and with the+ /// worker given `ready_sink` as its restore readiness sink.+ async fn launch(+ driver: &DefaultDriver,+ dir: &Path,+ init: &str,+ ready_sink: Option<std::fs::File>,+ ) -> anyhow::Result<Self> {+ anyhow::ensure!(Path::new("/dev/kvm").exists(), "this test needs /dev/kvm");+ let kernel = test_artifact("OPENVMM_TEST_LINUX_KERNEL", "vmlinux");+ let base_initrd = test_artifact("OPENVMM_TEST_LINUX_INITRD", "initrd");+ let initrd = dir.join("initrd");+ std::fs::write(+ &initrd,+ initrd_cpio::inject_into_initrd(+ &fs_err::read(&base_initrd)?,+ GUEST_INIT_PATH,+ init.as_bytes(),+ 0o100755,+ )?,+ )?;+ let path = |path: &Path| path.to_str().unwrap().to_owned();+ let mut opt = Options::try_parse_from([+ "openvmm".to_owned(),+ "--machine".to_owned(),+ "microvm".to_owned(),+ "--hypervisor".to_owned(),+ "kvm".to_owned(),+ "--processors".to_owned(),+ "1".to_owned(),+ "--memory".to_owned(),+ "128M".to_owned(),+ "--kernel".to_owned(),+ path(&kernel),+ "--initrd".to_owned(),+ path(&initrd),+ "--cmdline".to_owned(),+ format!("rdinit=/{GUEST_INIT_PATH}"),+ "--snapshot-destination".to_owned(),+ path(&dir.join("snapshot")),+ "--single-process".to_owned(),+ ])?;++ // As `run_control_inner` builds and launches the VM worker.+ let mesh = VmmMesh::new(driver, opt.single_process)?;+ let restore = crate::snapshot_restore::SnapshotRestore::open(&opt)?;+ let microvm_restore = crate::microvm::prepare_restore(&mut opt, restore.snapshot())?;+ let (vm_config, mut resources) =+ crate::vm_config_from_command_line(driver, &mesh, &opt, µvm_restore).await?;+ let mut microvm = MicrovmLaunch::new(+ &opt,+ &vm_config,+ std::mem::take(&mut resources.microvm),+ microvm_restore,+ )?;+ let (rpc, rpc_recv) = mesh::channel();+ let (notify_send, halts) = mesh::channel();+ let (snapshot_boundary_requests, snapshot_ready, snapshot_requests) =+ microvm.snapshot_channels();+ let hypervisor =+ openvmm_helpers::hypervisor::hypervisor_resource(opt.hypervisor.as_deref().unwrap())?;+ let shared_memory = microvm.capture_shared_memory()?;+ let restore = restore.into_worker();+ let params = VmWorkerParameters {+ hypervisor,+ cfg: vm_config,+ saved_state: None,+ shared_memory,+ shared_memory_copy_on_write: restore.shared_memory_copy_on_write,+ snapshot_restore_guards: restore.guards,+ snapshot_boundary_requests,+ snapshot_ready,+ restore_time: restore.time,+ restore_ready_sink: ready_sink,+ restore_gate_timeout: microvm.restore_gate_timeout(&opt),+ restore_vp_count: opt.microvm.restore_processors,+ rpc: rpc_recv,+ notify: notify_send,+ };+ let worker = mesh+ .make_host("vm", None)+ .await?+ .launch_worker(VM_WORKER, params)+ .await?;++ Ok(Self {+ rpc,+ halts,+ snapshot_requests: snapshot_requests+ .ok_or_else(|| anyhow::anyhow!("the microVM has no snapshot-request port"))?,+ quiesce_timeout: Duration::from_millis(opt.microvm.snapshot_quiesce_timeout_ms),+ worker,+ mesh,+ _microvm: microvm,+ _resources: resources,+ })+ }++ /// Waits until the worker holds the guest's snapshot request at its+ /// boundary and notifies the controller.+ async fn snapshot_boundary(&mut self) -> anyhow::Result<MicrovmSnapshotScratchPolicy> {+ Ok(CancelContext::new()+ .with_timeout(BOOT_TIMEOUT)+ .until_cancelled(self.snapshot_requests.recv())+ .await+ .map_err(|_| anyhow::anyhow!("the guest did not request a snapshot in time"))??)+ }++ /// Returns the guest's next halt within `timeout`, if any.+ async fn next_halt(&mut self, timeout: Duration) -> anyhow::Result<Option<HaltReason>> {+ match CancelContext::new()+ .with_timeout(timeout)+ .until_cancelled(self.halts.recv())+ .await+ {+ Ok(halt) => Ok(Some(halt?)),+ Err(_) => Ok(None),+ }+ }++ /// Waits up to `timeout` for the VM worker to end on its own, and returns+ /// how it ended.+ async fn worker_exit(&mut self, timeout: Duration) -> WorkerExit {+ let worker = &mut self.worker;+ let ended = async move {+ while let Some(event) = worker.next().await {+ match event {+ WorkerEvent::Stopped => return WorkerExit::Stopped,+ WorkerEvent::Failed(err) => return WorkerExit::Failed(format!("{err:#}")),+ WorkerEvent::Started | WorkerEvent::RestartFailed(_) => {}+ }+ }+ WorkerExit::Vanished+ };+ CancelContext::new()+ .with_timeout(timeout)+ .until_cancelled(ended)+ .await+ .unwrap_or(WorkerExit::StillRunning)+ }++ async fn shutdown(mut self) -> anyhow::Result<()> {+ self.worker.stop();+ self.worker.join().await?;+ self.mesh.shutdown().await;+ Ok(())+ }+}++/// OpenVMM's Linux direct test artifact `name`, or the file `var` names.+fn test_artifact(var: &str, name: &str) -> PathBuf {+ std::env::var_os(var).map_or_else(+ || {+ Path::new(env!("CARGO_MANIFEST_DIR"))+ .join("../../.packages/underhill-deps-private/x64")+ .join(name)+ },+ PathBuf::from,+ )+}++/// ReadMemory of a size that guest memory cannot hold fails the request, and+/// the VM worker keeps serving.+///+/// The sizes: one that no buffer can have (more than `isize::MAX` bytes), one+/// that this host cannot allocate, and one it can allocate but the 128-MiB+/// guest cannot hold.+#[test]+#[ignore = "needs /dev/kvm and the Linux direct test kernel and initrd"]+fn read_memory_of_an_impossible_size_fails_the_request() {+ DefaultPool::run_with(async |driver| {+ let dir = tempfile::tempdir().unwrap();+ let vm = TestVm::start_with_init(&driver, dir.path(), GUEST_IDLE_INIT)+ .await+ .unwrap();+ for size in [usize::MAX, 1 << 40, 1 << 30] {+ let read = vm.rpc.call_failable(VmRpc::ReadMemory, (0, size)).await;+ assert!(+ matches!(read, Err(mesh::rpc::RpcError::Call(_))),+ "ReadMemory of {size:#x} bytes at 0 returned {:?}",+ read.map(|bytes| bytes.len())+ );+ }+ let read = vm.rpc.call_failable(VmRpc::ReadMemory, (0, 16)).await;+ assert!(+ matches!(&read, Ok(bytes) if bytes.len() == 16),+ "after the failed reads, ReadMemory of 16 bytes returned {:?}",+ read.map(|bytes| bytes.len())+ );+ vm.shutdown().await.unwrap();+ })+}diff --git a/openvmm/openvmm_entry/Cargo.toml b/openvmm/openvmm_entry/Cargo.toml--- a/openvmm/openvmm_entry/Cargo.toml+++ b/openvmm/openvmm_entry/Cargo.toml@@ -132,6 +132,11 @@
build_rs_guest_arch.workspace = true
[dev-dependencies]
+initrd_cpio.workspace = true+# The in-process microVM tests link what the `openvmm` binary links: the+# resource resolvers and the VM worker, and the KVM backend.+openvmm_hypervisors = { workspace = true, features = ["virt_kvm"] }+openvmm_resources = { workspace = true, features = ["virt_kvm"] }
test_with_tracing.workspace = true
[lints]
diff --git a/openvmm/openvmm_entry/src/vm_controller/microvm.rs b/openvmm/openvmm_entry/src/vm_controller/microvm.rs--- a/openvmm/openvmm_entry/src/vm_controller/microvm.rs+++ b/openvmm/openvmm_entry/src/vm_controller/microvm.rs@@ -554,6 +554,9 @@
}
}
+#[cfg(all(test, target_os = "linux", guest_arch = "x86_64"))]+mod kvm_tests;+
#[cfg(test)]
mod tests {
use super::*;
diff --git a/Cargo.lock b/Cargo.lock--- a/Cargo.lock+++ b/Cargo.lock@@ -6279,6 +6279,7 @@
"guid",
"hyperv_ic_resources",
"ide_resources",
+ "initrd_cpio",
"input_core",
"inspect",
"inspect_proto",
@@ -6295,7 +6296,9 @@
"openvmm_build_info",
"openvmm_defs",
"openvmm_helpers",
+ "openvmm_hypervisors",
"openvmm_pcat_locator",
+ "openvmm_resources",
"openvmm_ttrpc_vmservice",
"otel_tracing",
"pal",
Activity
tangruize commentedon Nov 1, 2026
Proposed patch
Patch against OpenVMM
998b9f2402050cd16d1faa897bc831ef1434be48. Save the contents below asfix.patchand rungit apply fix.patch.fix.patch