From 1a777450e650dd33474be378b36fcd78994145fb Mon Sep 17 00:00:00 2001 From: Mathieu Tarral Date: Mon, 12 Oct 2026 10:09:59 +0000 Subject: [PATCH 1/2] virtualbox: test VBoxVMI disconnect cleanup --- doc/src/developer/tests.md | 5 +- doc/src/reference/drivers/virtualbox.md | 4 +- docs/adr/0002-vboxvmi-unix-socket.md | 8 +- tests/common/virtualbox.rs | 2 +- tests/integration.rs | 5 + tests/tests/virtualbox.rs | 146 ++++++++++++++++++++++++ 6 files changed, 163 insertions(+), 7 deletions(-) diff --git a/doc/src/developer/tests.md b/doc/src/developer/tests.md index a55b2e44..fe918b5a 100644 --- a/doc/src/developer/tests.md +++ b/doc/src/developer/tests.md @@ -75,13 +75,14 @@ Run the generic tests the method supports, or the VirtualBox ones: MICROVMI_TEST_FILTER=virtualbox_ cargo test --features virtualbox-api --test integration MICROVMI_TEST_FILTER=virtualbox_ VBOX_TEST_MEMORY_MB=5120 cargo test --features virtualbox-api --test integration MICROVMI_TEST_FILTER=vcpu VBOX_TEST_CPUS=2 cargo test --features virtualbox-api --test integration -for f in init vcpu_count pause double_resume virtualbox_read_ virtualbox_write_physical_ registers_match write_registers; do MICROVMI_TEST_FILTER=$f VBOX_TEST_METHOD=vboxvmi cargo test --features virtualbox-vmi --test integration; done +for f in init vcpu_count pause double_resume virtualbox_read_ virtualbox_write_physical_ registers_match write_registers vboxvmi_disconnect_; do MICROVMI_TEST_FILTER=$f VBOX_TEST_METHOD=vboxvmi cargo test --features virtualbox-vmi --test integration; done for f in vcpu pause registers_match write_registers; do MICROVMI_TEST_FILTER=$f VBOX_TEST_CPUS=2 VBOX_TEST_METHOD=vboxvmi cargo test --features virtualbox-vmi --test integration; done ~~~ The VBoxVMI filters above run every test, each once; `virtualbox_write_physical_` checks random patterns against a fresh core dump, after resume/re-pause and after restoring the original pages. -The other filters cover `pause`, +`vboxvmi_disconnect_` covers normal close, a killed client process, both external-pause ownership +cases, and immediate `Busy` refusal of a second client. The other filters cover `pause`, `double_pause`, `pause_resume`, `multiple_pause_resume`, `virtualbox_pause_*` and `virtualbox_registers_unchanged_while_paused`; `vcpu_count` covers `get_vcpu_count` and `virtualbox_vcpu_count_matches_vmm`. With `VBOX_TEST_CPUS=2`, the register tests read every vCPU diff --git a/doc/src/reference/drivers/virtualbox.md b/doc/src/reference/drivers/virtualbox.md index 487c90f3..02b917bf 100644 --- a/doc/src/reference/drivers/virtualbox.md +++ b/doc/src/reference/drivers/virtualbox.md @@ -24,7 +24,7 @@ A physical write to a non-RAM page fails; other pages in the same batch may alre Pause is VirtualBox's own pause (the VM shows as Paused in the GUI and `VBoxManage`). It is synchronous, all vCPUs are stopped when it returns, and idempotent: pausing a paused VM succeeds, and one resume resumes it. Pauses are not counted, also across clients: resume resumes the VM even if the GUI, `VBoxManage` or another tool paused it too. Pause and resume fail while VirtualBox controls the VM's run state: powering on or off, saving or restoring its state, live snapshot, teleport, or a configuration change (medium or network attachment). Resume also fails while the host's power management has the VM paused, and after a failed teleport of a paused VM until it is resumed through VirtualBox. -A VM paused through VBoxVMI stays paused if the tool exits or disconnects without resuming it ([#287](https://github.com/Wenzel/libmicrovmi/issues/287)). +A client disconnect leaves no session state behind: the server removes its intercepts and synchronously resumes a pause the client created. A pause that already existed, or an external pause after the client's pause was externally resumed, remains paused. A register read or write is one request, served by the vCPU itself between two of its instructions: a read is a consistent snapshot of that vCPU and a write changes all its registers at once, also while the VM runs (pause first to read or write several vCPUs at the same point). The MSRs of `X86Registers` (EFER, STAR, LSTAR, SYSENTER_CS/ESP/EIP, APIC base) are included; an MSR the guest CPU doesn't have reads as 0. A write is all or nothing: a value the CPU itself would not load (RFLAGS with bit 1 clear or a reserved bit set, reserved bits in CR0, CR3, CR4 or EFER, an inconsistent long mode state, an MSR value `WRMSR` refuses) fails the whole write, and nothing is written. Registers keeping their value are not touched, so writing back registers read earlier always succeeds and has no side effects. @@ -38,7 +38,7 @@ Segment selectors, bases and limits are written as raw state: no descriptor tabl The socket is `/vmi/.sock` (e.g. `~/.config/VirtualBox/vmi/`), or the path set in `VBoxInternal/VMI/SocketPath`. - libmicrovmi runs as the user running the VM (the socket is mode 0600 and the server checks the peer's uid; root is refused too). - A custom `SocketPath` must be absolute, in a directory owned by that user and not writable by group or others (unless sticky); otherwise the VM refuses to start. -- One client per VM at a time: a second client gets a "another VBoxVMI client is already connected" error. +- One client per VM at a time: a second client immediately gets the error "another VBoxVMI client is already connected". ### Initialization parameters diff --git a/docs/adr/0002-vboxvmi-unix-socket.md b/docs/adr/0002-vboxvmi-unix-socket.md index 5358082b..363ad2aa 100644 --- a/docs/adr/0002-vboxvmi-unix-socket.md +++ b/docs/adr/0002-vboxvmi-unix-socket.md @@ -16,11 +16,15 @@ matched by id. Bulk memory reads are batched to amortise the per-message cost. ## Consequences -- The kernel tells the server when the client goes away (EOF), so it can resume a VM the client - paused and remove its intercepts. With shared memory this needs heartbeats or a side channel. +- The kernel tells the server when the client goes away (EOF), so one session cleanup point removes + its intercepts and resumes only a pause that session created and nobody has since resumed. A + pre-existing pause, and a new external pause after the client's pause was resumed, remain paused. + With shared memory this needs heartbeats or a side channel. - Access control is the socket file's mode (0600) plus a peer uid check (`SO_PEERCRED`), with no protocol-level authentication. - One ordered channel carries replies and, in phase 2, events (request id 0). +- While one client is active, the server accepts a second connection only to greet it with + `VERR_RESOURCE_BUSY` and close it, rather than leaving it waiting in the listen backlog. - `SOCK_SEQPACKET` keeps message boundaries, so neither side frames a byte stream. - Each request costs a syscall round trip and a copy. Page-by-page reads of whole guest RAM are expected to stay practical; batching covers bulk reads. diff --git a/tests/common/virtualbox.rs b/tests/common/virtualbox.rs index 0b34fa31..27f332d6 100644 --- a/tests/common/virtualbox.rs +++ b/tests/common/virtualbox.rs @@ -74,7 +74,7 @@ fn default_method() -> VBoxInitParams { } /// The configured or default VBoxVMI socket path. -fn vboxvmi_socket() -> String { +pub fn vboxvmi_socket() -> String { if let Some(path) = extradata(VMI_SOCKET) { return path; } diff --git a/tests/integration.rs b/tests/integration.rs index de4af791..7af5bb46 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -14,6 +14,11 @@ mod common; mod tests; fn main() { + #[cfg(all(feature = "virtualbox-vmi", not(feature = "kvm")))] + if std::env::var_os("MICROVMI_VBOXVMI_PAUSE_CHILD").is_some() { + tests::virtualbox::vboxvmi_pause_child(); + } + // init logger env_logger::builder().is_test(true).init(); // run only the tests whose name contains MICROVMI_TEST_FILTER diff --git a/tests/tests/virtualbox.rs b/tests/tests/virtualbox.rs index cf35f69f..2600d729 100644 --- a/tests/tests/virtualbox.rs +++ b/tests/tests/virtualbox.rs @@ -2,8 +2,12 @@ use std::convert::TryInto; use std::env; use std::fs::{self, File}; +#[cfg(feature = "virtualbox-vmi")] +use std::io::{BufRead, BufReader, Write}; use std::ops::Range; use std::os::unix::fs::FileExt; +#[cfg(feature = "virtualbox-vmi")] +use std::process::{Command, Stdio}; use std::thread::sleep; use std::time::{Duration, Instant, SystemTime}; @@ -12,6 +16,8 @@ use microvmi::api::registers::{Registers, X86Registers}; use microvmi::api::{Introspectable, PAGE_SIZE}; use super::IntegrationTest; +#[cfg(feature = "virtualbox-vmi")] +use crate::common::virtualbox::vboxvmi_socket; use crate::common::virtualbox::{core_path, try_vboxmanage, vboxmanage, vm_info, vm_name}; const PAGE: u64 = PAGE_SIZE as u64; @@ -318,6 +324,146 @@ inventory::submit!(IntegrationTest { test_fn: pause_idempotent_and_visible }); +#[cfg(feature = "virtualbox-vmi")] +fn connect_vboxvmi() -> vboxvmi::Client { + let deadline = Instant::now() + Duration::from_secs(10); + loop { + match vboxvmi::Client::connect(vboxvmi_socket()) { + Ok(client) => return client, + Err(error) if Instant::now() < deadline => { + debug!("waiting for the prior VBoxVMI session to close: {}", error); + sleep(Duration::from_millis(20)); + } + Err(error) => panic!("failed to connect to VBoxVMI: {}", error), + } + } +} + +#[cfg(feature = "virtualbox-vmi")] +fn wait_for_vm_state(expected: &str) { + let deadline = Instant::now() + Duration::from_secs(10); + loop { + let state = vm_info("VMState"); + if state == expected { + return; + } + assert!( + Instant::now() < deadline, + "VM did not become {} after VBoxVMI disconnect; state is {}", + expected, + state + ); + sleep(Duration::from_millis(20)); + } +} + +/// Entry point for the subprocess killed by `vboxvmi_killed_client_resumes`. +#[cfg(feature = "virtualbox-vmi")] +pub fn vboxvmi_pause_child() -> ! { + let mut client = connect_vboxvmi(); + client + .pause() + .expect("child failed to pause through VBoxVMI"); + println!("paused"); + std::io::stdout() + .flush() + .expect("child failed to flush readiness"); + loop { + sleep(Duration::from_secs(60)); + } +} + +#[cfg(feature = "virtualbox-vmi")] +fn vboxvmi_normal_close_resumes(mut drv: Box) { + drv.pause().unwrap(); + assert_eq!(vm_info("VMState"), "paused"); + drop(drv); + wait_for_vm_state("running"); +} + +#[cfg(feature = "virtualbox-vmi")] +inventory::submit!(IntegrationTest { + name: "vboxvmi_disconnect_normal_close_resumes", + test_fn: vboxvmi_normal_close_resumes +}); + +#[cfg(feature = "virtualbox-vmi")] +fn vboxvmi_killed_client_resumes(drv: Box) { + drop(drv); + let mut child = Command::new(std::env::current_exe().unwrap()) + .env("MICROVMI_VBOXVMI_PAUSE_CHILD", "1") + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .expect("failed to start VBoxVMI pause child"); + let mut ready = String::new(); + BufReader::new(child.stdout.take().unwrap()) + .read_line(&mut ready) + .expect("failed to read child readiness"); + if ready.trim_end() != "paused" { + let _ = child.kill(); + let _ = child.wait(); + panic!("VBoxVMI pause child did not become ready: {:?}", ready); + } + assert_eq!(vm_info("VMState"), "paused"); + child.kill().expect("failed to kill VBoxVMI pause child"); + child.wait().expect("failed to reap VBoxVMI pause child"); + wait_for_vm_state("running"); +} + +#[cfg(feature = "virtualbox-vmi")] +inventory::submit!(IntegrationTest { + name: "vboxvmi_disconnect_killed_client_resumes", + test_fn: vboxvmi_killed_client_resumes +}); + +#[cfg(feature = "virtualbox-vmi")] +fn vboxvmi_external_pause_is_not_resumed(mut drv: Box) { + vboxmanage(&["controlvm", &vm_name(), "pause"]); + drv.pause().unwrap(); + drop(drv); + drop(connect_vboxvmi()); // accepting the next client proves disconnect cleanup finished + assert_eq!(vm_info("VMState"), "paused"); +} + +#[cfg(feature = "virtualbox-vmi")] +inventory::submit!(IntegrationTest { + name: "vboxvmi_disconnect_preserves_preexisting_vm_state", + test_fn: vboxvmi_external_pause_is_not_resumed +}); + +#[cfg(feature = "virtualbox-vmi")] +fn vboxvmi_resume_generation_preserves_external_pause(mut drv: Box) { + drv.pause().unwrap(); + vboxmanage(&["controlvm", &vm_name(), "resume"]); + vboxmanage(&["controlvm", &vm_name(), "pause"]); + drop(drv); + drop(connect_vboxvmi()); // accepting the next client proves disconnect cleanup finished + assert_eq!(vm_info("VMState"), "paused"); +} + +#[cfg(feature = "virtualbox-vmi")] +inventory::submit!(IntegrationTest { + name: "vboxvmi_disconnect_preserves_new_external_vm_state", + test_fn: vboxvmi_resume_generation_preserves_external_pause +}); + +#[cfg(feature = "virtualbox-vmi")] +fn vboxvmi_second_client_is_busy(drv: Box) { + match vboxvmi::Client::connect(vboxvmi_socket()) { + Err(vboxvmi::Error::Busy) => {} + Err(error) => panic!("second VBoxVMI client got the wrong error: {}", error), + Ok(_) => panic!("second VBoxVMI client connected"), + } + drop(drv); +} + +#[cfg(feature = "virtualbox-vmi")] +inventory::submit!(IntegrationTest { + name: "vboxvmi_disconnect_second_client_busy", + test_fn: vboxvmi_second_client_is_busy +}); + /// Resumes the VM even if selecting pages or dumping the first core fails. struct ResumeVm; From cdd942475901a8f82a15179469291ef70cc2bb9c Mon Sep 17 00:00:00 2001 From: Mathieu Tarral Date: Mon, 12 Oct 2026 10:32:19 +0000 Subject: [PATCH 2/2] test: race VBoxVMI disconnect with external pause --- doc/src/developer/tests.md | 5 +- tests/tests/virtualbox.rs | 102 +++++++++++++++++++++++++++++++++---- 2 files changed, 94 insertions(+), 13 deletions(-) diff --git a/doc/src/developer/tests.md b/doc/src/developer/tests.md index fe918b5a..8f578030 100644 --- a/doc/src/developer/tests.md +++ b/doc/src/developer/tests.md @@ -75,14 +75,15 @@ Run the generic tests the method supports, or the VirtualBox ones: MICROVMI_TEST_FILTER=virtualbox_ cargo test --features virtualbox-api --test integration MICROVMI_TEST_FILTER=virtualbox_ VBOX_TEST_MEMORY_MB=5120 cargo test --features virtualbox-api --test integration MICROVMI_TEST_FILTER=vcpu VBOX_TEST_CPUS=2 cargo test --features virtualbox-api --test integration -for f in init vcpu_count pause double_resume virtualbox_read_ virtualbox_write_physical_ registers_match write_registers vboxvmi_disconnect_; do MICROVMI_TEST_FILTER=$f VBOX_TEST_METHOD=vboxvmi cargo test --features virtualbox-vmi --test integration; done +for f in init vcpu_count pause double_resume virtualbox_read_ virtualbox_write_physical_ registers_match write_registers vboxvmi_disconnect_; do MICROVMI_TEST_FILTER=$f MICROVMI_TEST_TIMEOUT=120 VBOX_TEST_METHOD=vboxvmi cargo test --features virtualbox-vmi --test integration; done for f in vcpu pause registers_match write_registers; do MICROVMI_TEST_FILTER=$f VBOX_TEST_CPUS=2 VBOX_TEST_METHOD=vboxvmi cargo test --features virtualbox-vmi --test integration; done ~~~ The VBoxVMI filters above run every test, each once; `virtualbox_write_physical_` checks random patterns against a fresh core dump, after resume/re-pause and after restoring the original pages. `vboxvmi_disconnect_` covers normal close, a killed client process, both external-pause ownership -cases, and immediate `Busy` refusal of a second client. The other filters cover `pause`, +cases, a repeated external resume/pause versus disconnect race, and immediate `Busy` refusal of a +second client. The other filters cover `pause`, `double_pause`, `pause_resume`, `multiple_pause_resume`, `virtualbox_pause_*` and `virtualbox_registers_unchanged_while_paused`; `vcpu_count` covers `get_vcpu_count` and `virtualbox_vcpu_count_matches_vmm`. With `VBOX_TEST_CPUS=2`, the register tests read every vCPU diff --git a/tests/tests/virtualbox.rs b/tests/tests/virtualbox.rs index 2600d729..0dcc5d05 100644 --- a/tests/tests/virtualbox.rs +++ b/tests/tests/virtualbox.rs @@ -3,11 +3,15 @@ use std::convert::TryInto; use std::env; use std::fs::{self, File}; #[cfg(feature = "virtualbox-vmi")] -use std::io::{BufRead, BufReader, Write}; +use std::io::{self, BufRead, BufReader, Write}; use std::ops::Range; use std::os::unix::fs::FileExt; #[cfg(feature = "virtualbox-vmi")] -use std::process::{Command, Stdio}; +use std::process::{Child, Command, Stdio}; +#[cfg(feature = "virtualbox-vmi")] +use std::sync::mpsc; +#[cfg(feature = "virtualbox-vmi")] +use std::thread; use std::thread::sleep; use std::time::{Duration, Instant, SystemTime}; @@ -387,27 +391,63 @@ inventory::submit!(IntegrationTest { test_fn: vboxvmi_normal_close_resumes }); +#[cfg(feature = "virtualbox-vmi")] +struct ChildGuard(Option); + +#[cfg(feature = "virtualbox-vmi")] +impl ChildGuard { + fn new(child: Child) -> Self { + Self(Some(child)) + } + + fn child_mut(&mut self) -> &mut Child { + self.0.as_mut().unwrap() + } + + fn kill_and_wait(&mut self) -> io::Result<()> { + { + let child = self.0.as_mut().unwrap(); + child.kill()?; + child.wait()?; + } + self.0 = None; + Ok(()) + } +} + +#[cfg(feature = "virtualbox-vmi")] +impl Drop for ChildGuard { + fn drop(&mut self) { + if let Some(mut child) = self.0.take() { + let _ = child.kill(); + let _ = child.wait(); + } + } +} + #[cfg(feature = "virtualbox-vmi")] fn vboxvmi_killed_client_resumes(drv: Box) { drop(drv); - let mut child = Command::new(std::env::current_exe().unwrap()) + let child = Command::new(std::env::current_exe().unwrap()) .env("MICROVMI_VBOXVMI_PAUSE_CHILD", "1") .stdout(Stdio::piped()) .stderr(Stdio::inherit()) .spawn() .expect("failed to start VBoxVMI pause child"); + let mut child = ChildGuard::new(child); let mut ready = String::new(); - BufReader::new(child.stdout.take().unwrap()) + BufReader::new(child.child_mut().stdout.take().unwrap()) .read_line(&mut ready) .expect("failed to read child readiness"); - if ready.trim_end() != "paused" { - let _ = child.kill(); - let _ = child.wait(); - panic!("VBoxVMI pause child did not become ready: {:?}", ready); - } + assert_eq!( + ready.trim_end(), + "paused", + "VBoxVMI pause child did not become ready" + ); assert_eq!(vm_info("VMState"), "paused"); - child.kill().expect("failed to kill VBoxVMI pause child"); - child.wait().expect("failed to reap VBoxVMI pause child"); + child + .kill_and_wait() + .expect("failed to kill and reap VBoxVMI pause child"); wait_for_vm_state("running"); } @@ -448,6 +488,46 @@ inventory::submit!(IntegrationTest { test_fn: vboxvmi_resume_generation_preserves_external_pause }); +#[cfg(feature = "virtualbox-vmi")] +fn vboxvmi_disconnect_race_preserves_external_state(drv: Box) { + drop(drv); + for iteration in 0..8 { + if iteration != 0 { + vboxmanage(&["controlvm", &vm_name(), "resume"]); + } + let mut client = connect_vboxvmi(); + client.pause().unwrap(); + + let (resumed_tx, resumed_rx) = mpsc::channel(); + let external = thread::spawn(move || { + let _ = try_vboxmanage(&["controlvm", &vm_name(), "resume"]); + resumed_tx.send(()).unwrap(); + vboxmanage(&["controlvm", &vm_name(), "pause"]); + }); + if iteration % 2 != 0 { + resumed_rx.recv().unwrap(); + } else { + thread::yield_now(); + } + drop(client); + external.join().expect("external resume/pause panicked"); + + drop(connect_vboxvmi()); // accepting the next client proves disconnect cleanup finished + assert_eq!( + vm_info("VMState"), + "paused", + "disconnect resumed external pause in race iteration {}", + iteration + ); + } +} + +#[cfg(feature = "virtualbox-vmi")] +inventory::submit!(IntegrationTest { + name: "vboxvmi_disconnect_race_preserves_external_state", + test_fn: vboxvmi_disconnect_race_preserves_external_state +}); + #[cfg(feature = "virtualbox-vmi")] fn vboxvmi_second_client_is_busy(drv: Box) { match vboxvmi::Client::connect(vboxvmi_socket()) {