From fb0589001b2d6c696aac12ca3c48d192645f0973 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 20 Oct 2026 12:50:34 -0700 Subject: [PATCH 01/10] nvx-rs: add the VMM supervisor with a spawn allowlist, stop, and reattach Add `nvx::host::supervisor`, beneath the CLI, `host::instance`, and external orchestrators. It launches OpenVMM through the launch journal of the sandbox store, passes it only the descriptors or handles that the caller allows, waits for its exit by event, stops it through the caller's graceful step with a bounded forced step on request, and reattaches to it after the orchestrator restarts within the same host boot. On Linux, only the VMM's own process clears the close-on-exec flag of the allowed descriptors, so a spawn on another thread inherits none of them. On Windows, the allowed handles join the spawn's explicit handle list, and are inheritable only while the VMM is created. The spawn helpers of `host::process` now go through the supervisor with an empty allowlist, until the CLI and `host::instance` move to it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03 --- nvx-rs/Cargo.toml | 2 +- nvx-rs/README.md | 10 +- nvx-rs/src/host/mod.rs | 12 +- nvx-rs/src/host/platform/linux.rs | 34 +- nvx-rs/src/host/platform/other.rs | 11 +- nvx-rs/src/host/platform/windows.rs | 201 ++++- nvx-rs/src/host/process.rs | 304 +------- nvx-rs/src/host/store/mod.rs | 24 +- nvx-rs/src/host/supervisor/child.rs | 536 +++++++++++++ nvx-rs/src/host/supervisor/mod.rs | 608 +++++++++++++++ nvx-rs/src/host/supervisor/tests.rs | 456 +++++++++++ nvx-rs/src/host/sys/process.rs | 28 +- nvx-rs/tests/managed_lifecycle.rs | 2 +- nvx-rs/tests/supervisor.rs | 1113 +++++++++++++++++++++++++-- 14 files changed, 2956 insertions(+), 385 deletions(-) create mode 100644 nvx-rs/src/host/supervisor/child.rs create mode 100644 nvx-rs/src/host/supervisor/mod.rs create mode 100644 nvx-rs/src/host/supervisor/tests.rs diff --git a/nvx-rs/Cargo.toml b/nvx-rs/Cargo.toml index 1b8c62255..6c7deff7b 100644 --- a/nvx-rs/Cargo.toml +++ b/nvx-rs/Cargo.toml @@ -99,7 +99,7 @@ sha2 = { version = "0.10", optional = true } serde_json = "1.0.120" tempfile = "3.10" -# The job objects and process security descriptors of the process supervision tests. +# The job objects, process security descriptors, and handles of the process supervision tests. [target.'cfg(windows)'.dev-dependencies] windows-sys = { version = "0.61", features = [ "Win32_Foundation", diff --git a/nvx-rs/README.md b/nvx-rs/README.md index fd089d2d4..6d83c2a36 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -61,7 +61,8 @@ nvx-rs/ | `nvx::host::store` | `SandboxStore`: the runtime records of one sandbox in a private directory with one lock, which every host that supervises the sandbox shares. The `RuntimeRecord` names the running VM: the `ProcessRef` of its VMM, its control endpoint, and the `RuntimeIdentity` that it was launched with, the identities of its OpenVMM, kernel, and initramfs from install or staging metadata and its boot and control contract versions, which decide whether another build of the host may reattach to it. The `LaunchJournal` of a launch holds a marker written before the spawn, the VMM's `ProcessRef` right after it, and an inherited claim on the VMM's log, which proves that a launch that never published its endpoint has exited, so that recovery ends an interrupted launch and signals only a VMM that it holds through a verified `Process`. Records are JSON in one versioned format, and records in any other format fail rather than being migrated | | `nvx::host::instance` | `Instance`: a VM in a directory of its own. It starts OpenVMM with a fresh capability and waits until the guest is ready and has mounted the mapped paths, reconciles recorded state, recovers interrupted launches, runs workloads and pumps their output, and stops the VM with a forced fallback | | `nvx::host::nvxs` | `HostConnection`: the host's role of the NVXS session over a platform transport, on which `nvx::host::session` and other host applications run their protocols. It attaches with the launch capability, waiting while the broker asks it to, sends application bytes in as many DATA records as they take, and returns each DATA payload from the guest, with deadlines; a receive that times out keeps a partial record | -| `nvx::host::process` | The detached launch of OpenVMM, and verified process references. A `ProcessRef` names one process by its ID, its start time, and the `BootId` of the host boot during which it started. `Process::open` checks a reference and holds its process through a pidfd on Linux or a process handle on Windows, whether or not the caller started it, so that waits sleep until the process exits and termination never reaches a process that reuses its ID. A reference from an earlier boot names an exited process, and a process whose identity this user cannot check, such as another user's, makes `Process::open` fail rather than report an exit. Linux hosts without pidfds poll the reference instead, and cannot terminate the process | +| `nvx::host::process` | Verified process references, and the launch of OpenVMM for the CLI and `nvx::host::instance`, through `nvx::host::supervisor`. A `ProcessRef` names one process by its ID, its start time, and the `BootId` of the host boot during which it started. `Process::open` checks a reference and holds its process through a pidfd on Linux or a process handle on Windows, whether or not the caller started it, so that waits sleep until the process exits and termination never reaches a process that reuses its ID. A reference from an earlier boot names an exited process, and a process whose identity this user cannot check, such as another user's, makes `Process::open` fail rather than report an exit. Linux hosts without pidfds poll the reference instead, and cannot terminate the process | +| `nvx::host::supervisor` | The supervisor of a sandbox's VMM, beneath the CLI, `Instance`, and external orchestrators, which keep the configuration, the authorization, and the application protocol. `launch` runs the store's launch steps under the sandbox's lock: it writes the journal's marker, spawns OpenVMM with the claimed log, records its `ProcessRef` right after, and returns a `Starting` launch that the caller commits once the VMM serves its control endpoint, or abandons. A launch that fails never leaves an unrecorded VMM running. `VmmCommand::inherit` lists the descriptors (Linux) or handles (Windows) that the VMM inherits under their numbers, besides its standard streams, and it inherits nothing else of the caller's: on Linux, only the VMM's own process clears their close-on-exec flag, and on Windows, they join the VMM's explicit handle list. `Vmm::stop` runs one graceful step of the caller's and waits for the VMM's exit event until a deadline, and takes a bounded forced step through the VMM's verified identity only on request. `reattach` recovers an interrupted launch, and holds the recorded VMM again after the orchestrator restarts within the same host boot: `Live`, `Mismatched` with another runtime identity, `Exited`, or `Indeterminate` | | `nvx::host::session`, `platform`, `report` | The authenticated control session, which carries NVX's control protocol, `NVXC`, one frame per DATA record of a `HostConnection`; Linux and Windows primitives; and OpenVMM's outcome report | | `nvx::host::sandbox` | The launch profile of `nvx.py sandbox`, ported from the one that `scripts/nvx_tools/sandbox.py` held: `SandboxLaunch`, with the EROFS distro `SandboxLayer`, the one layer that a sandbox takes, the ext4 scratch disk, the entrypoint, arguments, hostname, workload identity, and resource limits, and its `--microvm-sandbox-block` and `--microvm-workload-identity` arguments and kernel command line, within the 1024-byte budget that OpenVMM leaves it. `mount`: `SandboxMount` live shares of host directories and files, their guest targets and the reserved ones, their access policies and owner, the aggregate of several shares, and the checks of their host paths. `options`: the checks that `nvx.py sandbox` and `nvx.py run` apply to their options: the attribution of `--mount-deny`, `--mount-allow`, and `--mount-write` to shares, the systemd policy, the targets of `nvx.py run` shares, and the managed exec environment; see [Python parity](#python-parity) | | `nvx::testing::fake_openvmm` | A fake OpenVMM that serves the control console and runs scripted workloads without a VM, for direct launches and for the managed and one-shot sandboxes of `nvx-rs sandbox` and `nvx.py sandbox` | @@ -343,7 +344,7 @@ in sandbox mode. code page unless `PYTHONIOENCODING` or UTF-8 mode says otherwise. - `start` refuses a control endpoint that another process serves, which it treats as one that has not appeared yet, and, on Windows, starts OpenVMM - without a console window, as `nvx::host::process` starts it. + without a console window, as `nvx::host::supervisor` starts it. - `exec` refuses more than 1 MiB of workload output, which the guest agent never forwards. @@ -538,6 +539,11 @@ broker that relays records as OpenVMM's does, with messages of an application framing of its own that span records. The store test crashes orchestrator processes at each step of a launch, recovers the VMM stand-ins that they leave behind, and loads records that another build wrote, in `tests/fixtures/store/`. +The process-supervision test crashes orchestrator processes at each step of a +launch through the supervisor, reattaches to the VMM stand-ins that they leave +behind and stops them, and checks that a VMM inherits only its standard streams +and the descriptors or handles that it is allowed while another thread spawns +processes. On Linux, a loopback test drives the host `ControlSession` through a broker shim into the real managed agent's session, dispatch, and execution engine, and `nvx.py` also lints and tests the diff --git a/nvx-rs/src/host/mod.rs b/nvx-rs/src/host/mod.rs index aecd50489..d9dfe5872 100644 --- a/nvx-rs/src/host/mod.rs +++ b/nvx-rs/src/host/mod.rs @@ -14,14 +14,17 @@ //! - [`store`] keeps the runtime records of a sandbox in a private directory with one lock: the //! runtime record of its VM, with the VM's runtime identity, and the journal of its launch, //! through which an interrupted launch is recovered without signaling an unverified process. +//! - [`supervisor`] launches a sandbox's VMM through the store's journal, passing it only the +//! descriptors or handles that the caller allows, stops it through the caller's graceful step, +//! with a bounded forced step on request, and reattaches to it after the orchestrator restarts +//! within the same host boot. //! - [`session`] speaks NVX's control protocol, `NVXC`, over [`nvxs`], which runs the host's role //! of the NVXS session for it and for any other application protocol, over a platform //! transport. [`platform`] holds the primitives of Linux and Windows hosts. Other hosts compile //! but report NVX as unavailable. -//! - [`process`] launches OpenVMM processes. Its verified process references name a process by -//! its ID, start time, and host boot, and hold it through a pidfd or a process handle, so that -//! waits sleep until it exits and termination reaches only it, whether or not the caller -//! started it. +//! - [`process`] names processes by verified references: their ID, start time, and host boot, +//! through which a pidfd or a process handle holds them, so that waits sleep until a process +//! exits and termination reaches only it, whether or not the caller started it. //! - [`sandbox`] is the launch profile of `nvx.py sandbox`: a container over read-only EROFS //! layers and a private ext4 scratch disk, with live shares, and the checks of its options. //! - `cli` (feature `cli`) is the `nvx-rs` command-line interface, which `nvx.py` runs for the @@ -51,6 +54,7 @@ pub mod report; pub mod sandbox; pub mod session; pub mod store; +pub mod supervisor; #[cfg(unix)] #[allow(unsafe_code)] mod sys; diff --git a/nvx-rs/src/host/platform/linux.rs b/nvx-rs/src/host/platform/linux.rs index a8c452c90..1ca3fff0d 100644 --- a/nvx-rs/src/host/platform/linux.rs +++ b/nvx-rs/src/host/platform/linux.rs @@ -1,6 +1,6 @@ use std::fs::{self, DirBuilder, File, OpenOptions, Permissions, TryLockError}; use std::io::{self, Read}; -use std::os::fd::AsRawFd; +use std::os::fd::{AsFd, AsRawFd, RawFd}; use std::os::unix::fs::{DirBuilderExt, MetadataExt, OpenOptionsExt, PermissionsExt}; use std::os::unix::net::UnixStream; use std::path::Path; @@ -9,6 +9,7 @@ use std::thread; use std::time::{Duration, Instant}; use super::Transport; +use crate::host::supervisor::Inherited; use crate::host::sys::{self, Errno, PidFd}; use crate::hypervisor::Hypervisor; @@ -240,10 +241,33 @@ pub(crate) fn boot_id() -> io::Result<[u8; 16]> { }) } -/// Starts the command in a new session so it outlives the caller and its terminal, and with no -/// descriptor of the caller beyond the standard streams that the command sets up. -pub(crate) fn detach(command: &mut Command, _breakaway_from_job: bool) { - sys::detach(command); +/// Starts the command in a new session so it outlives the caller and its terminal, with the +/// descriptors `inherit` under their numbers in the caller, and no other descriptor of the caller +/// beyond the standard streams that the command sets up. +/// +/// Only the new process clears the close-on-exec flag of the descriptors in `inherit`, so no +/// process that another thread spawns meanwhile inherits them. Fails with +/// [`io::ErrorKind::InvalidInput`] for a descriptor of a standard stream, 0 to 2, which the +/// command's own standard streams replace in the new process. +pub(crate) fn detach(command: &mut Command, inherit: &[Inherited<'_>]) -> io::Result<()> { + let mut descriptors: Vec = Vec::with_capacity(inherit.len()); + for inherited in inherit { + let descriptor = inherited.as_fd().as_raw_fd(); + if descriptor <= 2 { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "descriptor {descriptor} cannot be inherited: the VMM's own standard streams \ + replace descriptors 0 to 2" + ), + )); + } + if !descriptors.contains(&descriptor) { + descriptors.push(descriptor); + } + } + sys::detach(command, descriptors); + Ok(()) } /// Checks that the hypervisor device is present and accessible. diff --git a/nvx-rs/src/host/platform/other.rs b/nvx-rs/src/host/platform/other.rs index 582da1424..89fdf6a03 100644 --- a/nvx-rs/src/host/platform/other.rs +++ b/nvx-rs/src/host/platform/other.rs @@ -5,6 +5,7 @@ use std::process::Command; use std::time::{Duration, Instant}; use super::Transport; +use crate::host::supervisor::Inherited; use crate::hypervisor::Hypervisor; #[cfg(all(feature = "cli", unix))] @@ -105,7 +106,15 @@ pub(crate) fn private_file_problem(_file: &File) -> io::Result> { Err(unsupported()) } -pub(crate) fn detach(_command: &mut Command, _breakaway_from_job: bool) {} +/// Leaves the command as it is, unless it should inherit descriptors: this host cannot restrict +/// what the command inherits, and fails with [`io::ErrorKind::Unsupported`] instead. +pub(crate) fn detach(_command: &mut Command, inherit: &[Inherited<'_>]) -> io::Result<()> { + if inherit.is_empty() { + Ok(()) + } else { + Err(unsupported()) + } +} pub(crate) fn probe_hypervisor(_hypervisor: Hypervisor) -> Result<(), String> { Err(unsupported().to_string()) diff --git a/nvx-rs/src/host/platform/windows.rs b/nvx-rs/src/host/platform/windows.rs index f03ea50f3..71c02f277 100644 --- a/nvx-rs/src/host/platform/windows.rs +++ b/nvx-rs/src/host/platform/windows.rs @@ -3,7 +3,7 @@ use std::fs::{self, OpenOptions}; use std::io; use std::os::windows::ffi::OsStrExt; use std::os::windows::fs::OpenOptionsExt; -use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle, RawHandle}; +use std::os::windows::io::{AsHandle, AsRawHandle, FromRawHandle, OwnedHandle, RawHandle}; use std::path::Path; use std::time::{Duration, Instant}; use std::{mem, ptr}; @@ -12,8 +12,9 @@ use windows_sys::Win32::Foundation::{ ERROR_ACCESS_DENIED, ERROR_ALREADY_EXISTS, ERROR_BROKEN_PIPE, ERROR_FILE_NOT_FOUND, ERROR_INVALID_PARAMETER, ERROR_IO_PENDING, ERROR_MORE_DATA, ERROR_NO_DATA, ERROR_OPERATION_ABORTED, ERROR_PIPE_BUSY, ERROR_PIPE_NOT_CONNECTED, ERROR_SHARING_VIOLATION, - ERROR_SUCCESS, FILETIME, FreeLibrary, GENERIC_READ, GENERIC_WRITE, GetLastError, HANDLE, - HANDLE_FLAG_INHERIT, INVALID_HANDLE_VALUE, SetHandleInformation, WAIT_OBJECT_0, WAIT_TIMEOUT, + ERROR_SUCCESS, FILETIME, FreeLibrary, GENERIC_READ, GENERIC_WRITE, GetHandleInformation, + GetLastError, HANDLE, HANDLE_FLAG_INHERIT, INVALID_HANDLE_VALUE, SetHandleInformation, + WAIT_OBJECT_0, WAIT_TIMEOUT, }; use windows_sys::Win32::Security::{ ACCESS_ALLOWED_ACE, ACE_HEADER, ACL, ACL_REVISION, ACL_SIZE_INFORMATION, AclSizeInformation, @@ -54,6 +55,7 @@ use windows_sys::Win32::System::Threading::{ }; use super::Transport; +use crate::host::supervisor::Inherited; use crate::hypervisor::Hypervisor; /// Whether this host can run NVX microVMs. @@ -386,33 +388,92 @@ impl Drop for AttributeList { } } +/// Handles of a caller that a spawn makes inheritable for as long as it needs them, and whose +/// earlier inheritability it restores when dropped. +struct InheritableForSpawn { + /// Each handle, and whether it was inheritable before. + handles: Vec<(HANDLE, bool)>, +} + +impl InheritableForSpawn { + /// Makes each of `handles`, which the caller keeps open until this is dropped, inheritable. + /// + /// Fails if a handle cannot be, and then restores the handles that it changed. + fn new(handles: &[HANDLE]) -> io::Result { + let mut inheritable = Self { + handles: Vec::with_capacity(handles.len()), + }; + for &handle in handles { + let mut flags = 0; + // SAFETY: the handle is open, and the output pointer is valid. + if unsafe { GetHandleInformation(handle, &mut flags) } == 0 { + return Err(io::Error::last_os_error()); + } + // SAFETY: the handle is open. + if unsafe { SetHandleInformation(handle, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT) } + == 0 + { + return Err(io::Error::last_os_error()); + } + inheritable + .handles + .push((handle, flags & HANDLE_FLAG_INHERIT != 0)); + } + Ok(inheritable) + } +} + +impl Drop for InheritableForSpawn { + fn drop(&mut self) { + for &(handle, inheritable) in &self.handles { + let flags = if inheritable { HANDLE_FLAG_INHERIT } else { 0 }; + // SAFETY: the caller keeps the handle open until this is dropped. A handle whose flag + // cannot be restored stays inheritable, which only widens what later spawns without a + // handle list may inherit. + unsafe { SetHandleInformation(handle, HANDLE_FLAG_INHERIT, flags) }; + } + } +} + /// Starts OpenVMM without the caller's console so it outlives the caller. /// /// `std::process::Command` lets the child inherit every inheritable handle of the caller. A /// caller whose own standard output is a pipe, such as a phase process whose parent collects /// its output, would then hand that pipe to OpenVMM, and the parent would wait for the VM to -/// exit before seeing end-of-file. The handle list restricts inheritance to `stdio`. +/// exit before seeing end-of-file. The handle list restricts inheritance to `stdio` and the +/// caller's handles `inherit`, which keep their values in the child. /// -/// The `stdio` handles are inheritable only from here until the caller drops them after the -/// spawn, so a process that the caller spawns concurrently through `std::process` could inherit -/// them in that window. +/// A handle in the list must be inheritable while the process is created. The `stdio` handles +/// are inheritable from here until the spawn closes them, and the handles `inherit` from here +/// until the spawn restores their earlier inheritability, right after the process is created. +/// A process that the caller spawns concurrently without a handle list, as `std::process` does, +/// could inherit them in that window; a process that it spawns with one could not. pub(crate) fn spawn_detached( program: &Path, arguments: &[OsString], working_dir: &Path, stdio: [OwnedHandle; 3], + inherit: &[Inherited<'_>], breakaway_from_job: bool, ) -> io::Result { let application = wide(program); let mut command_line = command_line(program.as_os_str(), arguments)?; let directory = wide(working_dir); - let handles = stdio.each_ref().map(raw); - for handle in handles { + let mut handles = stdio.each_ref().map(raw).to_vec(); + for handle in &handles { // SAFETY: the handle is open and owned by `stdio`. - if unsafe { SetHandleInformation(handle, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT) } == 0 { + if unsafe { SetHandleInformation(*handle, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT) } == 0 { return Err(io::Error::last_os_error()); } } + for inherited in inherit { + let handle = inherited.as_handle().as_raw_handle() as HANDLE; + // A handle that the list names twice fails the spawn. + if !handles.contains(&handle) { + handles.push(handle); + } + } + let inheritable = InheritableForSpawn::new(&handles[stdio.len()..])?; let mut attributes = AttributeList::new(1)?; // SAFETY: the list has room for one attribute, and `handles` outlives CreateProcessW. if unsafe { @@ -421,7 +482,7 @@ pub(crate) fn spawn_detached( 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST as usize, handles.as_ptr().cast(), - mem::size_of_val(&handles), + mem::size_of_val(handles.as_slice()), ptr::null_mut(), ptr::null(), ) @@ -433,11 +494,9 @@ pub(crate) fn spawn_detached( let mut startup: STARTUPINFOEXW = unsafe { mem::zeroed() }; startup.StartupInfo.cb = mem::size_of::() as u32; startup.StartupInfo.dwFlags = STARTF_USESTDHANDLES; - [ - startup.StartupInfo.hStdInput, - startup.StartupInfo.hStdOutput, - startup.StartupInfo.hStdError, - ] = handles; + startup.StartupInfo.hStdInput = handles[0]; + startup.StartupInfo.hStdOutput = handles[1]; + startup.StartupInfo.hStdError = handles[2]; startup.lpAttributeList = attributes.as_ptr(); let mut flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT @@ -464,9 +523,16 @@ pub(crate) fn spawn_detached( &mut information, ) }; - if created == 0 { - return Err(io::Error::last_os_error()); - } + let created = if created == 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + }; + // The child holds its own copies of the handles now, so the caller's need not be + // inheritable any longer. + drop(inheritable); + drop(stdio); + created?; drop(owned(information.hThread)); let process = owned(information.hProcess) .ok_or_else(|| io::Error::other("CreateProcessW returned no process handle"))?; @@ -1264,6 +1330,7 @@ mod tests { &[OsString::from("/c"), OsString::from("ping -n 30 127.0.0.1")], &std::env::temp_dir(), [stdin.into(), stdout.into(), stderr.into()], + &[], false, ) .unwrap(); @@ -1283,4 +1350,100 @@ mod tests { .is_empty() ); } + + /// Returns whether the handle `value` of `process` refers to the object that `ours` holds, as + /// a handle that the process inherited from this one does, under the same value. + fn holds(process: &OwnedHandle, value: HANDLE, ours: &impl AsRawHandle) -> bool { + use windows_sys::Win32::Foundation::{ + CompareObjectHandles, DUPLICATE_SAME_ACCESS, DuplicateHandle, + }; + + let mut copy = ptr::null_mut(); + // SAFETY: the process handle has full access, the value is only looked up in that + // process, and the output pointer is valid. + let duplicated = unsafe { + DuplicateHandle( + raw(process), + value, + GetCurrentProcess(), + &mut copy, + 0, + 0, + DUPLICATE_SAME_ACCESS, + ) + } != 0; + // A value that names no handle of the process names none of ours. + let Some(copy) = duplicated.then(|| owned(copy)).flatten() else { + return false; + }; + // SAFETY: both handles are open. + unsafe { CompareObjectHandles(raw(©), ours.as_raw_handle() as HANDLE) != 0 } + } + + /// Returns whether `handle` is inheritable. + fn inheritable(handle: &impl AsRawHandle) -> bool { + let mut flags = 0; + // SAFETY: the handle is open, and the output pointer is valid. + let read = unsafe { GetHandleInformation(handle.as_raw_handle() as HANDLE, &mut flags) }; + assert_ne!(read, 0, "{}", io::Error::last_os_error()); + flags & HANDLE_FLAG_INHERIT != 0 + } + + /// Makes `handle` inheritable, as native code or a parent may have left it. + fn make_inheritable(handle: &impl AsRawHandle) { + // SAFETY: the handle is open. + let set = unsafe { + SetHandleInformation( + handle.as_raw_handle() as HANDLE, + HANDLE_FLAG_INHERIT, + HANDLE_FLAG_INHERIT, + ) + }; + assert_ne!(set, 0, "{}", io::Error::last_os_error()); + } + + #[test] + fn children_inherit_the_allowed_handles_while_they_are_created() { + let (_allowed_reader, allowed) = io::pipe().unwrap(); + // A handle that was inheritable before the spawn stays so. + let (_kept_reader, kept) = io::pipe().unwrap(); + make_inheritable(&kept); + let (_other_reader, other) = io::pipe().unwrap(); + make_inheritable(&other); + assert!(!inheritable(&allowed)); + let (stdin, _stdin_writer) = io::pipe().unwrap(); + let (_stdout_reader, stdout) = io::pipe().unwrap(); + let stderr = stdout.try_clone().unwrap(); + let system = std::env::var_os("SystemRoot").unwrap(); + let ping = Path::new(&system).join("System32").join("PING.EXE"); + // A handle that the list names twice is inherited once. + let inherit = [ + Inherited::new(&allowed), + Inherited::new(&kept), + Inherited::new(&allowed), + ]; + let child = spawn_detached( + &ping, + &["-n", "30", "127.0.0.1"].map(OsString::from), + &std::env::temp_dir(), + [stdin.into(), stdout.into(), stderr.into()], + &inherit, + false, + ) + .unwrap(); + let value = |handle: &io::PipeWriter| handle.as_raw_handle() as HANDLE; + let (allowed_held, kept_held, other_held) = ( + holds(&child.process, value(&allowed), &allowed), + holds(&child.process, value(&kept), &kept), + holds(&child.process, value(&other), &other), + ); + let (allowed_inheritable, kept_inheritable) = (inheritable(&allowed), inheritable(&kept)); + assert!(child.terminate(Duration::from_secs(10))); + assert!( + allowed_held && kept_held, + "an allowed handle was not inherited" + ); + assert!(!other_held, "a handle that was not allowed was inherited"); + assert!(!allowed_inheritable && kept_inheritable); + } } diff --git a/nvx-rs/src/host/process.rs b/nvx-rs/src/host/process.rs index fc474588d..9cae954ec 100644 --- a/nvx-rs/src/host/process.rs +++ b/nvx-rs/src/host/process.rs @@ -1,4 +1,4 @@ -//! Launch and supervision of OpenVMM processes, through verified references to them. +//! Verified references to OpenVMM processes, through which callers wait for them and end them. //! //! A [`ProcessRef`] names one process by its ID, its start time, and the [`BootId`] of the host //! boot during which it started, which together never name another process. [`Process::open`] @@ -15,17 +15,17 @@ //! Linux 5.3 or in sandboxes that refuse them, check the reference at a short interval instead, //! until the deadline of the wait, and cannot terminate a process that they cannot hold. //! -//! [`spawn`] starts OpenVMM detached from the caller, and [`Launched::process`] holds the child -//! that it started through a handle of its own, which outlives the [`Launched`] that -//! [`detach_child`] releases. +//! The [`supervisor`] launches OpenVMM, and its [`Launched::process`] holds the child that it +//! started through a handle of its own. [`spawn`], [`detach_child`], and [`kill_child`] launch and +//! release OpenVMM for the CLI and [`instance`](super::instance), which keep records of their own: +//! they go through the supervisor, and remain until those callers move to it, as do +//! [`wait_for_exit`] and [`kill`]. use std::ffi::OsString; use std::fmt; use std::fs::File; -use std::io::{self, Write}; +use std::io; use std::path::Path; -#[cfg(not(windows))] -use std::process::{Child, Command, Stdio}; use std::str::FromStr; use std::sync::OnceLock; use std::thread; @@ -34,13 +34,15 @@ use std::time::{Duration, Instant}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use super::platform; +pub use super::supervisor::Launched; +use super::supervisor::{self, VmmCommand}; use crate::error::Error; use crate::protocol::CAPABILITY_LEN; /// Longest wait for a terminated OpenVMM process to exit. pub(crate) const KILL_TIMEOUT: Duration = Duration::from_secs(10); /// Interval at which process shutdown and identity are polled. -const POLL_INTERVAL: Duration = Duration::from_millis(25); +pub(crate) const POLL_INTERVAL: Duration = Duration::from_millis(25); /// The identity of one boot of the host, which changes whenever the host restarts. /// @@ -190,6 +192,12 @@ impl Process { })) } + /// Returns the process that `handle` holds, whose identity the caller verified to be + /// `reference`, as a child that it has not reaped has. + pub(crate) fn held(reference: ProcessRef, handle: platform::ProcessHandle) -> Self { + Self { reference, handle } + } + /// Returns the reference that the process matched. pub fn reference(&self) -> &ProcessRef { &self.reference @@ -299,151 +307,9 @@ fn poll_exit(deadline: Instant, mut runs: impl FnMut() -> io::Result) -> i } } -/// An OpenVMM process this caller launched and has not released yet. -/// -/// Release it with [`detach_child`], or terminate it with [`kill_child`]. [`process`] holds it -/// beyond that. -/// -/// [`process`]: Self::process -pub struct Launched { - #[cfg(not(windows))] - /// Child process retained until it is detached or terminated. - child: Child, - #[cfg(not(windows))] - /// Whether the child has been reaped, after which its process ID may name another process. - reaped: bool, - #[cfg(windows)] - /// Owned Windows process handle and identifier. - process: platform::LaunchedProcess, -} - -impl Launched { - /// Returns the process ID. - pub fn id(&self) -> u32 { - #[cfg(not(windows))] - return self.child.id(); - #[cfg(windows)] - return self.process.id(); - } - - /// Returns a [`Process`] that holds the launched process through a handle of its own. - /// - /// The caller has not reaped the process, so its ID and start time still identify it, and - /// the reference of the `Process` is the one to record: a successor of the caller can find - /// the process again with [`Process::open`]. The `Process` stays valid after - /// [`detach_child`] or [`kill_child`] releases this `Launched`. A process that exited - /// already keeps its identity, and the `Process` reports that it exited. - /// - /// Fails if the identity of the process or of the current boot cannot be read, and with - /// [`io::ErrorKind::Unsupported`] on hosts other than Linux and Windows. - pub fn process(&self) -> io::Result { - #[cfg(windows)] - let (handle, start_time) = self.process.pin()?; - #[cfg(not(windows))] - let (handle, start_time) = if self.reaped { - return Err(io::Error::other(format!( - "launched process {} was reaped, so its ID may name another process", - self.id() - ))); - } else { - platform::pin_child(self.id())? - }; - Ok(Process { - reference: ProcessRef { - pid: self.id(), - start_time, - boot: BootId::current()?, - }, - handle, - }) - } - - /// Waits up to `timeout` for the process to exit, and returns its status if it has, as - /// `Popen.wait` and `Popen.poll` report it: the exit code, or on Unix the negated number of - /// the signal that ended the process. - #[cfg(feature = "cli")] - pub(crate) fn wait_timeout(&mut self, timeout: Duration) -> io::Result> { - #[cfg(windows)] - return Ok(self.process.wait_timeout(timeout)?.map(i64::from)); - #[cfg(not(windows))] - { - let deadline = Instant::now().checked_add(timeout); - loop { - if let Some(status) = self.try_reap()? { - return Ok(Some(status_value(status))); - } - let now = Instant::now(); - match deadline { - Some(deadline) if now >= deadline => return Ok(None), - Some(deadline) => thread::sleep((deadline - now).min(POLL_INTERVAL)), - None => thread::sleep(POLL_INTERVAL), - } - } - } - } - - /// Reaps the child if it has exited, and returns its status if it has. - #[cfg(all(feature = "cli", not(windows)))] - fn try_reap(&mut self) -> io::Result> { - let status = self.child.try_wait()?; - self.reaped |= status.is_some(); - Ok(status) - } - - /// Asks the process to end, as `Popen.terminate` does: with `SIGTERM` on Unix, and by - /// terminating it with status 1 on Windows. A process that has exited is left alone. - #[cfg(feature = "cli")] - pub(crate) fn terminate(&mut self) -> io::Result<()> { - #[cfg(windows)] - return self.process.request_termination(); - #[cfg(not(windows))] - self.signal(nix::sys::signal::Signal::SIGTERM) - } - - /// Ends the process, as `Popen.kill` does: with `SIGKILL` on Unix, and by terminating it with - /// status 1 on Windows. A process that has exited is left alone. - #[cfg(feature = "cli")] - pub(crate) fn kill(&mut self) -> io::Result<()> { - #[cfg(windows)] - return self.process.request_termination(); - #[cfg(not(windows))] - self.signal(nix::sys::signal::Signal::SIGKILL) - } - - /// Sends `signal` to the process unless it has exited. The child is not reaped until it is - /// waited for, so its process ID cannot name another process meanwhile. - #[cfg(all(feature = "cli", not(windows)))] - fn signal(&mut self, signal: nix::sys::signal::Signal) -> io::Result<()> { - if self.try_reap()?.is_some() { - return Ok(()); - } - let pid = i32::try_from(self.child.id()).map_err(io::Error::other)?; - match nix::sys::signal::kill(nix::unistd::Pid::from_raw(pid), signal) { - Ok(()) | Err(nix::errno::Errno::ESRCH) => Ok(()), - Err(error) => Err(error.into()), - } - } -} - -/// Returns `Popen.returncode` for `status`: the exit code, or the negated signal number. -#[cfg(all(feature = "cli", not(windows)))] -fn status_value(status: std::process::ExitStatus) -> i64 { - use std::os::unix::process::ExitStatusExt; - - match (status.code(), status.signal()) { - (Some(code), _) => i64::from(code), - (None, Some(signal)) => -i64::from(signal), - (None, None) => 0, - } -} - -/// Starts OpenVMM detached from the caller. -/// -/// OpenVMM reads its capability from standard input as soon as it starts, so the capability is -/// written into a pipe whose write end is closed before the spawn. The capability never appears -/// in arguments, the environment, or the log. OpenVMM output goes to `log`, and OpenVMM inherits -/// no other handle of the caller, so a caller whose own output is a pipe sees end-of-file when it -/// exits rather than when the VM does. +/// Starts OpenVMM detached from the caller, with `capability` on its standard input, `log` as +/// its standard output and error, and nothing else of the caller's, as [`supervisor::spawn`] +/// starts a VMM that inherits no descriptor or handle beyond its standard streams. /// /// On Windows, `breakaway_from_job` starts OpenVMM outside the caller's job object, which must /// permit breakaway, so that the VM survives the caller. @@ -455,84 +321,21 @@ pub fn spawn( log: File, working_dir: &Path, ) -> io::Result { - let (stdin, mut writer) = io::pipe()?; - writer.write_all(capability)?; - drop(writer); - let stderr = log.try_clone()?; - #[cfg(windows)] - { - platform::spawn_detached( - openvmm, - arguments, - working_dir, - [stdin.into(), log.into(), stderr.into()], - breakaway_from_job, - ) - .map(|process| Launched { process }) - } - #[cfg(not(windows))] - { - // Rust opens its own descriptors close-on-exec, and `platform::detach` marks any other - // descriptor of the caller, so OpenVMM receives only these. - let mut command = Command::new(openvmm); - command - .args(arguments) - .current_dir(working_dir) - .stdin(Stdio::from(stdin)) - .stdout(Stdio::from(log)) - .stderr(Stdio::from(stderr)); - platform::detach(&mut command, breakaway_from_job); - command.spawn().map(|child| Launched { - child, - reaped: false, - }) - } + let mut command = VmmCommand::new(openvmm, arguments.to_vec(), working_dir); + command.breakaway_from_job = breakaway_from_job; + supervisor::spawn(&command, capability, log) } -/// Releases a launched process without waiting for it. -/// -/// On Unix a background thread reaps the child once it exits so it does not linger as a zombie -/// while this process lives. If that thread cannot start, the child becomes a zombie after it -/// exits; liveness checks treat zombies without threads as exited, so only the process table -/// entry leaks. The OpenVMM process keeps running either way. +/// Releases a launched process without waiting for it, as [`Launched::detach`] does. pub fn detach_child(launched: Launched) { - #[cfg(not(windows))] - { - let mut child = launched.child; - let _ = thread::Builder::new() - .name("nvx-openvmm-reaper".to_owned()) - .spawn(move || { - let _ = child.wait(); - }); - } - #[cfg(windows)] - drop(launched); + launched.detach(); } /// Kills a launched process that was never recorded, through its handle, which cannot race with -/// process ID reuse. Returns whether it exited within ten seconds. +/// process ID reuse, as [`Launched::kill_and_wait`] does. Returns whether it exited within ten +/// seconds. pub fn kill_child(launched: Launched) -> bool { - #[cfg(windows)] - return launched.process.terminate(KILL_TIMEOUT); - #[cfg(not(windows))] - { - let mut child = launched.child; - if child.kill().is_ok() { - let deadline = Instant::now() + KILL_TIMEOUT; - loop { - match child.try_wait() { - Ok(Some(_)) => return true, - Ok(None) if Instant::now() < deadline => thread::sleep(POLL_INTERVAL), - _ => break, - } - } - } - detach_child(Launched { - child, - reaped: launched.reaped, - }); - false - } + launched.kill_and_wait() } /// Waits until the process with the given identity is gone, returning whether it exited before @@ -871,68 +674,17 @@ mod reference_tests { #[cfg(all(test, target_os = "linux"))] mod tests { use std::fs; - use std::os::fd::AsRawFd; + use std::process::{Child, Command, Stdio}; use super::*; use crate::host::sys; - /// Spawns a long-running child while this process holds an inheritable pipe writer, as a - /// descriptor that the caller's parent or native code left without close-on-exec would be. - /// Returns whether the child holds a copy, as its own descriptor table shows. Unlike the - /// pipe reaching end-of-file, that cannot depend on the children of concurrent tests, which - /// may inherit the writer too. - fn child_inherits_writer(spawn: impl FnOnce() -> Child) -> bool { - let (pipe, child) = { - let (_, writer) = io::pipe().unwrap(); - sys::clear_close_on_exec(&writer).unwrap(); - let pipe = fs::read_link(format!("/proc/self/fd/{}", writer.as_raw_fd())).unwrap(); - (pipe, Reaped(spawn())) - }; - // `spawn` returns once the child has executed its program, which closed the child's - // close-on-exec copies, so a copy that remains was inherited. - fs::read_dir(format!("/proc/{}/fd", child.0.id())) - .unwrap() - .any(|entry| fs::read_link(entry.unwrap().path()).is_ok_and(|target| target == pipe)) - } - fn sleeper() -> Command { let mut command = Command::new("sleep"); command.arg("60").stdin(Stdio::null()); command } - #[test] - fn openvmm_inherits_no_descriptor_beyond_its_standard_streams() { - // A plain child keeps the writer, which shows that the check detects a leak. - let plain = || sleeper().spawn().unwrap(); - assert!(child_inherits_writer(plain)); - - let directory = tempfile::tempdir().unwrap(); - let openvmm = || { - let log = File::create(directory.path().join("openvmm.log")).unwrap(); - let arguments = [OsString::from("60")]; - let capability = [1; CAPABILITY_LEN]; - let launched = spawn( - Path::new("sleep"), - false, - &arguments, - &capability, - log, - directory.path(), - ); - launched.unwrap().child - }; - assert!(!child_inherits_writer(openvmm)); - - // Kernels without CLOSE_RANGE_CLOEXEC take the /proc/self/fd fallback. - let fallback = || { - let mut command = sleeper(); - sys::mark_listed_descriptors_on_spawn(&mut command); - command.spawn().unwrap() - }; - assert!(!child_inherits_writer(fallback)); - } - #[test] fn zombies_run_while_other_threads_remain() { // Field 22 of a stat line, the start time, is 22 here; the command name holds a space diff --git a/nvx-rs/src/host/store/mod.rs b/nvx-rs/src/host/store/mod.rs index aa161da5f..ceb8c9b89 100644 --- a/nvx-rs/src/host/store/mod.rs +++ b/nvx-rs/src/host/store/mod.rs @@ -49,12 +49,15 @@ //! 3. Once the VM is ready, [`Launch::commit`] replaces the journal with the runtime record. A //! launch that fails ends with [`Launch::abandon`]. //! +//! [`supervisor::launch`](super::supervisor::launch) runs these steps. A caller that runs them +//! itself does so as follows: +//! //! ```no_run //! use std::time::{Duration, Instant}; //! //! use nvx::host::launch::LaunchSpec; -//! use nvx::host::process; //! use nvx::host::store::{RuntimeIdentity, SandboxStore}; +//! use nvx::host::supervisor::{self, VmmCommand}; //! use nvx::protocol::CAPABILITY_LEN; //! //! fn launch( @@ -67,12 +70,13 @@ //! locked.recover()?; //! let mut launch = locked.begin_launch(runtime, capability)?; //! let arguments = spec.openvmm_arguments(launch.endpoint(), &store.outcome_path())?; +//! let command = VmmCommand::new(&spec.openvmm, arguments, store.dir()); //! let log = launch.take_log()?; -//! let child = process::spawn(&spec.openvmm, false, &arguments, capability, log, store.dir())?; +//! let child = supervisor::spawn(&command, capability, log)?; //! launch.record_process(&child)?; //! // Wait until the VM is ready, then commit, or else abandon the launch. //! launch.commit()?; -//! process::detach_child(child); +//! child.detach(); //! Ok(()) //! } //! ``` @@ -131,8 +135,9 @@ //! exited once the claim is released; until then, a process of the launch may still run, and //! recovery is indeterminate. //! -//! The runtime record of a committed launch names its running VM, and reattaching to that VM -//! belongs to its supervisor. +//! The runtime record of a committed launch names its running VM, and +//! [`supervisor::reattach`](super::supervisor::reattach) holds that VM again after its +//! orchestrator restarts. mod files; mod identity; @@ -662,7 +667,7 @@ impl Launch<'_> { } /// Takes the claimed log, which the caller passes to the VMM as its standard output and - /// error, as [`spawn`](super::process::spawn) does with its `log`, once. + /// error, as [`spawn`](super::supervisor::spawn) does with its `log`, once. /// /// The launch keeps no copy of the log, so the claim lasts as long as the processes that hold /// it. @@ -717,10 +722,9 @@ impl Launch<'_> { /// nothing of it runs. /// /// A caller that still holds the VMM's [`Launched`] process releases it as usual. If the - /// launch never recorded that process, the caller ends it with - /// [`kill_child`](super::process::kill_child) first: an unrecorded VMM that serves no - /// endpoint yet holds the log's claim, so the launch would end as - /// [`Recovery::Indeterminate`]. + /// launch never recorded that process, the caller ends it with [`Launched::kill_and_wait`] + /// first: an unrecorded VMM that serves no endpoint yet holds the log's claim, so the launch + /// would end as [`Recovery::Indeterminate`]. pub fn abandon(mut self) -> Result { // A log that no VMM received holds no claim once it closes. drop(self.log.take()); diff --git a/nvx-rs/src/host/supervisor/child.rs b/nvx-rs/src/host/supervisor/child.rs new file mode 100644 index 000000000..8a6127e38 --- /dev/null +++ b/nvx-rs/src/host/supervisor/child.rs @@ -0,0 +1,536 @@ +//! The detached spawn of a VMM, with an explicit allowlist of the descriptors or handles of the +//! caller that it inherits, and the child that the spawn returns until its caller releases it. + +use std::ffi::OsString; +use std::fmt; +use std::fs::File; +use std::io::{self, Write}; +#[cfg(not(any(unix, windows)))] +use std::marker::PhantomData; +#[cfg(unix)] +use std::os::fd::{AsFd, BorrowedFd}; +#[cfg(windows)] +use std::os::windows::io::{AsHandle, BorrowedHandle}; +use std::path::PathBuf; +#[cfg(not(windows))] +use std::process::{Child, Command, Stdio}; +#[cfg(not(windows))] +use std::thread; +#[cfg(feature = "cli")] +use std::time::Duration; +#[cfg(not(windows))] +use std::time::Instant; + +use crate::host::platform; +#[cfg(not(windows))] +use crate::host::process::POLL_INTERVAL; +use crate::host::process::{BootId, KILL_TIMEOUT, Process, ProcessRef}; +use crate::protocol::CAPABILITY_LEN; + +/// The descriptor of the caller that an [`Inherited`] borrows. +#[cfg(unix)] +type Borrowed<'a> = BorrowedFd<'a>; +/// The handle of the caller that an [`Inherited`] borrows. +#[cfg(windows)] +type Borrowed<'a> = BorrowedHandle<'a>; +/// What an [`Inherited`] would borrow on a host without descriptors or handles, which has none. +#[cfg(not(any(unix, windows)))] +#[derive(Debug, Clone, Copy)] +struct Borrowed<'a>(std::convert::Infallible, PhantomData<&'a ()>); + +/// A descriptor (Unix) or handle (Windows) of the caller, such as that of an open [`File`], that a +/// VMM inherits under the number that it has in the caller, which the caller can therefore name in +/// the VMM's arguments. +/// +/// [`spawn`] passes the VMM these and its standard streams, and nothing else of the caller's: +/// +/// - On Linux, the descriptor stays close-on-exec in the caller, as Rust opens its files, and +/// only the VMM's own process clears the flag, between its fork and its exec, so no process +/// that another thread spawns meanwhile inherits it. The VMM opens it again as +/// `/proc/self/fd/N`, which `proc_path` returns. +/// - On Windows, the handle joins the explicit list of handles that the VMM inherits, and is +/// inheritable only while the VMM is created, after which the spawn restores its earlier +/// inheritability. A process that another thread creates in that window without such a list, +/// as `std::process::Command` does, can inherit it too. +/// +/// Other hosts fail a spawn that passes any. +#[derive(Debug, Clone, Copy)] +pub struct Inherited<'a>(Borrowed<'a>); + +impl<'a> Inherited<'a> { + /// Names the descriptor of `object`, such as a [`File`]. + #[cfg(unix)] + pub fn new(object: &'a impl AsFd) -> Self { + Self(object.as_fd()) + } + + /// Names the handle of `object`, such as a [`File`]. + #[cfg(windows)] + pub fn new(object: &'a impl AsHandle) -> Self { + Self(object.as_handle()) + } +} + +#[cfg(target_os = "linux")] +impl Inherited<'_> { + /// Returns the path through which the VMM opens the descriptor again: `/proc/self/fd/N`, + /// where `N` is its number in the caller and in the VMM. + pub fn proc_path(&self) -> PathBuf { + use std::os::fd::AsRawFd; + + PathBuf::from(format!("/proc/self/fd/{}", self.0.as_raw_fd())) + } +} + +#[cfg(unix)] +impl<'a> From> for Inherited<'a> { + fn from(descriptor: BorrowedFd<'a>) -> Self { + Self(descriptor) + } +} + +#[cfg(windows)] +impl<'a> From> for Inherited<'a> { + fn from(handle: BorrowedHandle<'a>) -> Self { + Self(handle) + } +} + +#[cfg(unix)] +impl AsFd for Inherited<'_> { + fn as_fd(&self) -> BorrowedFd<'_> { + self.0 + } +} + +#[cfg(windows)] +impl AsHandle for Inherited<'_> { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.0 + } +} + +/// How [`spawn`] starts a VMM: its executable, arguments, and working directory, whether it +/// leaves the caller's job object on Windows, and the descriptors or handles of the caller that it +/// inherits besides its standard streams. +#[derive(Debug, Clone)] +pub struct VmmCommand<'a> { + /// The VMM's executable. + pub program: PathBuf, + /// The VMM's arguments, after the name of its executable. + pub arguments: Vec, + /// The VMM's working directory. + pub working_dir: PathBuf, + /// Whether the VMM starts outside the caller's job object on Windows, which must permit + /// breakaway, so that the VMM survives the job. Other hosts ignore it. + pub breakaway_from_job: bool, + /// The descriptors or handles of the caller that the VMM inherits under their numbers in the + /// caller, besides its standard streams. One that the list names twice is inherited once. + pub inherit: Vec>, +} + +impl VmmCommand<'_> { + /// Returns the command that starts `program` with `arguments` in `working_dir`, inside the + /// caller's job object, and with nothing of the caller's besides its standard streams. + pub fn new( + program: impl Into, + arguments: Vec, + working_dir: impl Into, + ) -> Self { + Self { + program: program.into(), + arguments, + working_dir: working_dir.into(), + breakaway_from_job: false, + inherit: Vec::new(), + } + } +} + +/// Starts the VMM that `command` describes, detached from the caller, with `capability` on its +/// standard input and `log` as its standard output and error. +/// +/// OpenVMM reads its capability from standard input as soon as it starts, so the capability is +/// written into a pipe whose write end is closed before the spawn: it never appears in arguments, +/// the environment, or the log. The VMM inherits its standard streams and the descriptors or +/// handles that `command` allows, as [`Inherited`] describes, and nothing else of the caller's, +/// so a caller whose own output is a pipe sees its end when the caller exits rather than when the +/// VM does. On Linux, the VMM starts in a new session, so that it outlives the caller and its +/// terminal; on Windows, without a console window, and outside the caller's job object when +/// `command` asks for it. +/// +/// Record the VMM's identity right after the spawn, with +/// [`Launch::record_process`](crate::host::store::Launch::record_process), as +/// [`launch`](super::launch) does. Fails with [`io::ErrorKind::InvalidInput`] for a descriptor of +/// a standard stream, 0 to 2, on Linux, and with [`io::ErrorKind::Unsupported`] for any descriptor +/// or handle on hosts other than Linux and Windows. +pub fn spawn( + command: &VmmCommand<'_>, + capability: &[u8; CAPABILITY_LEN], + log: File, +) -> io::Result { + let (stdin, mut writer) = io::pipe()?; + writer.write_all(capability)?; + drop(writer); + let stderr = log.try_clone()?; + #[cfg(windows)] + { + platform::spawn_detached( + &command.program, + &command.arguments, + &command.working_dir, + [stdin.into(), log.into(), stderr.into()], + &command.inherit, + command.breakaway_from_job, + ) + .map(|process| Launched { process }) + } + #[cfg(not(windows))] + { + // Rust opens its own descriptors close-on-exec, and `platform::detach` marks any other + // descriptor of the caller, except those that `command` allows, so OpenVMM receives only + // these and its standard streams. + let mut child = Command::new(&command.program); + child + .args(&command.arguments) + .current_dir(&command.working_dir) + .stdin(Stdio::from(stdin)) + .stdout(Stdio::from(log)) + .stderr(Stdio::from(stderr)); + platform::detach(&mut child, &command.inherit)?; + child.spawn().map(|child| Launched { + child, + reaped: false, + }) + } +} + +/// A VMM that [`spawn`] started, which the caller has not released yet: on Unix, its child +/// process, which the caller has not reaped, so that its ID names no other process; on Windows, +/// its process handle. +/// +/// Record it with [`Launch::record_process`](crate::host::store::Launch::record_process), or hold +/// it with [`process`](Self::process). Then release it with [`detach`](Self::detach), or end it +/// with [`kill_and_wait`](Self::kill_and_wait); the [`Process`] stays valid either way. +pub struct Launched { + #[cfg(not(windows))] + /// Child process retained until it is detached or terminated. + child: Child, + #[cfg(not(windows))] + /// Whether the child has been reaped, after which its process ID may name another process. + reaped: bool, + #[cfg(windows)] + /// Owned Windows process handle and identifier. + process: platform::LaunchedProcess, +} + +impl fmt::Debug for Launched { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("Launched") + .field("id", &self.id()) + .finish_non_exhaustive() + } +} + +impl Launched { + /// Returns the process ID. + pub fn id(&self) -> u32 { + #[cfg(not(windows))] + return self.child.id(); + #[cfg(windows)] + return self.process.id(); + } + + /// Returns a [`Process`] that holds the launched process through a handle of its own. + /// + /// The caller has not reaped the process, so its ID and start time still identify it, and + /// the reference of the `Process` is the one to record: a successor of the caller can find + /// the process again with [`Process::open`]. The `Process` stays valid after + /// [`detach`](Self::detach) or [`kill_and_wait`](Self::kill_and_wait) releases this + /// `Launched`. A process that exited already keeps its identity, and the `Process` reports + /// that it exited. + /// + /// Fails if the identity of the process or of the current boot cannot be read, and with + /// [`io::ErrorKind::Unsupported`] on hosts other than Linux and Windows. + pub fn process(&self) -> io::Result { + #[cfg(windows)] + let (handle, start_time) = self.process.pin()?; + #[cfg(not(windows))] + let (handle, start_time) = if self.reaped { + return Err(io::Error::other(format!( + "launched process {} was reaped, so its ID may name another process", + self.id() + ))); + } else { + platform::pin_child(self.id())? + }; + let reference = ProcessRef { + pid: self.id(), + start_time, + boot: BootId::current()?, + }; + Ok(Process::held(reference, handle)) + } + + /// Releases the process without waiting for it: it keeps running. + /// + /// On Unix a background thread reaps the child once it exits so it does not linger as a + /// zombie while this process lives. If that thread cannot start, the child becomes a zombie + /// after it exits; liveness checks treat zombies without threads as exited, so only the + /// process table entry leaks. + pub fn detach(self) { + #[cfg(not(windows))] + { + let mut child = self.child; + let _ = thread::Builder::new() + .name("nvx-openvmm-reaper".to_owned()) + .spawn(move || { + let _ = child.wait(); + }); + } + #[cfg(windows)] + drop(self); + } + + /// Ends the process through its handle, which cannot race with the reuse of its ID, and + /// returns whether it exited within ten seconds. A process that did not is released as + /// [`detach`](Self::detach) releases it. + pub fn kill_and_wait(self) -> bool { + #[cfg(windows)] + return self.process.terminate(KILL_TIMEOUT); + #[cfg(not(windows))] + { + let mut child = self.child; + if child.kill().is_ok() { + let deadline = Instant::now() + KILL_TIMEOUT; + loop { + match child.try_wait() { + Ok(Some(_)) => return true, + Ok(None) if Instant::now() < deadline => thread::sleep(POLL_INTERVAL), + _ => break, + } + } + } + Self { + child, + reaped: self.reaped, + } + .detach(); + false + } + } + + /// Waits up to `timeout` for the process to exit, and returns its status if it has, as + /// `Popen.wait` and `Popen.poll` report it: the exit code, or on Unix the negated number of + /// the signal that ended the process. + #[cfg(feature = "cli")] + pub(crate) fn wait_timeout(&mut self, timeout: Duration) -> io::Result> { + #[cfg(windows)] + return Ok(self.process.wait_timeout(timeout)?.map(i64::from)); + #[cfg(not(windows))] + { + let deadline = Instant::now().checked_add(timeout); + loop { + if let Some(status) = self.try_reap()? { + return Ok(Some(status_value(status))); + } + let now = Instant::now(); + match deadline { + Some(deadline) if now >= deadline => return Ok(None), + Some(deadline) => thread::sleep((deadline - now).min(POLL_INTERVAL)), + None => thread::sleep(POLL_INTERVAL), + } + } + } + } + + /// Reaps the child if it has exited, and returns its status if it has. + #[cfg(all(feature = "cli", not(windows)))] + fn try_reap(&mut self) -> io::Result> { + let status = self.child.try_wait()?; + self.reaped |= status.is_some(); + Ok(status) + } + + /// Asks the process to end, as `Popen.terminate` does: with `SIGTERM` on Unix, and by + /// terminating it with status 1 on Windows. A process that has exited is left alone. + #[cfg(feature = "cli")] + pub(crate) fn terminate(&mut self) -> io::Result<()> { + #[cfg(windows)] + return self.process.request_termination(); + #[cfg(not(windows))] + self.signal(nix::sys::signal::Signal::SIGTERM) + } + + /// Ends the process, as `Popen.kill` does: with `SIGKILL` on Unix, and by terminating it with + /// status 1 on Windows. A process that has exited is left alone. + #[cfg(feature = "cli")] + pub(crate) fn kill(&mut self) -> io::Result<()> { + #[cfg(windows)] + return self.process.request_termination(); + #[cfg(not(windows))] + self.signal(nix::sys::signal::Signal::SIGKILL) + } + + /// Sends `signal` to the process unless it has exited. The child is not reaped until it is + /// waited for, so its process ID cannot name another process meanwhile. + #[cfg(all(feature = "cli", not(windows)))] + fn signal(&mut self, signal: nix::sys::signal::Signal) -> io::Result<()> { + if self.try_reap()?.is_some() { + return Ok(()); + } + let pid = i32::try_from(self.child.id()).map_err(io::Error::other)?; + match nix::sys::signal::kill(nix::unistd::Pid::from_raw(pid), signal) { + Ok(()) | Err(nix::errno::Errno::ESRCH) => Ok(()), + Err(error) => Err(error.into()), + } + } +} + +/// Returns `Popen.returncode` for `status`: the exit code, or the negated signal number. +#[cfg(all(feature = "cli", not(windows)))] +fn status_value(status: std::process::ExitStatus) -> i64 { + use std::os::unix::process::ExitStatusExt; + + match (status.code(), status.signal()) { + (Some(code), _) => i64::from(code), + (None, Some(signal)) => -i64::from(signal), + (None, None) => 0, + } +} + +#[cfg(all(test, target_os = "linux"))] +mod tests { + use std::fs; + use std::os::fd::{AsRawFd, RawFd}; + use std::path::Path; + + use nix::fcntl::{FcntlArg, FdFlag, fcntl}; + + use super::*; + use crate::host::sys; + + /// Kills and reaps a child process when dropped, even when a test fails. + struct Reaped(Child); + + impl Drop for Reaped { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } + } + + /// Returns the object that the descriptor `descriptor` of process `pid` holds, as the target + /// of its link in `/proc`, or `None` if the process has no such descriptor. + fn object(pid: u32, descriptor: RawFd) -> Option { + fs::read_link(format!("/proc/{pid}/fd/{descriptor}")).ok() + } + + /// Returns whether process `pid` holds a descriptor of `object`, under any number. + fn holds(pid: u32, object: &Path) -> bool { + fs::read_dir(format!("/proc/{pid}/fd")) + .unwrap() + .any(|entry| fs::read_link(entry.unwrap().path()).is_ok_and(|target| target == object)) + } + + /// Returns whether the descriptor `descriptor` of this process is close-on-exec. + fn close_on_exec(descriptor: BorrowedFd<'_>) -> bool { + FdFlag::from_bits_retain(fcntl(descriptor, FcntlArg::F_GETFD).unwrap()) + .contains(FdFlag::FD_CLOEXEC) + } + + /// Spawns a long-running child with `spawn` while this process holds an inheritable pipe + /// writer, as a descriptor that the caller's parent or native code left without close-on-exec + /// would be. Returns whether the child holds a copy, as its own descriptor table shows. Unlike + /// the pipe reaching end-of-file, that cannot depend on the children of concurrent tests, + /// which may inherit the writer too. + fn child_inherits_writer(spawn: impl FnOnce() -> Child) -> bool { + let (pipe, child) = { + let (_, writer) = io::pipe().unwrap(); + sys::clear_close_on_exec(&writer).unwrap(); + let pipe = object(std::process::id(), writer.as_raw_fd()).unwrap(); + (pipe, Reaped(spawn())) + }; + // `spawn` returns once the child has executed its program, which closed the child's + // close-on-exec copies, so a copy that remains was inherited. + holds(child.0.id(), &pipe) + } + + /// Returns a command that runs for a minute unless it is killed. + fn sleeper() -> Command { + let mut command = Command::new("sleep"); + command.arg("60").stdin(Stdio::null()); + command + } + + /// Starts `sleep 60` as a VMM in `directory`, which also inherits `inherit`. + fn spawn_sleeper(directory: &Path, inherit: Vec>) -> io::Result { + let log = File::create(directory.join("vmm.log"))?; + let mut command = VmmCommand::new("sleep", vec![OsString::from("60")], directory); + command.inherit = inherit; + spawn(&command, &[1; CAPABILITY_LEN], log) + } + + #[test] + fn vmms_inherit_no_descriptor_beyond_their_standard_streams() { + // A plain child keeps the writer, which shows that the check detects a leak. + let plain = || sleeper().spawn().unwrap(); + assert!(child_inherits_writer(plain)); + + let directory = tempfile::tempdir().unwrap(); + let vmm = || spawn_sleeper(directory.path(), Vec::new()).unwrap().child; + assert!(!child_inherits_writer(vmm)); + + // Kernels without CLOSE_RANGE_CLOEXEC take the /proc/self/fd fallback. + let fallback = || { + let mut command = sleeper(); + sys::mark_listed_descriptors_on_spawn(&mut command); + command.spawn().unwrap() + }; + assert!(!child_inherits_writer(fallback)); + } + + #[test] + fn vmms_inherit_the_allowed_descriptors_under_their_numbers() { + let directory = tempfile::tempdir().unwrap(); + let (_reader, pipe) = io::pipe().unwrap(); + let file = File::create(directory.path().join("distro")).unwrap(); + // A descriptor that the list names twice is inherited once. + let inherit = vec![ + Inherited::new(&pipe), + Inherited::new(&file), + Inherited::new(&pipe), + ]; + let vmm = Reaped(spawn_sleeper(directory.path(), inherit).unwrap().child); + let (pid, this) = (vmm.0.id(), std::process::id()); + for descriptor in [pipe.as_raw_fd(), file.as_raw_fd()] { + let inherited = object(pid, descriptor); + assert!( + inherited.is_some(), + "descriptor {descriptor} was not inherited" + ); + assert_eq!(inherited, object(this, descriptor)); + } + // Only the VMM's process cleared their close-on-exec flags. + assert!(close_on_exec(pipe.as_fd())); + assert!(close_on_exec(file.as_fd())); + assert_eq!( + Inherited::new(&file).proc_path(), + PathBuf::from(format!("/proc/self/fd/{}", file.as_raw_fd())) + ); + } + + #[test] + fn standard_streams_cannot_be_inherited() { + let directory = tempfile::tempdir().unwrap(); + for stream in [ + io::stdin().as_fd(), + io::stdout().as_fd(), + io::stderr().as_fd(), + ] { + let error = spawn_sleeper(directory.path(), vec![Inherited::from(stream)]).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + } + } +} diff --git a/nvx-rs/src/host/supervisor/mod.rs b/nvx-rs/src/host/supervisor/mod.rs new file mode 100644 index 000000000..46f287f30 --- /dev/null +++ b/nvx-rs/src/host/supervisor/mod.rs @@ -0,0 +1,608 @@ +//! The supervisor of a sandbox's VMM, beneath the CLI, [`instance`](super::instance), and +//! external orchestrators: it launches OpenVMM through the [`store`](super::store)'s launch +//! journal, passing it only the descriptors or handles that the caller allows, waits for its exit +//! by event, stops it, and reattaches to it after the orchestrator restarts within the same host +//! boot. +//! +//! Callers keep the configuration, the launch planning, the authorization, and the application +//! protocol. The supervisor owns no control session: the caller checks that a launch is ready, and +//! asks a guest to shut down, through its own transport. +//! +//! # Launches +//! +//! [`launch`] runs the store's launch steps under the sandbox's lock: +//! +//! 1. [`LockedStore::begin_launch`] writes the journal's marker, and the caller's `command` builds +//! the VMM's [`VmmCommand`] with the control endpoint that the launch chose. +//! 2. [`spawn`] starts the VMM with the launch's claimed log as its standard output and error, and +//! [`Launch::record_process`] records its identity right after. +//! 3. The caller checks, through its own transport, that the VMM serves its control endpoint, and +//! anything else that it requires of a launch, then [`Starting::commit`]s it, which writes the +//! runtime record, or [`Starting::abandon`]s it. +//! +//! A launch that fails at any step ends as [`Launch::abandon`] ends it, and never leaves an +//! unrecorded VMM running: a VMM whose identity is not recorded is ended through its handle first. +//! +//! ```no_run +//! use std::time::{Duration, Instant}; +//! +//! use nvx::host::launch::LaunchSpec; +//! use nvx::host::store::{RuntimeIdentity, SandboxStore}; +//! use nvx::host::supervisor::{self, Reattached, StopOutcome, Vmm, VmmCommand}; +//! use nvx::protocol::CAPABILITY_LEN; +//! +//! /// Finds the sandbox's VM again after a restart of the orchestrator, or launches it. +//! fn run( +//! store: &SandboxStore, +//! spec: &LaunchSpec, +//! runtime: &RuntimeIdentity, +//! capability: &[u8; CAPABILITY_LEN], +//! ) -> nvx::Result { +//! let mut locked = store.lock(Some(Instant::now() + Duration::from_secs(10)))?; +//! match supervisor::reattach(&mut locked, runtime)? { +//! Reattached::Live(vmm) => return Ok(vmm), +//! Reattached::Exited { .. } => {} +//! // Other artifacts or contracts started the VM: end it through its verified handle, +//! // without a graceful step that it might not understand. +//! Reattached::Mismatched { vmm, .. } => { +//! let forced = Some(Duration::from_secs(10)); +//! let stopped = vmm.stop(&mut locked, Instant::now(), forced, |_, _| { +//! Ok::<(), std::convert::Infallible>(()) +//! })?; +//! if stopped.outcome == StopOutcome::TimedOut { +//! return Err(nvx::Error::timed_out("the VM of another runtime still runs")); +//! } +//! } +//! Reattached::Indeterminate(reason) => return Err(nvx::Error::other(reason)), +//! } +//! let starting = supervisor::launch(&mut locked, runtime, capability, |endpoint| { +//! let arguments = spec.openvmm_arguments(endpoint, &store.outcome_path())?; +//! Ok(VmmCommand::new(&spec.openvmm, arguments, store.dir())) +//! })?; +//! // Connect to `starting.endpoint()` through the caller's own transport, then commit once +//! // the VMM serves it, or else abandon the launch. +//! starting.commit() +//! } +//! ``` +//! +//! # Inheritance +//! +//! The VMM receives its capability on its standard input, its claimed log as its standard output +//! and error, and the descriptors (Linux) or handles (Windows) that the caller names in +//! [`VmmCommand::inherit`], under the numbers that they have in the caller, and nothing else of +//! the caller's. On Linux, every other descriptor stays close-on-exec, and only the VMM's own +//! process clears the flag of the allowed ones, so a process that another thread spawns meanwhile +//! inherits none of them. On Windows, the allowed handles join the explicit list of handles that +//! the VMM inherits, and stay inheritable only while it is created; see [`Inherited`]. +//! +//! # Exit waits and stops +//! +//! A [`Vmm`] holds its VMM through a [`Process`]: a pidfd on Linux and a process handle on +//! Windows, whose exit event wakes waiters, whether or not this caller started the VMM. Linux +//! hosts without pidfds poll the VMM's identity instead. +//! +//! [`Vmm::stop`] runs one graceful step of the caller's, with a bounded deadline, and takes a +//! bounded forced step against the VMM's verified identity only when the caller requests it. +//! +//! # Reattach +//! +//! After an orchestrator restarts within the same host boot, [`reattach`] ends an interrupted +//! launch with [`LockedStore::recover`], compares the runtime identity of the recorded VM with the +//! caller's own, with [`RuntimeRecord::check_identity`], and holds the VMM with +//! [`Process::open`]. A record from an earlier boot names a VMM that has exited. +//! +//! The supervisor signals only a process that it holds through a verified identity: the child +//! that it has not released, or a [`Process`] that matched its recorded reference. It never +//! signals a process that reuses a recorded ID. + +mod child; +#[cfg(test)] +mod tests; + +use std::io; +use std::time::{Duration, Instant}; + +pub use self::child::{Inherited, Launched, VmmCommand, spawn}; +use super::launch::timeout_problem; +use super::process::{Process, ProcessRef}; +use super::store::{ + IdentityCheck, IdentityPart, Launch, LaunchJournal, LockedStore, Recovery, RuntimeIdentity, + RuntimeRecord, SandboxStore, +}; +use crate::error::{Error, Result}; +use crate::protocol::CAPABILITY_LEN; + +/// Launches a VMM in the sandbox of `locked`, through the store's launch journal, under the +/// runtime identity `runtime`, with `capability` on its standard input. +/// +/// It begins the launch with [`LockedStore::begin_launch`], asks `command` for the VMM's command, +/// given the control endpoint that the launch chose, [`spawn`]s the VMM with the launch's claimed +/// log, and records its identity right after with [`Launch::record_process`]. The caller then +/// checks that the VMM serves its endpoint, through its own transport, and commits or abandons the +/// returned [`Starting`]. +/// +/// A step that fails ends the launch as [`Launch::abandon`] ends it, after it has ended a VMM whose +/// identity it could not record through its handle, and returns the step's error. A launch that +/// cannot be ended then, because its VMM did not exit, keeps its journal for +/// [`LockedStore::recover`], and the error says so. Fails as [`LockedStore::begin_launch`] does if +/// the sandbox's VM runs, or an interrupted launch has not been recovered. +pub fn launch<'s, 'h>( + locked: &'s mut LockedStore, + runtime: &RuntimeIdentity, + capability: &[u8; CAPABILITY_LEN], + command: impl FnOnce(&str) -> Result>, +) -> Result> { + let store = SandboxStore::clone(locked); + let mut launch = locked.begin_launch(runtime, capability)?; + let command = match command(launch.endpoint()) { + Ok(command) => command, + Err(error) => return Err(abandon_after(launch, error)), + }; + let log = match launch.take_log() { + Ok(log) => log, + Err(error) => return Err(abandon_after(launch, error)), + }; + let child = match spawn(&command, capability, log) { + Ok(child) => child, + Err(error) => { + let error = Error::io(format!("cannot launch {}", command.program.display())) + .with_source(error); + return Err(abandon_after(launch, error)); + } + }; + let process = match launch.record_process(&child) { + Ok(process) => process, + Err(error) => { + // An unrecorded VMM holds the claim of its launch, so only its handle can end it, + // before the launch ends. + child.kill_and_wait(); + return Err(abandon_after(launch, error)); + } + }; + Ok(Starting { + inner: Some(StartingInner { + launch, + child, + process, + }), + store, + }) +} + +/// Ends `launch`, which failed with `error`, and returns `error`, which says why the launch could +/// not be ended if it could not: its journal then remains for [`LockedStore::recover`]. +fn abandon_after(launch: Launch<'_>, error: Error) -> Error { + let reason = match launch.abandon() { + Ok(Recovery::Indeterminate(reason)) => reason, + Ok(_) => return error, + Err(failure) => failure.to_string(), + }; + let (kind, message, source) = error.into_parts(); + let error = Error::new( + kind, + format!("{message}; the launch could not be ended, and remains for recovery: {reason}"), + ); + match source { + Some(source) => error.with_source(source), + None => error, + } +} + +/// A launch whose VMM runs, and whose journal records the VMM's identity, until the caller +/// [commits](Self::commit) it, once the VMM serves its control endpoint, or +/// [abandons](Self::abandon) it. +/// +/// It borrows the sandbox's locked store exclusively, as the store's [`Launch`] does, so no other +/// lifecycle transition of the sandbox overlaps it. Dropping it without committing it abandons the +/// launch, as [`abandon`](Self::abandon) does, ignoring failures; a crash of the caller leaves the +/// launch's journal instead, for [`LockedStore::recover`]. +#[derive(Debug)] +pub struct Starting<'s> { + /// The launch and its VMM, until the launch is committed or abandoned. + inner: Option>, + /// The sandbox's store. + store: SandboxStore, +} + +/// The launch of a [`Starting`], and its VMM. +#[derive(Debug)] +struct StartingInner<'s> { + /// The store's launch, whose journal records the VMM. + launch: Launch<'s>, + /// The VMM's child, which the launch has not released. + child: Launched, + /// The VMM, held through its verified handle. + process: Process, +} + +impl StartingInner<'_> { + /// Ends the launch as [`Launch::abandon`] ends it, and releases its VMM's child. + fn abandon(self) -> Result { + let Self { + launch, + child, + process: _, + } = self; + let recovery = launch.abandon(); + match &recovery { + Ok(Recovery::Clean | Recovery::Exited | Recovery::Terminated(_)) => child.detach(), + // The VMM may still run: its handle ends it, and the journal that remains then shows + // that it exited. + Ok(Recovery::Indeterminate(_)) | Err(_) => { + child.kill_and_wait(); + } + } + recovery + } +} + +impl<'s> Starting<'s> { + /// Returns the launch and its VMM. + fn inner(&self) -> &StartingInner<'s> { + self.inner + .as_ref() + .expect("a launch is in progress until it is committed or abandoned") + } + + /// Returns the control endpoint that the launch chose, on which the VMM listens. + pub fn endpoint(&self) -> &str { + self.inner().launch.endpoint() + } + + /// Returns the launch's journal, which records the VMM's identity. + pub fn journal(&self) -> &LaunchJournal { + self.inner().launch.journal() + } + + /// Returns the VMM, held through its verified handle, through which the caller may wait for + /// it, for example while it checks that the VMM serves its endpoint. + pub fn process(&self) -> &Process { + &self.inner().process + } + + /// Commits the launch of a VMM that serves its control endpoint: writes its runtime record, + /// as [`Launch::commit`] does, releases the VMM's child, which keeps running, and returns the + /// VM. + /// + /// Fails if the runtime record cannot be written. The VMM is then ended through its handle, + /// and the journal that remains shows [`LockedStore::recover`] that it exited. + pub fn commit(mut self) -> Result { + let StartingInner { + launch, + child, + process, + } = self.inner.take().expect("a launch commits once"); + match launch.commit() { + Ok(record) => { + child.detach(); + Ok(Vmm { + store: self.store.clone(), + record, + process, + }) + } + Err(error) => { + // No runtime record names the VM, so its VMM must not run on. + child.kill_and_wait(); + Err(error) + } + } + } + + /// Ends a launch that failed, as [`Launch::abandon`] does: terminates its verified VMM, and + /// removes the launch's files once nothing of it runs. + /// + /// A launch that it cannot end, which [`Recovery::Indeterminate`] reports, keeps its journal + /// for [`LockedStore::recover`], after its VMM is ended through its handle. + pub fn abandon(mut self) -> Result { + self.inner + .take() + .expect("a launch is abandoned once") + .abandon() + } +} + +impl Drop for Starting<'_> { + fn drop(&mut self) { + if let Some(inner) = self.inner.take() { + let _ = inner.abandon(); + } + } +} + +/// A VM that the supervisor holds: the runtime record of its launch, and its VMM, held through a +/// verified handle, as [`Starting::commit`] and [`reattach`] return it. +/// +/// The VMM's exit event wakes its waiters, whether or not this caller started it. +#[derive(Debug)] +pub struct Vmm { + /// The sandbox's store. + store: SandboxStore, + /// The runtime record of the VM's launch. + record: RuntimeRecord, + /// The VMM, held through its verified handle. + process: Process, +} + +impl Vmm { + /// Returns the runtime record of the VM's launch: the reference to its VMM, its control + /// endpoint, and the runtime identity that it was launched with. + pub fn record(&self) -> &RuntimeRecord { + &self.record + } + + /// Returns the VM's control endpoint: a Unix socket path on Linux, and a named pipe on + /// Windows. + pub fn endpoint(&self) -> &str { + self.record.endpoint() + } + + /// Returns the VMM, held through its verified handle. + pub fn process(&self) -> &Process { + &self.process + } + + /// Waits until the VMM exits or `deadline` passes, and returns whether it exited, as + /// [`Process::wait`] does. + pub fn wait(&self, deadline: Instant) -> io::Result { + self.process.wait(deadline) + } + + /// Returns whether the VMM has exited, without waiting. + pub fn has_exited(&self) -> io::Result { + self.process.has_exited() + } + + /// Stops the VM: asks it to shut down through the caller's graceful step, waits for its VMM + /// to exit until `deadline`, and takes the forced step if the caller requests it with + /// `forced` and the VMM still runs. + /// + /// - The graceful step, `graceful`, runs once, with this VM and `deadline`, unless the VMM + /// has exited already. It asks the guest to shut down through the caller's own transport, + /// such as its control session, which the supervisor does not own. Once it succeeds, the + /// stop waits until the VMM exits or `deadline` passes. A graceful step that fails may not + /// have reached the guest, so the stop then only checks whether the VMM has exited, and + /// keeps the step's error in [`Stopped::graceful_error`]. + /// - The forced step terminates the VMM through its verified handle, and waits up to `forced` + /// for it to exit. Without `forced`, the stop never signals the VMM. + /// + /// The result says whether the VMM exited without the forced step, the forced step ended it, + /// or it still runs; see [`StopOutcome`]. Once the VMM has exited, the stop clears the runtime + /// record of its launch, as [`LockedStore::clear_runtime`] does, unless the record names + /// another launch by then. A VMM that still runs keeps its record. + /// + /// `locked` must be the locked store of the VM's sandbox. Fails with + /// [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) if `forced` is zero or longer + /// than [`MAX_TIMEOUT`](super::launch::MAX_TIMEOUT), and fails when the VMM's state cannot be + /// determined, or when the forced step cannot terminate it, as on Linux hosts without pidfds, + /// which cannot hold it. + pub fn stop( + &self, + locked: &mut LockedStore, + deadline: Instant, + forced: Option, + graceful: impl FnOnce(&Self, Instant) -> std::result::Result<(), E>, + ) -> Result> { + if locked.dir() != self.store.dir() { + return Err(Error::other(format!( + "the VM of the sandbox in {} cannot be stopped through the store in {}", + self.store.dir().display(), + locked.dir().display() + ))); + } + if let Some(problem) = forced.and_then(|bound| timeout_problem(bound, true)) { + return Err(Error::unsupported(format!( + "the bound of the forced step {problem}" + ))); + } + let stopped = stop_process( + &self.process, + self.record.process().pid, + deadline, + forced, + || graceful(self, deadline), + )?; + if stopped.outcome != StopOutcome::TimedOut + && locked + .runtime()? + .is_some_and(|record| record.launch() == self.record.launch()) + { + locked.clear_runtime()?; + } + Ok(stopped) + } +} + +/// How [`Vmm::stop`] ended. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum StopOutcome { + /// The VMM exited without the forced step: by the deadline of the graceful step, or before + /// the stop began. + Graceful, + /// The graceful step did not end the VMM, and the forced step that the caller requested + /// terminated it through its verified handle: it exited within the forced step's bound. + Forced, + /// The VMM still runs: it did not exit by the deadline of the graceful step, or, after a + /// graceful step that failed, had not exited, and the caller requested no forced step, or it + /// did not exit within the forced step's bound either. + TimedOut, +} + +/// What [`Vmm::stop`] did: how the stop ended, and the failure of its graceful step, if it +/// failed. +#[derive(Debug)] +pub struct Stopped { + /// How the stop ended. + pub outcome: StopOutcome, + /// The error of the graceful step, if the step ran and failed. + pub graceful_error: Option, +} + +/// The operations on a VMM that a stop takes, which tests replace. +trait Stoppable { + /// Returns whether the VMM has exited, without waiting. + fn has_exited(&self) -> io::Result; + + /// Waits until the VMM exits or `deadline` passes, and returns whether it exited. + fn wait(&self, deadline: Instant) -> io::Result; + + /// Ends the VMM at once through its verified handle, without waiting. + fn kill(&self) -> io::Result<()>; +} + +impl Stoppable for Process { + fn has_exited(&self) -> io::Result { + Process::has_exited(self) + } + + fn wait(&self, deadline: Instant) -> io::Result { + Process::wait(self, deadline) + } + + fn kill(&self) -> io::Result<()> { + Process::kill(self) + } +} + +/// Stops the VMM `process`, whose ID is `pid`, with the graceful step `graceful`, the deadline +/// `deadline`, and the forced step that `forced` requests, as [`Vmm::stop`] describes, without +/// clearing its runtime record. +fn stop_process( + process: &impl Stoppable, + pid: u32, + deadline: Instant, + forced: Option, + graceful: impl FnOnce() -> std::result::Result<(), E>, +) -> Result> { + let undetermined = |error: io::Error| { + Error::io(format!("cannot determine whether VMM process {pid} exited")).with_source(error) + }; + if process.has_exited().map_err(undetermined)? { + return Ok(Stopped { + outcome: StopOutcome::Graceful, + graceful_error: None, + }); + } + let graceful_error = graceful().err(); + let exited = match graceful_error { + None => process.wait(deadline), + Some(_) => process.has_exited(), + } + .map_err(undetermined)?; + let outcome = match forced { + _ if exited => StopOutcome::Graceful, + None => StopOutcome::TimedOut, + Some(bound) => { + process.kill().map_err(|error| { + Error::io(format!("cannot terminate VMM process {pid}")).with_source(error) + })?; + let forced_deadline = Instant::now() + bound; + if process.wait(forced_deadline).map_err(undetermined)? { + StopOutcome::Forced + } else { + StopOutcome::TimedOut + } + } + }; + Ok(Stopped { + outcome, + graceful_error, + }) +} + +/// What [`reattach`] found of the VM of a sandbox. +#[derive(Debug)] +pub enum Reattached { + /// The recorded VMM runs, and was launched with the caller's runtime identity: the caller may + /// keep driving the VM, which it now holds. + Live(Vmm), + /// The recorded VMM runs, but was launched with another runtime identity, which would drive + /// the VM through artifacts or contracts that did not start it: the caller must stop it, + /// through the [`Vmm`] that holds it by its verified identity. + Mismatched { + /// The VM, which the caller must stop. + vmm: Vmm, + /// The parts of the runtime identity that differ, as + /// [`RuntimeIdentity::differences`] returns them. + differences: Vec, + }, + /// No VMM of the sandbox runs: none was recorded, or the recorded one exited, as one that a + /// record of an earlier boot names always has. The runtime record of a VMM that exited is + /// cleared. + Exited { + /// The runtime record of the VMM that exited, which reattach cleared, if one was recorded. + record: Option, + /// How [`LockedStore::recover`] ended an interrupted launch: [`Recovery::Clean`] if none + /// was interrupted. + recovery: Recovery, + }, + /// Whether a VMM of the sandbox runs cannot be determined, for the reason that this gives: + /// an interrupted launch cannot be ended yet, or the recorded VMM cannot be examined. Nothing + /// is changed or signaled, and a later reattach may succeed. + Indeterminate(String), +} + +/// Reattaches to the VM of the sandbox of `locked` after the orchestrator restarts within the +/// same host boot, under the runtime identity `runtime`, which the caller would launch now. +/// +/// It ends an interrupted launch with [`LockedStore::recover`], reads the runtime record, holds +/// the recorded VMM with [`Process::open`], which verifies its process ID, its start time, and +/// the host boot, and compares the record's runtime identity with `runtime`, as +/// [`RuntimeRecord::check_identity`] does; see [`Reattached`]. A record from an earlier boot +/// names a VMM that has exited. +/// +/// Reattach does not replay the VM's start or its mappings, restore streams that the restart +/// lost, or resolve images: the caller's own protocol resumes through the recorded endpoint, if it +/// can. Fails with [`ErrorKind::Unsupported`](crate::ErrorKind::Unsupported) for an invalid +/// `runtime`, and as [`LockedStore::recover`] does for inconsistent records. +pub fn reattach(locked: &mut LockedStore, runtime: &RuntimeIdentity) -> Result { + reattach_with(locked, runtime, Process::open) +} + +/// Reattaches as [`reattach`] does, holding the recorded VMM with `open`, which is +/// [`Process::open`] outside tests. +fn reattach_with( + locked: &mut LockedStore, + runtime: &RuntimeIdentity, + open: impl FnOnce(&ProcessRef) -> io::Result>, +) -> Result { + runtime.check()?; + let recovery = locked.recover()?; + if let Recovery::Indeterminate(reason) = recovery { + return Ok(Reattached::Indeterminate(format!( + "an interrupted launch of the sandbox cannot be ended yet: {reason}" + ))); + } + let Some(record) = locked.runtime()? else { + return Ok(Reattached::Exited { + record: None, + recovery, + }); + }; + let pid = record.process().pid; + let process = match open(record.process()) { + Ok(Some(process)) => process, + Ok(None) => { + locked.clear_runtime()?; + return Ok(Reattached::Exited { + record: Some(record), + recovery, + }); + } + Err(error) => { + return Ok(Reattached::Indeterminate(format!( + "the recorded VMM process {pid} cannot be examined: {error}" + ))); + } + }; + let check = record.check_identity(runtime); + let vmm = Vmm { + store: SandboxStore::clone(locked), + record, + process, + }; + Ok(match check { + IdentityCheck::Matches => Reattached::Live(vmm), + IdentityCheck::Mismatched(differences) => Reattached::Mismatched { vmm, differences }, + }) +} diff --git a/nvx-rs/src/host/supervisor/tests.rs b/nvx-rs/src/host/supervisor/tests.rs new file mode 100644 index 000000000..2ef6cb877 --- /dev/null +++ b/nvx-rs/src/host/supervisor/tests.rs @@ -0,0 +1,456 @@ +//! Tests of the supervisor's stop and reattach decisions, and of launches that fail before their +//! VMM runs. Real VMMs are launched, stopped, and reattached to in `tests/supervisor.rs`. + +use std::cell::{Cell, RefCell}; +use std::fs; + +use serde_json::json; + +use super::*; +use crate::ErrorKind; +use crate::host::launch::MAX_TIMEOUT; +use crate::host::platform; +use crate::host::process::BootId; +use crate::host::store::{ArtifactIdentity, FORMAT, JOURNAL_NAME, RUNTIME_NAME}; + +/// A launch capability. +const CAPABILITY: [u8; CAPABILITY_LEN] = [7; CAPABILITY_LEN]; + +/// Returns the identity `sha256:` and 64 copies of `digit`. +fn artifact(digit: char) -> ArtifactIdentity { + ArtifactIdentity::sha256(&digit.to_string().repeat(64)).unwrap() +} + +/// Returns the runtime identity of these tests' VMs. +fn runtime() -> RuntimeIdentity { + RuntimeIdentity::new(artifact('a'), artifact('b'), artifact('c')) +} + +/// Returns a store in a fresh temporary directory. +fn store() -> (tempfile::TempDir, SandboxStore) { + let directory = tempfile::tempdir().unwrap(); + let store = SandboxStore::create(directory.path().join("sandbox")).unwrap(); + (directory, store) +} + +/// Returns the reference of this test process, which stands for a running VMM that reattach +/// holds but never signals. +fn this_process() -> ProcessRef { + let pid = std::process::id(); + ProcessRef { + pid, + start_time: platform::process_start_time(pid).unwrap().unwrap(), + boot: BootId::current().unwrap(), + } +} + +/// Writes the runtime record of a VM whose VMM is `process`, launched with `runtime`, as another +/// orchestrator would have committed it. +fn record(store: &SandboxStore, process: &ProcessRef, runtime: &RuntimeIdentity) { + let record = json!({ + "format": FORMAT, + "launch": "5d41402abc4b2a76b9719d911017c592", + "process": process, + "endpoint": "/var/lib/sandboxes/example/control.sock", + "runtime": runtime, + }); + fs::write( + store.dir().join(RUNTIME_NAME), + serde_json::to_vec(&record).unwrap(), + ) + .unwrap(); +} + +/// A VMM whose state drives a stop's decisions without a process: it exits once the graceful +/// step runs, if it heeds that step, or once it is killed, if killing ends it. +#[derive(Default)] +struct Scripted { + /// Whether the VMM has exited. + exited: Cell, + /// Whether the graceful step ends the VMM, even one that fails. + heeds_graceful: bool, + /// Whether killing the VMM ends it. + dies_when_killed: bool, + /// Whether checks of the VMM's state fail. + unreadable: bool, + /// Whether killing the VMM fails. + unkillable: bool, + /// The operations of the stop, in order. + operations: RefCell>, +} + +impl Scripted { + /// Records the operation `name`. + fn note(&self, name: &'static str) { + self.operations.borrow_mut().push(name); + } + + /// Returns whether the VMM has exited, or fails if its state is unreadable. + fn state(&self) -> io::Result { + if self.unreadable { + Err(io::Error::other("unreadable")) + } else { + Ok(self.exited.get()) + } + } + + /// Stops the VMM with `forced`, and with a graceful step that fails with `graceful_error` if + /// it is given. + fn stop( + &self, + forced: Option, + graceful_error: Option<&'static str>, + ) -> Result> { + let deadline = Instant::now() + Duration::from_secs(60); + stop_process(self, 7, deadline, forced, || { + self.note("graceful"); + if self.heeds_graceful { + self.exited.set(true); + } + graceful_error.map_or(Ok(()), Err) + }) + } + + /// Returns the operations of the stop. + fn operations(&self) -> Vec<&'static str> { + self.operations.borrow().clone() + } +} + +impl Stoppable for Scripted { + fn has_exited(&self) -> io::Result { + self.note("check"); + self.state() + } + + fn wait(&self, _deadline: Instant) -> io::Result { + self.note("wait"); + self.state() + } + + fn kill(&self) -> io::Result<()> { + self.note("kill"); + if self.unkillable { + return Err(io::Error::from(io::ErrorKind::Unsupported)); + } + if self.dies_when_killed { + self.exited.set(true); + } + Ok(()) + } +} + +#[test] +fn a_vmm_that_heeds_the_graceful_step_stops_gracefully() { + let vmm = Scripted { + heeds_graceful: true, + dies_when_killed: true, + ..Scripted::default() + }; + let stopped = vmm.stop(Some(Duration::from_secs(10)), None).unwrap(); + assert_eq!(stopped.outcome, StopOutcome::Graceful); + assert!(stopped.graceful_error.is_none()); + // The forced step that the caller requested is not taken. + assert_eq!(vmm.operations(), ["check", "graceful", "wait"]); +} + +#[test] +fn without_the_forced_step_a_stop_times_out_and_never_signals() { + let vmm = Scripted { + dies_when_killed: true, + ..Scripted::default() + }; + let stopped = vmm.stop(None, None).unwrap(); + assert_eq!(stopped.outcome, StopOutcome::TimedOut); + assert_eq!(vmm.operations(), ["check", "graceful", "wait"]); + assert!(!vmm.exited.get()); +} + +#[test] +fn a_requested_forced_step_that_ends_the_vmm_reports_forced() { + let vmm = Scripted { + dies_when_killed: true, + ..Scripted::default() + }; + let stopped = vmm.stop(Some(Duration::from_secs(10)), None).unwrap(); + assert_eq!(stopped.outcome, StopOutcome::Forced); + assert_eq!( + vmm.operations(), + ["check", "graceful", "wait", "kill", "wait"] + ); + + // A VMM that outlives the forced step's bound still runs. + let survivor = Scripted::default(); + let stopped = survivor.stop(Some(Duration::from_secs(10)), None).unwrap(); + assert_eq!(stopped.outcome, StopOutcome::TimedOut); + assert_eq!( + survivor.operations(), + ["check", "graceful", "wait", "kill", "wait"] + ); +} + +#[test] +fn a_failed_graceful_step_is_kept_and_not_waited_out() { + // The step may not have reached the guest, so only one check follows it. + let vmm = Scripted::default(); + let stopped = vmm.stop(None, Some("unreachable")).unwrap(); + assert_eq!(stopped.outcome, StopOutcome::TimedOut); + assert_eq!(stopped.graceful_error, Some("unreachable")); + assert_eq!(vmm.operations(), ["check", "graceful", "check"]); + + let forced = Scripted { + dies_when_killed: true, + ..Scripted::default() + }; + let stopped = forced + .stop(Some(Duration::from_secs(10)), Some("unreachable")) + .unwrap(); + assert_eq!(stopped.outcome, StopOutcome::Forced); + assert_eq!(stopped.graceful_error, Some("unreachable")); + assert_eq!( + forced.operations(), + ["check", "graceful", "check", "kill", "wait"] + ); + + // A VMM that exited anyway, as one that closed the caller's transport as it shut down, + // stopped without the forced step. + let exited = Scripted { + heeds_graceful: true, + dies_when_killed: true, + ..Scripted::default() + }; + let stopped = exited + .stop(Some(Duration::from_secs(10)), Some("closed")) + .unwrap(); + assert_eq!(stopped.outcome, StopOutcome::Graceful); + assert_eq!(stopped.graceful_error, Some("closed")); + assert_eq!(exited.operations(), ["check", "graceful", "check"]); +} + +#[test] +fn a_vmm_that_exited_already_skips_the_graceful_step() { + let vmm = Scripted::default(); + vmm.exited.set(true); + let stopped = vmm.stop(Some(Duration::from_secs(10)), None).unwrap(); + assert_eq!(stopped.outcome, StopOutcome::Graceful); + assert_eq!(vmm.operations(), ["check"]); +} + +#[test] +fn undetermined_states_and_failed_terminations_are_errors() { + let unreadable = Scripted { + unreadable: true, + ..Scripted::default() + }; + let error = unreadable.stop(None, None).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!( + error.message(), + "cannot determine whether VMM process 7 exited" + ); + + // A host that cannot hold the VMM cannot take the forced step. + let unkillable = Scripted { + unkillable: true, + ..Scripted::default() + }; + let error = unkillable + .stop(Some(Duration::from_secs(10)), None) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!(error.message(), "cannot terminate VMM process 7"); +} + +#[test] +fn a_sandbox_without_a_record_has_no_vm() { + let (_directory, store) = store(); + let mut locked = store.lock(None).unwrap(); + let Reattached::Exited { record, recovery } = reattach(&mut locked, &runtime()).unwrap() else { + panic!("a VM was found in an empty store"); + }; + assert!(record.is_none()); + assert_eq!(recovery, Recovery::Clean); +} + +#[test] +fn a_live_vmm_of_the_same_runtime_is_held() { + let (_directory, store) = store(); + let process = this_process(); + record(&store, &process, &runtime()); + let mut locked = store.lock(None).unwrap(); + let Reattached::Live(vmm) = reattach(&mut locked, &runtime()).unwrap() else { + panic!("the recorded VMM was not reattached"); + }; + assert_eq!(vmm.record().process(), &process); + assert_eq!(vmm.process().reference(), &process); + assert_eq!(vmm.endpoint(), "/var/lib/sandboxes/example/control.sock"); + assert!(!vmm.has_exited().unwrap()); + // The record stays. + assert_eq!(store.runtime().unwrap().as_ref(), Some(vmm.record())); +} + +#[test] +fn a_live_vmm_of_another_runtime_is_mismatched() { + let (_directory, store) = store(); + let process = this_process(); + record(&store, &process, &runtime()); + let mut current = runtime(); + current.initramfs = artifact('d'); + let mut locked = store.lock(None).unwrap(); + let Reattached::Mismatched { vmm, differences } = reattach(&mut locked, ¤t).unwrap() + else { + panic!("a VMM of another runtime was not reported as mismatched"); + }; + assert_eq!(differences, [IdentityPart::Initramfs]); + assert_eq!(vmm.process().reference(), &process); + assert!(store.runtime().unwrap().is_some()); +} + +#[test] +fn records_of_exited_vmms_are_cleared() { + let (_directory, store) = store(); + let live = this_process(); + let mut earlier_boot = *live.boot.as_bytes(); + earlier_boot[0] ^= 1; + for exited in [ + // A record of an earlier boot names a VMM that has exited. + ProcessRef { + boot: BootId::from_bytes(earlier_boot), + ..live.clone() + }, + // The recorded ID now names another process: this one. + ProcessRef { + start_time: live.start_time + 1, + ..live.clone() + }, + ] { + record(&store, &exited, &runtime()); + let mut locked = store.lock(None).unwrap(); + let Reattached::Exited { record, recovery } = reattach(&mut locked, &runtime()).unwrap() + else { + panic!("{exited:?} names a live VMM"); + }; + assert_eq!(record.unwrap().process(), &exited); + assert_eq!(recovery, Recovery::Clean); + assert!(store.runtime().unwrap().is_none()); + } +} + +#[test] +fn a_vmm_that_cannot_be_examined_is_indeterminate_and_keeps_its_record() { + let (_directory, store) = store(); + record(&store, &this_process(), &runtime()); + let mut locked = store.lock(None).unwrap(); + let hidden = |_: &ProcessRef| -> io::Result> { + Err(io::Error::new(io::ErrorKind::PermissionDenied, "hidden")) + }; + let Reattached::Indeterminate(reason) = reattach_with(&mut locked, &runtime(), hidden).unwrap() + else { + panic!("a VMM that cannot be examined was classified"); + }; + assert!(reason.ends_with("cannot be examined: hidden"), "{reason}"); + assert!(store.runtime().unwrap().is_some()); +} + +#[test] +fn an_interrupted_launch_that_cannot_be_ended_is_indeterminate() { + let (_directory, store) = store(); + // The journal of a launch whose log is gone, so that its claim cannot be checked. + let journal = json!({ + "format": FORMAT, + "launch": "5d41402abc4b2a76b9719d911017c592", + "endpoint": "/var/lib/sandboxes/example/control.sock", + "runtime": runtime(), + "log": {"identity": {"device": 2049, "index": 393_219}}, + }); + fs::write( + store.dir().join(JOURNAL_NAME), + serde_json::to_vec(&journal).unwrap(), + ) + .unwrap(); + let mut locked = store.lock(None).unwrap(); + let Reattached::Indeterminate(reason) = reattach(&mut locked, &runtime()).unwrap() else { + panic!("an interrupted launch was ended without proof"); + }; + assert!( + reason.starts_with("an interrupted launch of the sandbox cannot be ended yet: "), + "{reason}" + ); + assert!(store.journal().unwrap().is_some()); +} + +#[test] +fn reattach_refuses_an_invalid_runtime() { + let (_directory, store) = store(); + let mut locked = store.lock(None).unwrap(); + let mut invalid = runtime(); + invalid.control_contract = "with space".to_owned(); + let error = reattach(&mut locked, &invalid).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); +} + +#[test] +fn stops_check_their_store_and_forced_bound_before_anything_else() { + let (_directory, store) = store(); + let (_other_directory, other) = self::store(); + record(&store, &this_process(), &runtime()); + let mut locked = store.lock(None).unwrap(); + let Reattached::Live(vmm) = reattach(&mut locked, &runtime()).unwrap() else { + panic!("the recorded VMM was not reattached"); + }; + let never = |_: &Vmm, _: Instant| -> std::result::Result<(), ()> { + panic!("the graceful step of an invalid stop ran") + }; + // This test process stands for the VMM, and none of these stops reaches it. + let error = vmm + .stop(&mut other.lock(None).unwrap(), Instant::now(), None, never) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Other); + for bound in [Duration::ZERO, MAX_TIMEOUT + Duration::from_secs(1)] { + let error = vmm + .stop(&mut locked, Instant::now(), Some(bound), never) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported, "{bound:?}"); + } + assert!(store.runtime().unwrap().is_some()); +} + +#[test] +fn a_launch_whose_command_fails_ends_before_anything_runs() { + let (_directory, store) = store(); + let mut locked = store.lock(None).unwrap(); + let error = launch(&mut locked, &runtime(), &CAPABILITY, |endpoint| { + assert!(!endpoint.is_empty()); + Err(Error::unsupported("no command")) + }) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Unsupported); + assert_eq!(error.message(), "no command"); + // The launch ended: its journal and capability are gone, and its fresh log stays. + assert!(store.journal().unwrap().is_none()); + assert!(store.read_capability().is_err()); + assert!(store.log_path().exists()); + assert_eq!(locked.recover().unwrap(), Recovery::Clean); +} + +#[test] +fn a_launch_whose_vmm_cannot_start_ends() { + let (directory, store) = store(); + let mut locked = store.lock(None).unwrap(); + let missing = directory.path().join("missing-openvmm"); + let error = launch(&mut locked, &runtime(), &CAPABILITY, |_| { + Ok(VmmCommand::new(&missing, Vec::new(), directory.path())) + }) + .unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert_eq!( + error.message(), + format!("cannot launch {}", missing.display()) + ); + assert!(store.journal().unwrap().is_none()); + // The next launch begins at once. + let next = launch(&mut locked, &runtime(), &CAPABILITY, |_| { + Err(Error::other("the next launch began")) + }); + assert_eq!(next.unwrap_err().message(), "the next launch began"); +} diff --git a/nvx-rs/src/host/sys/process.rs b/nvx-rs/src/host/sys/process.rs index b20f57832..17bcdbc15 100644 --- a/nvx-rs/src/host/sys/process.rs +++ b/nvx-rs/src/host/sys/process.rs @@ -102,18 +102,36 @@ pub(crate) fn process_exists(pid: u32) -> io::Result { } /// Makes `command` start its process in a new session, with every descriptor of the caller -/// above standard error close-on-exec. -pub(crate) fn detach(command: &mut Command) { +/// above standard error close-on-exec, except the descriptors `inherit`, which the process +/// inherits under the same numbers. +/// +/// Only the new process clears the flag of a descriptor in `inherit`, between its fork and its +/// exec, so the descriptors stay close-on-exec in the caller, and no process that another thread +/// spawns meanwhile inherits them. The caller keeps them open until the spawn returns. +pub(crate) fn detach(command: &mut Command, inherit: Vec) { // SAFETY: the hook makes only async-signal-safe system calls and allocates nothing, as code - // that runs between fork and exec must. + // that runs between fork and exec must: `inherit` was allocated before the fork. unsafe { - command.pre_exec(|| { + command.pre_exec(move || { setsid()?; - mark_descriptors_close_on_exec() + mark_descriptors_close_on_exec()?; + inherit_descriptors(&inherit) }); } } +/// Clears the close-on-exec flag of each descriptor in `inherit`, which the caller of [`detach`] +/// keeps open across the spawn, in the new process between its fork and its exec. +fn inherit_descriptors(inherit: &[RawFd]) -> io::Result<()> { + for &descriptor in inherit { + // SAFETY: the caller of `detach` keeps the descriptor open across the spawn, and between + // fork and exec only this thread runs, which closes no descriptor while the borrow lasts. + let descriptor = unsafe { BorrowedFd::borrow_raw(descriptor) }; + fcntl(descriptor, FcntlArg::F_SETFD(FdFlag::empty()))?; + } + Ok(()) +} + /// Makes `command` mark only the descriptors that `/proc/self/fd` lists close-on-exec before it /// executes, as [`detach`] does on kernels without `CLOSE_RANGE_CLOEXEC`. #[cfg(test)] diff --git a/nvx-rs/tests/managed_lifecycle.rs b/nvx-rs/tests/managed_lifecycle.rs index ab4288a0e..73de817c5 100644 --- a/nvx-rs/tests/managed_lifecycle.rs +++ b/nvx-rs/tests/managed_lifecycle.rs @@ -339,7 +339,7 @@ fn a_managed_guest_runs_request_scoped_workloads_until_it_stops() { let log = File::create(output_dir.join("managed-lifecycle.log")).unwrap(); let capability = generate_capability().unwrap(); // OpenVMM reads its capability as soon as it starts, so its input already holds the - // capability and its end, as `nvx::host::process::spawn` prepares it. + // capability and its end, as `nvx::host::supervisor::spawn` prepares it. let (input, mut writer) = io::pipe().unwrap(); writer.write_all(&capability).unwrap(); drop(writer); diff --git a/nvx-rs/tests/supervisor.rs b/nvx-rs/tests/supervisor.rs index bb6e30990..7f1bbacd1 100644 --- a/nvx-rs/tests/supervisor.rs +++ b/nvx-rs/tests/supervisor.rs @@ -1,84 +1,256 @@ -//! Verified process references and event-driven exit waits, through the public API of -//! `nvx::host::process`, as a supervisor of VMM processes uses them. +//! Verified process references, event-driven exit waits, and the supervision of VMM processes, +//! through the public API of `nvx::host::process`, `nvx::host::supervisor`, and +//! `nvx::host::store`, as an orchestrator uses them. //! -//! The processes are this test executable, run again as one of two fixtures. A VMM fixture reads -//! the capability that `nvx::host::process::spawn` passes on its standard input, as OpenVMM does, -//! and runs until a file named `exit` appears in its working directory. A launcher fixture spawns -//! a VMM fixture, prints the VMM's reference as an orchestrator would record it, and runs until -//! its standard input closes. A launcher that the test kills, or whose job object the test -//! closes, stands for an orchestrator that crashed and restarts within the same boot: the test -//! then finds the VMM again from the recorded reference, as the restarted orchestrator would. +//! The processes are this test executable, run again as one of three fixtures. A VMM fixture +//! reads the capability that `nvx::host::supervisor::spawn` passes on its standard input, as +//! OpenVMM does, reports its descriptors on Linux, writes a line through each descriptor or handle +//! that its arguments name as inherited, and runs until a file named `exit` appears in its working +//! directory. A launcher fixture spawns a VMM fixture, prints the VMM's reference as an +//! orchestrator would record it, and runs until its standard input closes. An orchestrator fixture +//! launches a VMM fixture through the supervisor in a sandbox store, and exits without unwinding +//! after the launch step that the test names, as a crash would, or, once it has committed the +//! launch, runs until its standard input closes. A fixture that the test kills, or whose job +//! object the test closes, stands for an orchestrator that crashed and restarts within the same +//! boot: the test then finds the VMM again, as the restarted orchestrator would. #![cfg(all(feature = "host", any(target_os = "linux", windows)))] +use std::convert::Infallible; +use std::env; use std::ffi::OsString; -use std::fs::File; -use std::io::{self, BufRead, BufReader, Read, Write}; -use std::path::Path; +use std::fs::{self, File}; +use std::io::{self, BufRead, BufReader, PipeReader, PipeWriter, Read, Write}; +use std::mem::ManuallyDrop; +use std::path::{Path, PathBuf}; use std::process::{Child, ChildStdin, Command, ExitStatus, Stdio}; -use std::sync::mpsc; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::{Mutex, MutexGuard, PoisonError, mpsc}; use std::thread; use std::time::{Duration, Instant}; -use nvx::host::process::{self, BootId, Launched, Process, ProcessRef, WaitMethod}; +use nvx::host::process::{self, BootId, Process, ProcessRef, WaitMethod}; +#[cfg(windows)] +use nvx::host::store::FORMAT; +use nvx::host::store::{ + ArtifactIdentity, IdentityPart, LaunchState, LockedStore, RUNTIME_NAME, Recovery, + RuntimeIdentity, SandboxStore, +}; +use nvx::host::supervisor::{self, Inherited, Launched, Reattached, StopOutcome, Vmm, VmmCommand}; use nvx::protocol::CAPABILITY_LEN; +use serde_json::json; /// File whose presence in its working directory marks a VMM fixture. Without it, the fixture /// returns at once, as it does when `cargo test -- --ignored` runs it. const VMM_MARKER: &str = "vmm-fixture"; /// File whose appearance in its working directory ends a VMM fixture. const EXIT_MARKER: &str = "exit"; -/// Environment variable that marks a launcher fixture, and names the working directory of its -/// VMM. +/// File in which a VMM fixture reports its descriptors on Linux, a line for each: its number and +/// the object that it holds. +#[cfg(target_os = "linux")] +const DESCRIPTORS_REPORT: &str = "descriptors"; +/// Prefix of an argument of a VMM fixture that names, in decimal, a descriptor or handle that the +/// fixture inherited, and through which it writes a line. Libtest takes the argument for a test +/// name that matches no test. +const INHERITED_PREFIX: &str = "inherited="; +/// Environment variable that marks a launcher fixture, and names the working directory of the VMM +/// of a launcher or orchestrator fixture. const LAUNCHER_DIR: &str = "NVX_SUPERVISOR_TEST_VMM_DIR"; -/// Environment variable that asks a launcher fixture to start its VMM outside its job object. +/// Environment variable that asks a launcher or orchestrator fixture to start its VMM outside its +/// job object. const LAUNCHER_BREAKAWAY: &str = "NVX_SUPERVISOR_TEST_BREAKAWAY"; -/// Marker before the reference that a launcher fixture prints. Libtest's own output, the name -/// of the fixture, precedes it on its line. +/// Environment variable that marks an orchestrator fixture, and names its sandbox store. +const ORCHESTRATOR_STORE: &str = "NVX_SUPERVISOR_TEST_STORE"; +/// Environment variable that names the launch step after which an orchestrator fixture crashes: +/// `before-spawn`, `after-spawn`, or `after-identity`; or `committed`, after which it runs until +/// its standard input closes. +const ORCHESTRATOR_STEP: &str = "NVX_SUPERVISOR_TEST_STEP"; +/// Environment variable that asks an orchestrator fixture to launch the fake OpenVMM, which +/// serves its control endpoint, instead of a VMM fixture. +#[cfg(feature = "testing")] +const ORCHESTRATOR_FAKE: &str = "NVX_SUPERVISOR_TEST_FAKE"; +/// Marker before the reference that a launcher or orchestrator fixture prints. Libtest's own +/// output, the name of the fixture, precedes it on its line. const REFERENCE_PREFIX: &str = "vmm-reference "; -/// Longest wait for a process that is expected to exit, or for a launcher to answer. +/// The capability of every VMM of these tests. +const CAPABILITY: [u8; CAPABILITY_LEN] = [0x5a; CAPABILITY_LEN]; +/// Longest wait for a process that is expected to exit, or for a fixture to answer. const TIMEOUT: Duration = Duration::from_secs(60); /// Longest life of a VMM fixture that nothing ends, so that a failed test leaves no process. const VMM_LIFETIME: Duration = Duration::from_secs(180); /// A wait that a running process outlasts. const SHORT_WAIT: Duration = Duration::from_millis(100); -/// The VMM fixture: reads its capability as OpenVMM does, then runs until a file named -/// [`EXIT_MARKER`] appears in its working directory. +/// Serializes the tests that spawn processes. On Windows, a process that one test spawns while +/// another test's handles are inheritable for a spawn of its own, as the log of a launch is, +/// could hold them, and so the claim of that log, for a while. +static SPAWNS: Mutex<()> = Mutex::new(()); + +/// Takes [`SPAWNS`]. +fn serialized() -> MutexGuard<'static, ()> { + SPAWNS.lock().unwrap_or_else(PoisonError::into_inner) +} + +/// The VMM fixture: reads its capability as OpenVMM does, reports its descriptors on Linux, +/// writes a line through each descriptor or handle that an argument names as inherited, then runs +/// until a file named [`EXIT_MARKER`] appears in its working directory. #[test] #[ignore = "fixture that the supervision tests run as a VMM"] fn vmm_fixture() { - let directory = std::env::current_dir().unwrap(); + let directory = env::current_dir().unwrap(); if !directory.join(VMM_MARKER).exists() { return; } let mut capability = [0; CAPABILITY_LEN]; io::stdin().read_exact(&mut capability).unwrap(); + assert_eq!(capability, CAPABILITY); + #[cfg(target_os = "linux")] + report_descriptors(&directory); + for argument in env::args() { + if let Some(number) = argument.strip_prefix(INHERITED_PREFIX) { + // A VMM keeps what it inherits open. + let mut inherited = ManuallyDrop::new(inherited_file(number.parse().unwrap())); + writeln!(inherited, "inherited {number}").unwrap(); + } + } let started = Instant::now(); while !directory.join(EXIT_MARKER).exists() && started.elapsed() < VMM_LIFETIME { thread::sleep(Duration::from_millis(10)); } } +/// Writes the descriptors of this VMM fixture, but for the one that lists them, to the file +/// [`DESCRIPTORS_REPORT`] in `directory`, which appears whole. +#[cfg(target_os = "linux")] +fn report_descriptors(directory: &Path) { + // The descriptor that lists the others holds this directory. + let listing = PathBuf::from(format!("/proc/{}/fd", std::process::id())); + let mut descriptors: Vec<(usize, PathBuf)> = fs::read_dir("/proc/self/fd") + .unwrap() + .filter_map(|entry| { + let entry = entry.unwrap(); + let number = entry.file_name().to_str().unwrap().parse().unwrap(); + let object = fs::read_link(entry.path()).ok()?; + (object != listing).then_some((number, object)) + }) + .collect(); + descriptors.sort(); + let report: String = descriptors + .iter() + .map(|(number, object)| format!("{number} {}\n", object.display())) + .collect(); + let temporary = directory.join(format!("{DESCRIPTORS_REPORT}.tmp")); + fs::write(&temporary, report).unwrap(); + fs::rename(&temporary, directory.join(DESCRIPTORS_REPORT)).unwrap(); +} + +/// Takes the descriptor `number`, which this VMM fixture inherited. +#[cfg(target_os = "linux")] +fn inherited_file(number: usize) -> File { + use std::os::fd::FromRawFd; + + // SAFETY: the test named a descriptor that this process inherited under that number, and + // that nothing else in this process owns. + unsafe { File::from_raw_fd(i32::try_from(number).unwrap()) } +} + +/// Takes the handle `number`, which this VMM fixture inherited. +#[cfg(windows)] +fn inherited_file(number: usize) -> File { + use std::os::windows::io::{FromRawHandle, RawHandle}; + + // SAFETY: the test named a handle that this process inherited under that value, and that + // nothing else in this process owns. + unsafe { File::from_raw_handle(number as RawHandle) } +} + /// The launcher fixture: when the test writes a line to its standard input, spawns a VMM fixture /// in the directory that [`LAUNCHER_DIR`] names, prints its reference, releases it, and runs /// until its standard input closes. #[test] #[ignore = "fixture that the supervision tests run as a launcher"] fn launcher_fixture() { - let Some(directory) = std::env::var_os(LAUNCHER_DIR) else { + let Some(directory) = env::var_os(LAUNCHER_DIR) else { return; }; let mut input = io::stdin().lock(); input.read_line(&mut String::new()).unwrap(); - let breakaway = std::env::var_os(LAUNCHER_BREAKAWAY).is_some(); + let breakaway = env::var_os(LAUNCHER_BREAKAWAY).is_some(); let launched = spawn_vmm(Path::new(&directory), breakaway); - let reference = launched.process().unwrap().reference().clone(); + print_reference(launched.process().unwrap().reference()); + launched.detach(); + io::copy(&mut input, &mut io::sink()).unwrap(); +} + +/// The orchestrator fixture: when the test writes a line to its standard input, launches a VMM +/// fixture, or the fake OpenVMM, through the supervisor in the store that [`ORCHESTRATOR_STORE`] +/// names, and stops after the step that [`ORCHESTRATOR_STEP`] names. It prints the reference of +/// the VMM once it has spawned it. +#[test] +#[ignore = "fixture that the supervision tests run as an orchestrator"] +fn orchestrator_fixture() { + let Some(store) = env::var_os(ORCHESTRATOR_STORE) else { + return; + }; + let step = env::var(ORCHESTRATOR_STEP).unwrap(); + let directory = PathBuf::from(env::var_os(LAUNCHER_DIR).unwrap()); + let breakaway = env::var_os(LAUNCHER_BREAKAWAY).is_some(); + let mut input = io::stdin().lock(); + input.read_line(&mut String::new()).unwrap(); + let store = SandboxStore::open(store).unwrap(); + let mut locked = store.lock(None).unwrap(); + let command = |endpoint: &str| -> nvx::Result> { + #[cfg(feature = "testing")] + if env::var_os(ORCHESTRATOR_FAKE).is_some() { + return Ok(fake_openvmm(&directory, endpoint, &store)); + } + let _ = endpoint; + Ok(vmm_command(&directory, breakaway)) + }; + match step.as_str() { + "before-spawn" => { + let _ = supervisor::launch(&mut locked, &runtime(), &CAPABILITY, |_| crash()); + unreachable!("the orchestrator crashed before its spawn"); + } + "after-spawn" => { + // The steps of `supervisor::launch`, up to the spawn. + let mut launch = locked.begin_launch(&runtime(), &CAPABILITY).unwrap(); + let command = command(launch.endpoint()).unwrap(); + let log = launch.take_log().unwrap(); + let launched = supervisor::spawn(&command, &CAPABILITY, log).unwrap(); + print_reference(launched.process().unwrap().reference()); + crash(); + } + "after-identity" => { + let starting = + supervisor::launch(&mut locked, &runtime(), &CAPABILITY, command).unwrap(); + print_reference(starting.process().reference()); + crash(); + } + "committed" => { + let starting = + supervisor::launch(&mut locked, &runtime(), &CAPABILITY, command).unwrap(); + let vmm = starting.commit().unwrap(); + drop(locked); + print_reference(vmm.record().process()); + io::copy(&mut input, &mut io::sink()).unwrap(); + } + step => panic!("unknown launch step {step}"), + } +} + +/// Ends this process at once, once its output is flushed, without unwinding or running +/// destructors, as a crash would. +fn crash() -> ! { + io::stdout().flush().unwrap(); + std::process::exit(0) +} + +/// Prints `reference` as JSON, after [`REFERENCE_PREFIX`]. +fn print_reference(reference: &ProcessRef) { println!( "{REFERENCE_PREFIX}{}", - serde_json::to_string(&reference).unwrap() + serde_json::to_string(reference).unwrap() ); - process::detach_child(launched); - io::copy(&mut input, &mut io::sink()).unwrap(); } /// Returns the arguments that run only the fixture `name` of this test executable. @@ -97,24 +269,122 @@ fn fixture_arguments(name: &str) -> Vec { .to_vec() } +/// Returns the identity `sha256:` and 64 copies of `digit`. +fn artifact(digit: char) -> ArtifactIdentity { + ArtifactIdentity::sha256(&digit.to_string().repeat(64)).unwrap() +} + +/// Returns the runtime identity of the launches of these tests. +fn runtime() -> RuntimeIdentity { + RuntimeIdentity::new(artifact('1'), artifact('2'), artifact('3')) +} + +/// Returns a sandbox store in a fresh temporary directory, which it returns too. +fn sandbox() -> (tempfile::TempDir, SandboxStore) { + let directory = tempfile::tempdir().unwrap(); + let store = SandboxStore::create(directory.path().join("sandbox")).unwrap(); + (directory, store) +} + +/// Returns the command of a VMM fixture in `directory`, which it marks, outside the caller's job +/// object if `breakaway`. +fn vmm_command(directory: &Path, breakaway: bool) -> VmmCommand<'static> { + File::create(directory.join(VMM_MARKER)).unwrap(); + let mut command = VmmCommand::new( + env::current_exe().unwrap(), + fixture_arguments("vmm_fixture"), + directory, + ); + command.breakaway_from_job = breakaway; + command +} + +/// Returns the command of the fake OpenVMM in `directory`, which serves `endpoint` at once, and +/// writes its outcome report to that of `store`. +#[cfg(feature = "testing")] +fn fake_openvmm(directory: &Path, endpoint: &str, store: &SandboxStore) -> VmmCommand<'static> { + use nvx::host::launch::LaunchSpec; + use nvx::{Artifacts, Hypervisor}; + + fs::write(directory.join("vmlinux"), b"kernel").unwrap(); + fs::write(directory.join("initramfs.cpio.gz"), b"initrd").unwrap(); + let artifacts = Artifacts::new( + env!("CARGO_BIN_EXE_nvx-fake-openvmm"), + directory.join("vmlinux"), + directory.join("initramfs.cpio.gz"), + ); + let mut spec = LaunchSpec::new(artifacts, Hypervisor::platform_default().unwrap()); + spec.kernel_parameters = "fake_boot_delay_ms=0".to_owned(); + let arguments = spec + .openvmm_arguments(endpoint, &store.outcome_path()) + .unwrap(); + VmmCommand::new(&spec.openvmm, arguments, directory) +} + /// Spawns a VMM fixture in `directory` as OpenVMM is spawned, outside the caller's job object if /// `breakaway`. fn spawn_vmm(directory: &Path, breakaway: bool) -> Launched { - File::create(directory.join(VMM_MARKER)).unwrap(); let log = File::create(directory.join("vmm.log")).unwrap(); - process::spawn( - &std::env::current_exe().unwrap(), - breakaway, - &fixture_arguments("vmm_fixture"), - &[0x5a; CAPABILITY_LEN], - log, - directory, + supervisor::spawn(&vmm_command(directory, breakaway), &CAPABILITY, log).unwrap() +} + +/// Launches a VMM fixture in `vmm` through the supervisor in `locked`, under [`runtime`], and +/// commits the launch, as an orchestrator does once the VMM serves its control endpoint. +fn launch_vmm(locked: &mut LockedStore, vmm: &mut VmmDirectory) -> Vmm { + let starting = supervisor::launch(locked, &runtime(), &CAPABILITY, |endpoint| { + assert!(!endpoint.is_empty()); + Ok(vmm_command(vmm.path(), false)) + }) + .unwrap(); + vmm.record(starting.process().reference()); + // The launch recorded its VMM before it returned. + assert_eq!(starting.journal().state(), LaunchState::Spawned); + assert_eq!( + starting.journal().process(), + Some(starting.process().reference()) + ); + starting.commit().unwrap() +} + +/// Reattaches until the result is determinate, as a caller retries an indeterminate reattach, and +/// returns it. +/// +/// On Windows, a scanner that briefly opens the log of a launch can keep its claim held for a +/// moment. +fn reattach_until_determinate(locked: &mut LockedStore) -> Reattached { + let deadline = Instant::now() + TIMEOUT; + loop { + match supervisor::reattach(locked, &runtime()).unwrap() { + Reattached::Indeterminate(reason) if Instant::now() < deadline => { + eprintln!("reattach is indeterminate: {reason}"); + thread::sleep(SHORT_WAIT); + } + reattached => return reattached, + } + } +} + +/// Writes the runtime record of a VM whose VMM is `process` to `store`, as another orchestrator +/// would have committed it. +#[cfg(windows)] +fn write_record(store: &SandboxStore, process: &ProcessRef) { + let record = json!({ + "format": FORMAT, + "launch": "5d41402abc4b2a76b9719d911017c592", + "process": process, + "endpoint": r"\\.\pipe\openvmm-microvm-5d41402abc4b2a76b9719d911017c592", + "runtime": runtime(), + }); + fs::write( + store.dir().join(RUNTIME_NAME), + serde_json::to_vec(&record).unwrap(), ) - .unwrap() + .unwrap(); } -/// The working directory of a VMM fixture. Dropping it ends the fixture and waits for it to -/// exit, so that a failed test leaves no process behind. +/// The working directory of a VMM fixture. Dropping it ends the fixture, or a VMM that ignores +/// the exit marker, as the fake OpenVMM does, and waits for it to exit, so that a failed test +/// leaves no process behind. struct VmmDirectory { /// The reference of the VMM, once the test knows it. reference: Option, @@ -152,15 +422,17 @@ impl Drop for VmmDirectory { let _ = File::create(self.path().join(EXIT_MARKER)); if let Some(reference) = &self.reference && let Ok(Some(process)) = Process::open(reference) + && !process.wait(Instant::now() + TIMEOUT).unwrap_or(false) { + let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); } } } -/// A launcher fixture, which the test drives through its standard streams. +/// A launcher or orchestrator fixture, which the test drives through its standard streams. struct Launcher { - /// The launcher process. + /// The fixture's process. child: Child, /// Its standard input, which the test closes to end it. input: Option, @@ -170,23 +442,46 @@ impl Launcher { /// Starts a launcher whose VMM runs in `directory`, outside the launcher's job object if /// `breakaway`. fn start(directory: &Path, breakaway: bool) -> Self { - let mut command = Command::new(std::env::current_exe().unwrap()); + let mut command = Command::new(env::current_exe().unwrap()); command .args(fixture_arguments("launcher_fixture")) - .env(LAUNCHER_DIR, directory) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()); + .env(LAUNCHER_DIR, directory); if breakaway { command.env(LAUNCHER_BREAKAWAY, "1"); } + Self::spawn(command) + } + + /// Returns the command of an orchestrator that launches a VMM in `directory` through `store`, + /// and stops after `step`. + fn orchestrator(store: &SandboxStore, directory: &Path, step: &str) -> Command { + let mut command = Command::new(env::current_exe().unwrap()); + command + .args(fixture_arguments("orchestrator_fixture")) + .env(ORCHESTRATOR_STORE, store.dir()) + .env(ORCHESTRATOR_STEP, step) + .env(LAUNCHER_DIR, directory); + command + } + + /// Starts an orchestrator that launches a VMM in `directory` through `store`, and stops after + /// `step`. + fn orchestrate(store: &SandboxStore, directory: &Path, step: &str) -> Self { + Self::spawn(Self::orchestrator(store, directory, step)) + } + + /// Starts the fixture that `command` runs, with its standard input and output piped. + fn spawn(mut command: Command) -> Self { + command.stdin(Stdio::piped()).stdout(Stdio::piped()); let mut child = command.spawn().unwrap(); let input = child.stdin.take(); Self { child, input } } - /// Asks the launcher to spawn its VMM, and returns the reference that it prints, as the - /// launcher would record it: as JSON. - fn launch(&mut self) -> String { + /// Asks the fixture to start its VMM, and returns the reference that it prints, as the + /// fixture would record it: as JSON. Returns `None` if the fixture exits without printing + /// one. + fn try_launch(&mut self) -> Option { let input = self.input.as_mut().unwrap(); input.write_all(b"launch\n").unwrap(); input.flush().unwrap(); @@ -199,28 +494,39 @@ impl Launcher { Some(line.split_once(REFERENCE_PREFIX)?.1.to_owned()) }); let _ = sender.send(reference); - // Read the rest, so that the launcher can still write its own output. + // Read the rest, so that the fixture can still write its own output. lines.for_each(drop); }); receiver .recv_timeout(TIMEOUT) - .expect("the launcher printed no reference in time") - .expect("the launcher exited before it printed a reference") + .expect("the fixture printed no reference in time") + } + + /// Asks the fixture to start its VMM, and returns the reference that it prints, as the + /// fixture would record it: as JSON. + fn launch(&mut self) -> String { + self.try_launch() + .expect("the fixture exited before it printed a reference") } - /// Waits for the launcher to exit, and returns its status. + /// Asks the fixture to start its VMM, and returns the reference that it prints. + fn launch_reference(&mut self) -> ProcessRef { + serde_json::from_str(&self.launch()).unwrap() + } + + /// Waits for the fixture to exit, and returns its status. fn wait(&mut self) -> ExitStatus { let deadline = Instant::now() + TIMEOUT; loop { if let Some(status) = self.child.try_wait().unwrap() { return status; } - assert!(Instant::now() < deadline, "the launcher did not exit"); + assert!(Instant::now() < deadline, "the fixture did not exit"); thread::sleep(Duration::from_millis(10)); } } - /// Closes the launcher's input, which ends it, and waits for it to exit. + /// Closes the fixture's input, which ends it, and waits for it to exit. fn finish(&mut self) { drop(self.input.take()); assert!(self.wait().success()); @@ -241,7 +547,8 @@ fn open_running(reference: &ProcessRef) -> Process { .unwrap_or_else(|| panic!("{reference:?} does not run")) } -/// Checks that the process that `reference` names has exited, and that no process matches it. +/// Checks that the process that `process` holds has exited, and that no process matches its +/// reference. fn assert_ended(process: &Process) { assert!(process.has_exited().unwrap()); assert!(Process::open(process.reference()).unwrap().is_none()); @@ -253,10 +560,12 @@ fn processes_can_be_shared_between_threads() { fn shared() {} shared::(); shared::(); + shared::(); } #[test] fn a_child_that_exits_wakes_its_waiter() { + let _spawns = serialized(); let mut vmm = VmmDirectory::new(); let launched = spawn_vmm(vmm.path(), false); let process = launched.process().unwrap(); @@ -278,14 +587,15 @@ fn a_child_that_exits_wakes_its_waiter() { assert_ended(&process); // Ending a process that exited does nothing. process.kill().unwrap(); - assert!(process::kill_child(launched)); + assert!(launched.kill_and_wait()); } #[test] fn a_process_that_is_not_a_child_wakes_its_waiters() { + let _spawns = serialized(); let mut vmm = VmmDirectory::new(); let mut launcher = Launcher::start(vmm.path(), false); - let reference: ProcessRef = serde_json::from_str(&launcher.launch()).unwrap(); + let reference = launcher.launch_reference(); vmm.record(&reference); // The launcher exits, so its VMM is no child of this process. launcher.finish(); @@ -311,6 +621,7 @@ fn a_process_that_is_not_a_child_wakes_its_waiters() { #[test] fn a_restarted_launcher_finds_its_vmm_again_and_ends_it() { + let _spawns = serialized(); let mut vmm = VmmDirectory::new(); // On Windows, the launcher runs in a job object that terminates it when the job closes, and // its VMM breaks away from the job, as an orchestrator's VMMs do to survive it. @@ -341,6 +652,7 @@ fn a_restarted_launcher_finds_its_vmm_again_and_ends_it() { #[test] fn a_reused_process_id_is_never_signaled() { + let _spawns = serialized(); let mut vmm = VmmDirectory::new(); let launched = spawn_vmm(vmm.path(), false); let process = launched.process().unwrap(); @@ -382,11 +694,12 @@ fn a_reused_process_id_is_never_signaled() { open_running(&reference); vmm.end(); assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); - assert!(process::kill_child(launched)); + assert!(launched.kill_and_wait()); } #[test] fn references_that_name_no_process_are_never_opened() { + let _spawns = serialized(); let boot = BootId::current().unwrap(); let invalid = ProcessRef { pid: 0, @@ -412,19 +725,679 @@ fn references_that_name_no_process_are_never_opened() { let process = launched.process().unwrap(); vmm.end(); assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); - assert!(process::kill_child(launched)); + assert!(launched.kill_and_wait()); assert!(Process::open(process.reference()).unwrap().is_none()); assert!(process.has_exited().unwrap()); } +#[test] +fn a_launch_that_crashes_before_its_spawn_is_ended_on_reattach() { + let _spawns = serialized(); + let (_directory, store) = sandbox(); + let vmm = VmmDirectory::new(); + let mut orchestrator = Launcher::orchestrate(&store, vmm.path(), "before-spawn"); + assert_eq!(orchestrator.try_launch(), None); + assert!(orchestrator.wait().success()); + assert_eq!( + store.journal().unwrap().unwrap().state(), + LaunchState::Prepared + ); + assert!(!vmm.path().join(VMM_MARKER).exists()); + // The crash released the orchestrator's lock, and the claim of the log that it never passed + // on. + let mut locked = store.lock(Some(Instant::now() + TIMEOUT)).unwrap(); + match reattach_until_determinate(&mut locked) { + Reattached::Exited { + record: None, + recovery: Recovery::Exited, + } => {} + other => panic!("unexpected reattach: {other:?}"), + } + assert!(store.journal().unwrap().is_none()); + assert!(store.read_capability().is_err()); +} + +#[test] +fn an_unrecorded_vmm_that_serves_no_endpoint_is_never_signaled() { + let _spawns = serialized(); + let (_directory, store) = sandbox(); + let mut vmm = VmmDirectory::new(); + let mut orchestrator = Launcher::orchestrate(&store, vmm.path(), "after-spawn"); + let reference = orchestrator.launch_reference(); + vmm.record(&reference); + assert!(orchestrator.wait().success()); + assert_eq!( + store.journal().unwrap().unwrap().state(), + LaunchState::Prepared + ); + // The VMM, which is no child of this process, holds the claim of its launch, and serves no + // control endpoint that would name it, so reattach can neither prove that it exited nor + // verify it: it signals nothing, and keeps the journal. + let process = open_running(&reference); + let mut locked = store.lock(Some(Instant::now() + TIMEOUT)).unwrap(); + let Reattached::Indeterminate(reason) = supervisor::reattach(&mut locked, &runtime()).unwrap() + else { + panic!("reattach ended a launch whose VMM runs unverified"); + }; + assert!( + reason.contains("no process serves its control endpoint yet"), + "{reason}" + ); + assert!(!process.has_exited().unwrap()); + assert!(store.journal().unwrap().is_some()); + // Once the VMM exits, the claim that it released proves it. + vmm.end(); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + match reattach_until_determinate(&mut locked) { + Reattached::Exited { + record: None, + recovery: Recovery::Exited, + } => {} + other => panic!("unexpected reattach: {other:?}"), + } + assert!(store.journal().unwrap().is_none()); +} + +#[cfg(feature = "testing")] +#[test] +fn an_unrecorded_vmm_that_serves_its_endpoint_is_ended_on_reattach() { + let _spawns = serialized(); + let (_directory, store) = sandbox(); + let mut vmm = VmmDirectory::new(); + let mut command = Launcher::orchestrator(&store, vmm.path(), "after-spawn"); + command.env(ORCHESTRATOR_FAKE, "1"); + let mut orchestrator = Launcher::spawn(command); + let reference = orchestrator.launch_reference(); + vmm.record(&reference); + assert!(orchestrator.wait().success()); + let process = open_running(&reference); + // Once the fake OpenVMM serves its endpoint, which names it, reattach verifies that it holds + // the claimed log, and ends it. + let mut locked = store.lock(Some(Instant::now() + TIMEOUT)).unwrap(); + match reattach_until_determinate(&mut locked) { + Reattached::Exited { + record: None, + recovery: Recovery::Terminated(terminated), + } => assert_eq!(terminated, reference), + other => panic!("unexpected reattach: {other:?}"), + } + assert_ended(&process); + assert!(store.journal().unwrap().is_none()); +} + +#[test] +fn a_launch_that_crashes_before_it_commits_ends_its_recorded_vmm_on_reattach() { + let _spawns = serialized(); + let (_directory, store) = sandbox(); + let mut vmm = VmmDirectory::new(); + let mut orchestrator = Launcher::orchestrate(&store, vmm.path(), "after-identity"); + let reference = orchestrator.launch_reference(); + vmm.record(&reference); + assert!(orchestrator.wait().success()); + let journal = store.journal().unwrap().unwrap(); + assert_eq!(journal.state(), LaunchState::Spawned); + assert_eq!(journal.process(), Some(&reference)); + let process = open_running(&reference); + // The VMM is no child of this process, and its verified reference ends it. + let mut locked = store.lock(Some(Instant::now() + TIMEOUT)).unwrap(); + match supervisor::reattach(&mut locked, &runtime()).unwrap() { + Reattached::Exited { + record: None, + recovery: Recovery::Terminated(terminated), + } => assert_eq!(terminated, reference), + other => panic!("unexpected reattach: {other:?}"), + } + assert_ended(&process); + assert!(store.journal().unwrap().is_none()); + assert!(store.read_capability().is_err()); +} + +#[test] +fn a_committed_vmm_outlives_its_orchestrator_and_stops_gracefully() { + let _spawns = serialized(); + let (_directory, store) = sandbox(); + let mut vmm = VmmDirectory::new(); + let mut orchestrator = Launcher::orchestrate(&store, vmm.path(), "committed"); + let reference = orchestrator.launch_reference(); + vmm.record(&reference); + orchestrator.finish(); + + // The restarted orchestrator holds the VMM, which is no child of it, again. + let mut locked = store.lock(Some(Instant::now() + TIMEOUT)).unwrap(); + let Reattached::Live(running) = supervisor::reattach(&mut locked, &runtime()).unwrap() else { + panic!("the committed VMM was not reattached"); + }; + assert_eq!(running.record().process(), &reference); + assert_eq!(running.record().runtime(), &runtime()); + assert_eq!(running.process().wait_method(), WaitMethod::Event); + assert!(!running.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert_eq!(store.read_capability().unwrap(), CAPABILITY); + + // The graceful step asks the VMM to exit through the caller's own channel, and the stop waits + // for the VMM's exit event. + let stopped = running + .stop( + &mut locked, + Instant::now() + TIMEOUT, + None, + |stopping, _deadline| { + assert_eq!(stopping.endpoint(), running.endpoint()); + vmm.end(); + Ok::<(), Infallible>(()) + }, + ) + .unwrap(); + assert_eq!(stopped.outcome, StopOutcome::Graceful); + assert!(stopped.graceful_error.is_none()); + assert_ended(running.process()); + // The stop cleared the runtime record, and kept the log. + assert!(store.runtime().unwrap().is_none()); + assert!(store.read_capability().is_err()); + assert!(store.log_path().exists()); + match supervisor::reattach(&mut locked, &runtime()).unwrap() { + Reattached::Exited { + record: None, + recovery: Recovery::Clean, + } => {} + other => panic!("unexpected reattach: {other:?}"), + } +} + +#[test] +fn a_vmm_of_another_runtime_is_ended_only_by_a_requested_forced_step() { + let _spawns = serialized(); + let (_directory, store) = sandbox(); + let mut vmm = VmmDirectory::new(); + let mut locked = store.lock(None).unwrap(); + // The orchestrator that launched the VMM restarts. + drop(launch_vmm(&mut locked, &mut vmm)); + + // Another build of the orchestrator would launch another initramfs. + let mut upgraded = runtime(); + upgraded.initramfs = artifact('4'); + let Reattached::Mismatched { + vmm: running, + differences, + } = supervisor::reattach(&mut locked, &upgraded).unwrap() + else { + panic!("a VMM of another runtime was not reported as mismatched"); + }; + assert_eq!(differences, [IdentityPart::Initramfs]); + // The VMM heeds no graceful step of this build. Without the forced step, the stop gives up at + // its deadline, and leaves the VMM running, with its record. + let ignored = |_: &Vmm, _: Instant| Ok::<(), Infallible>(()); + let stopped = running + .stop(&mut locked, Instant::now() + SHORT_WAIT, None, ignored) + .unwrap(); + assert_eq!(stopped.outcome, StopOutcome::TimedOut); + assert!(!running.has_exited().unwrap()); + assert!(store.runtime().unwrap().is_some()); + // The forced step that the caller requests ends it through its verified handle. + let stopped = running + .stop( + &mut locked, + Instant::now() + SHORT_WAIT, + Some(TIMEOUT), + ignored, + ) + .unwrap(); + assert_eq!(stopped.outcome, StopOutcome::Forced); + assert_ended(running.process()); + assert!(store.runtime().unwrap().is_none()); +} + +#[test] +fn records_of_reused_ids_and_earlier_boots_name_vmms_that_exited() { + let _spawns = serialized(); + let (_directory, store) = sandbox(); + let mut vmm = VmmDirectory::new(); + let mut locked = store.lock(None).unwrap(); + let launched = launch_vmm(&mut locked, &mut vmm); + let reference = launched.record().process().clone(); + let path = store.dir().join(RUNTIME_NAME); + let recorded: serde_json::Value = serde_json::from_slice(&fs::read(&path).unwrap()).unwrap(); + // The recorded ID with another start time names another process, which reused the ID after + // the VMM exited, and a record of another boot names a VMM that exited with that boot. + let mut other_boot = *reference.boot.as_bytes(); + other_boot[0] ^= 1; + for (field, value) in [ + ("startTime", json!(reference.start_time + 1)), + ("boot", json!(BootId::from_bytes(other_boot))), + ] { + let mut tampered = recorded.clone(); + tampered["process"][field] = value; + fs::write(&path, serde_json::to_vec(&tampered).unwrap()).unwrap(); + let Reattached::Exited { + record: Some(record), + recovery: Recovery::Clean, + } = supervisor::reattach(&mut locked, &runtime()).unwrap() + else { + panic!("a record with another {field} named a live VMM"); + }; + assert_ne!(record.process(), &reference); + assert!(store.runtime().unwrap().is_none()); + // The process that has the ID was not signaled. + assert!(!launched.has_exited().unwrap()); + } + // The true record still names the VMM. + fs::write(&path, serde_json::to_vec(&recorded).unwrap()).unwrap(); + let Reattached::Live(running) = supervisor::reattach(&mut locked, &runtime()).unwrap() else { + panic!("the recorded VMM was not reattached"); + }; + let stopped = running + .stop( + &mut locked, + Instant::now() + TIMEOUT, + Some(TIMEOUT), + |_, _| { + vmm.end(); + Ok::<(), Infallible>(()) + }, + ) + .unwrap(); + assert_eq!(stopped.outcome, StopOutcome::Graceful); +} + +/// Sets its flag when dropped, also when its thread panics. +struct SetOnDrop<'a>(&'a AtomicBool); + +impl Drop for SetOnDrop<'_> { + fn drop(&mut self) { + self.0.store(true, Ordering::SeqCst); + } +} + +#[test] +fn vmms_inherit_only_their_streams_and_allowed_descriptors_while_others_spawn() { + let _spawns = serialized(); + /// The VMMs that the test launches. + const VMMS: usize = 6; + let (readers, writers): (Vec, Vec) = + (0..VMMS).map(|_| io::pipe().unwrap()).unzip(); + // Only the VMM that inherits one of these pipes may hold it. + #[cfg(target_os = "linux")] + let forbidden: Vec = writers + .iter() + .map(|writer| this_object(number_of(writer))) + .collect(); + #[cfg(windows)] + let forbidden: Vec = Vec::new(); + let done = AtomicBool::new(false); + let rounds = AtomicUsize::new(0); + let others = Mutex::new(Vec::new()); + let leaks = thread::scope(|scope| { + let stop_spawner = SetOnDrop(&done); + let spawner = scope.spawn(|| spawn_others(&done, &rounds, &others, &forbidden)); + // The other thread spawns throughout the launches of the VMMs. + let deadline = Instant::now() + TIMEOUT; + while rounds.load(Ordering::SeqCst) == 0 { + assert!( + Instant::now() < deadline && !spawner.is_finished(), + "the other thread spawned nothing" + ); + thread::sleep(Duration::from_millis(10)); + } + for (reader, writer) in readers.into_iter().zip(&writers) { + check_vmm(reader, writer, &others); + } + drop(stop_spawner); + spawner.join().unwrap() + }); + assert!(rounds.load(Ordering::SeqCst) > 0); + assert!(leaks.is_empty(), "{leaks:?}"); +} + +/// Spawns processes until `done`, as other threads of an orchestrator do, and counts its rounds +/// in `rounds`. Each round spawns a VMM fixture through the supervisor, which inherits a pipe of +/// its own, and a process through `std::process`, which inherits another pipe, which the round +/// makes inheritable for the spawn, as code that spawns without an allowlist does. +/// +/// It lists the numbers of those pipes in `others`, and keeps the pipes open until it returns. +/// It returns the processes that it spawned through `std::process` that held a descriptor of any +/// of the objects `forbidden`. +fn spawn_others( + done: &AtomicBool, + rounds: &AtomicUsize, + others: &Mutex>, + forbidden: &[PathBuf], +) -> Vec { + // Without the VMM marker, the fixtures that it spawns return at once. + let directory = tempfile::tempdir().unwrap(); + let mut pipes = Vec::new(); + let mut leaks = Vec::new(); + while !done.load(Ordering::SeqCst) { + let (_, allowed) = io::pipe().unwrap(); + let (_, naive) = io::pipe().unwrap(); + others + .lock() + .unwrap() + .extend([number_of(&allowed), number_of(&naive)]); + + let mut command = VmmCommand::new( + env::current_exe().unwrap(), + fixture_arguments("vmm_fixture"), + directory.path(), + ); + command.inherit.push(Inherited::new(&allowed)); + let round = rounds.load(Ordering::SeqCst); + let log = File::create(directory.path().join(format!("{round}.log"))).unwrap(); + assert!( + supervisor::spawn(&command, &CAPABILITY, log) + .unwrap() + .kill_and_wait() + ); + + set_inheritable(&naive, true); + let child = sleeper().spawn(); + set_inheritable(&naive, false); + let mut child = child.unwrap(); + leaks.extend( + held_objects(child.id(), forbidden) + .into_iter() + .map(|object| format!("process {} held {}", child.id(), object.display())), + ); + let _ = child.kill(); + child.wait().unwrap(); + pipes.extend([allowed, naive]); + rounds.fetch_add(1, Ordering::SeqCst); + } + leaks +} + +/// Spawns a VMM fixture that inherits `writer` while another thread spawns processes whose pipes +/// `others` lists, and checks that the VMM holds its standard streams and `writer`, under its +/// number, and nothing else of this process's. +fn check_vmm(reader: PipeReader, writer: &PipeWriter, others: &Mutex>) { + let mut vmm = VmmDirectory::new(); + let number = number_of(writer); + let mut command = vmm_command(vmm.path(), false); + command + .arguments + .push(format!("{INHERITED_PREFIX}{number}").into()); + command.inherit.push(Inherited::new(writer)); + let log = File::create(vmm.path().join("vmm.log")).unwrap(); + let launched = supervisor::spawn(&command, &CAPABILITY, log).unwrap(); + let process = launched.process().unwrap(); + vmm.record(process.reference()); + // The VMM writes through its pipe, under the number that its arguments name. + assert_eq!(read_line(reader), format!("inherited {number}\n")); + #[cfg(target_os = "linux")] + { + let _ = others; + let reported = reported_descriptors(vmm.path()); + let numbers: Vec = reported.iter().map(|(number, _)| *number).collect(); + assert_eq!(numbers, [0, 1, 2, number], "{reported:?}"); + let log = fs::canonicalize(vmm.path().join("vmm.log")).unwrap(); + assert!( + reported[0].1.to_string_lossy().starts_with("pipe:"), + "{reported:?}" + ); + assert_eq!(reported[1].1, log); + assert_eq!(reported[2].1, log); + assert_eq!(reported[3].1, this_object(number)); + } + #[cfg(windows)] + { + let pid = launched.id(); + assert!(holds(pid, number), "the VMM did not inherit its pipe"); + for &other in others.lock().unwrap().iter() { + assert!( + !holds(pid, other), + "the VMM inherited handle {other} of another spawn" + ); + } + } + vmm.end(); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(launched.kill_and_wait()); +} + +/// Reads a line from `reader` within [`TIMEOUT`]. +fn read_line(reader: PipeReader) -> String { + let (sender, receiver) = mpsc::channel(); + thread::spawn(move || { + let mut line = String::new(); + let _ = BufReader::new(reader).read_line(&mut line); + let _ = sender.send(line); + }); + receiver + .recv_timeout(TIMEOUT) + .expect("the VMM wrote no line in time") +} + +/// Returns the number of the descriptor of `pipe`, which a VMM inherits under that number. +#[cfg(target_os = "linux")] +fn number_of(pipe: &PipeWriter) -> usize { + use std::os::fd::AsRawFd; + + usize::try_from(pipe.as_raw_fd()).unwrap() +} + +/// Returns the value of the handle of `pipe`, which a VMM inherits under that value. +#[cfg(windows)] +fn number_of(pipe: &PipeWriter) -> usize { + use std::os::windows::io::AsRawHandle; + + pipe.as_raw_handle() as usize +} + +/// Returns the object that the descriptor `number` of this process holds, as its link in `/proc` +/// names it. +#[cfg(target_os = "linux")] +fn this_object(number: usize) -> PathBuf { + fs::read_link(format!("/proc/self/fd/{number}")).unwrap() +} + +/// Waits for the VMM fixture in `directory` to report its descriptors, and returns them, with the +/// objects that they hold. +#[cfg(target_os = "linux")] +fn reported_descriptors(directory: &Path) -> Vec<(usize, PathBuf)> { + let path = directory.join(DESCRIPTORS_REPORT); + let deadline = Instant::now() + TIMEOUT; + let report = loop { + match fs::read_to_string(&path) { + Ok(report) => break report, + Err(error) if error.kind() == io::ErrorKind::NotFound && Instant::now() < deadline => { + thread::sleep(Duration::from_millis(10)); + } + Err(error) => panic!("the VMM reported no descriptors: {error}"), + } + }; + report + .lines() + .map(|line| { + let (number, object) = line.split_once(' ').unwrap(); + (number.parse().unwrap(), PathBuf::from(object)) + }) + .collect() +} + +/// Returns the objects of `forbidden` that process `pid` holds a descriptor of. +#[cfg(target_os = "linux")] +fn held_objects(pid: u32, forbidden: &[PathBuf]) -> Vec { + fs::read_dir(format!("/proc/{pid}/fd")) + .unwrap() + .filter_map(|entry| fs::read_link(entry.unwrap().path()).ok()) + .filter(|object| forbidden.contains(object)) + .collect() +} + +/// Returns the objects of `forbidden` that process `pid` holds a handle of: none, since a process +/// that another thread creates while a VMM is created can inherit the VMM's handles on Windows, as +/// [`Inherited`] documents, so no object is forbidden there. +#[cfg(windows)] +fn held_objects(_pid: u32, forbidden: &[PathBuf]) -> Vec { + assert!(forbidden.is_empty()); + Vec::new() +} + +/// Returns whether the handle `value` of process `pid` refers to the object of this process's +/// handle `value`, as a handle that the process inherited from this one does. +#[cfg(windows)] +fn holds(pid: u32, value: usize) -> bool { + use std::os::windows::io::{FromRawHandle, OwnedHandle}; + use std::ptr; + + use windows_sys::Win32::Foundation::{ + CompareObjectHandles, DUPLICATE_SAME_ACCESS, DuplicateHandle, HANDLE, + }; + use windows_sys::Win32::System::Threading::{ + GetCurrentProcess, OpenProcess, PROCESS_DUP_HANDLE, + }; + + let ours = value as HANDLE; + // SAFETY: OpenProcess has no memory-safety preconditions. + let process = unsafe { OpenProcess(PROCESS_DUP_HANDLE, 0, pid) }; + assert!( + !process.is_null(), + "cannot open process {pid}: {}", + io::Error::last_os_error() + ); + // SAFETY: the handle is new, and nothing else owns it. + let process = unsafe { OwnedHandle::from_raw_handle(process) }; + let mut copy = ptr::null_mut(); + // SAFETY: the process handle may duplicate the process's handles, the value is looked up in + // that process only, and the output pointer is valid. + let duplicated = unsafe { + DuplicateHandle( + std::os::windows::io::AsRawHandle::as_raw_handle(&process), + ours, + GetCurrentProcess(), + &mut copy, + 0, + 0, + DUPLICATE_SAME_ACCESS, + ) + } != 0; + // A value that names no handle of the process names none of ours. + if !duplicated { + return false; + } + // SAFETY: the copy is new, and nothing else owns it. + let copy = unsafe { OwnedHandle::from_raw_handle(copy) }; + // SAFETY: both handles are open. + unsafe { + CompareObjectHandles( + std::os::windows::io::AsRawHandle::as_raw_handle(©), + ours, + ) != 0 + } +} + +/// Makes `pipe` inheritable, or not, as code that spawns without an allowlist does around its +/// spawns. +#[cfg(target_os = "linux")] +fn set_inheritable(pipe: &PipeWriter, inheritable: bool) { + use nix::fcntl::{FcntlArg, FdFlag, fcntl}; + + let flags = if inheritable { + FdFlag::empty() + } else { + FdFlag::FD_CLOEXEC + }; + fcntl(pipe, FcntlArg::F_SETFD(flags)).unwrap(); +} + +/// Makes `pipe` inheritable, or not, as code that spawns without an allowlist does around its +/// spawns. +#[cfg(windows)] +fn set_inheritable(pipe: &PipeWriter, inheritable: bool) { + use std::os::windows::io::AsRawHandle; + + use windows_sys::Win32::Foundation::{HANDLE, HANDLE_FLAG_INHERIT, SetHandleInformation}; + + let flags = if inheritable { HANDLE_FLAG_INHERIT } else { 0 }; + // SAFETY: the handle is open. + let set = + unsafe { SetHandleInformation(pipe.as_raw_handle() as HANDLE, HANDLE_FLAG_INHERIT, flags) }; + assert_ne!(set, 0, "{}", io::Error::last_os_error()); +} + +/// Returns a command that runs for a few seconds, without standard streams. +fn sleeper() -> Command { + let mut command = if cfg!(windows) { + let mut command = Command::new("ping"); + command.args(["-n", "5", "127.0.0.1"]); + command + } else { + let mut command = Command::new("sleep"); + command.arg("5"); + command + }; + command + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + command +} + +#[cfg(windows)] +#[test] +fn a_vmm_that_breaks_away_survives_its_orchestrators_job() { + let _spawns = serialized(); + let (_directory, store) = sandbox(); + let mut vmm = VmmDirectory::new(); + let job = job::Job::new(true); + let mut command = Launcher::orchestrator(&store, vmm.path(), "committed"); + command.env(LAUNCHER_BREAKAWAY, "1"); + let mut orchestrator = Launcher::spawn(command); + job.assign(&orchestrator.child); + let reference = orchestrator.launch_reference(); + vmm.record(&reference); + // The orchestrator crashes with its job, and its VMM, which broke away, survives it. + drop(job); + orchestrator.wait(); + let mut locked = store.lock(Some(Instant::now() + TIMEOUT)).unwrap(); + let Reattached::Live(running) = supervisor::reattach(&mut locked, &runtime()).unwrap() else { + panic!("the VMM that broke away was not reattached"); + }; + assert_eq!(running.record().process(), &reference); + let stopped = running + .stop(&mut locked, Instant::now() + TIMEOUT, None, |_, _| { + vmm.end(); + Ok::<(), Infallible>(()) + }) + .unwrap(); + assert_eq!(stopped.outcome, StopOutcome::Graceful); + assert!(store.runtime().unwrap().is_none()); +} + +#[cfg(windows)] +#[test] +fn a_vmm_in_its_orchestrators_job_ends_with_it() { + let _spawns = serialized(); + let (_directory, store) = sandbox(); + let mut vmm = VmmDirectory::new(); + let job = job::Job::new(false); + let mut orchestrator = Launcher::orchestrate(&store, vmm.path(), "committed"); + job.assign(&orchestrator.child); + let reference = orchestrator.launch_reference(); + vmm.record(&reference); + let process = open_running(&reference); + // The VMM did not break away, so closing the job ends it with its orchestrator. + drop(job); + orchestrator.wait(); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + let mut locked = store.lock(Some(Instant::now() + TIMEOUT)).unwrap(); + match supervisor::reattach(&mut locked, &runtime()).unwrap() { + Reattached::Exited { + record: Some(record), + recovery: Recovery::Clean, + } => assert_eq!(record.process(), &reference), + other => panic!("unexpected reattach: {other:?}"), + } + assert!(store.runtime().unwrap().is_none()); +} + #[cfg(windows)] #[test] fn a_job_that_ends_its_vmm_wakes_the_waiters() { + let _spawns = serialized(); let mut vmm = VmmDirectory::new(); let job = job::Job::new(false); let mut launcher = Launcher::start(vmm.path(), false); job.assign(&launcher.child); - let reference: ProcessRef = serde_json::from_str(&launcher.launch()).unwrap(); + let reference = launcher.launch_reference(); vmm.record(&reference); // The VMM did not break away, so the job holds it too, and closing the job ends it. let process = open_running(&reference); @@ -441,6 +1414,7 @@ fn a_job_that_ends_its_vmm_wakes_the_waiters() { fn a_process_that_this_user_may_not_query_has_an_unknown_state() { use windows_sys::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION; + let _spawns = serialized(); let (mut child, reference) = restricted::spawn(0); if restricted::may_open(&child.0, PROCESS_QUERY_LIMITED_INFORMATION) { eprintln!("skipping: this user's privileges bypass the security descriptors of processes"); @@ -461,6 +1435,22 @@ fn a_process_that_this_user_may_not_query_has_an_unknown_state() { child.0.try_wait().unwrap().is_none(), "the process was ended" ); + + // A sandbox whose runtime record names it cannot tell whether its VMM runs: reattach changes + // nothing, and signals nothing. + let (_directory, store) = sandbox(); + write_record(&store, &reference); + let mut locked = store.lock(None).unwrap(); + let Reattached::Indeterminate(reason) = supervisor::reattach(&mut locked, &runtime()).unwrap() + else { + panic!("a VMM whose identity cannot be checked was classified"); + }; + assert!(reason.contains("cannot be examined"), "{reason}"); + assert!(store.runtime().unwrap().is_some()); + assert!( + child.0.try_wait().unwrap().is_none(), + "the process was ended" + ); } #[cfg(windows)] @@ -470,6 +1460,7 @@ fn a_process_that_this_user_may_not_terminate_is_waited_for_but_never_signaled() PROCESS_QUERY_LIMITED_INFORMATION, PROCESS_SYNCHRONIZE, PROCESS_TERMINATE, }; + let _spawns = serialized(); let (mut child, reference) = restricted::spawn(PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_SYNCHRONIZE); if restricted::may_open(&child.0, PROCESS_TERMINATE) { From 8d66624d9aa50af378f38ff500effc8fcfea771c Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 20 Oct 2026 13:14:01 -0700 Subject: [PATCH 02/10] nvx-rs: serialize the Windows spawns that allow handles Two spawns that allowed the same handle could each read the other's temporary inheritability as the handle's own: one could then find the handle no longer inheritable when it created its process, and leave it inheritable for good. Spawns that allow handles now make them inheritable, create their process, and restore the handles one at a time. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03 --- nvx-rs/src/host/platform/windows.rs | 62 ++++++++++++++++++++++++++++- nvx-rs/src/host/supervisor/child.rs | 5 ++- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/nvx-rs/src/host/platform/windows.rs b/nvx-rs/src/host/platform/windows.rs index 71c02f277..5721e8246 100644 --- a/nvx-rs/src/host/platform/windows.rs +++ b/nvx-rs/src/host/platform/windows.rs @@ -5,6 +5,7 @@ use std::os::windows::ffi::OsStrExt; use std::os::windows::fs::OpenOptionsExt; use std::os::windows::io::{AsHandle, AsRawHandle, FromRawHandle, OwnedHandle, RawHandle}; use std::path::Path; +use std::sync::{Mutex, MutexGuard, PoisonError}; use std::time::{Duration, Instant}; use std::{mem, ptr}; @@ -388,11 +389,21 @@ impl Drop for AttributeList { } } +/// Serializes the spawns that make handles of their caller inheritable, from the moment that one +/// reads the inheritability of its handles until it restores it. Two spawns that allow the same +/// handle would otherwise take each other's temporary inheritability for the handle's own: one +/// could find the handle no longer inheritable when it creates its process, and fail, and then +/// leave the handle inheritable for good. +static INHERITANCE: Mutex<()> = Mutex::new(()); + /// Handles of a caller that a spawn makes inheritable for as long as it needs them, and whose /// earlier inheritability it restores when dropped. struct InheritableForSpawn { /// Each handle, and whether it was inheritable before. handles: Vec<(HANDLE, bool)>, + /// [`INHERITANCE`], held while any of the handles is inheritable for this spawn, and released + /// after their inheritability is restored. + _serialized: Option>, } impl InheritableForSpawn { @@ -402,6 +413,8 @@ impl InheritableForSpawn { fn new(handles: &[HANDLE]) -> io::Result { let mut inheritable = Self { handles: Vec::with_capacity(handles.len()), + _serialized: (!handles.is_empty()) + .then(|| INHERITANCE.lock().unwrap_or_else(PoisonError::into_inner)), }; for &handle in handles { let mut flags = 0; @@ -446,8 +459,10 @@ impl Drop for InheritableForSpawn { /// A handle in the list must be inheritable while the process is created. The `stdio` handles /// are inheritable from here until the spawn closes them, and the handles `inherit` from here /// until the spawn restores their earlier inheritability, right after the process is created. -/// A process that the caller spawns concurrently without a handle list, as `std::process` does, -/// could inherit them in that window; a process that it spawns with one could not. +/// Spawns that allow handles of their caller run that part one at a time, so two that allow the +/// same handle restore what it was before either. A process that the caller spawns concurrently +/// without a handle list, as `std::process` does, could inherit them in that window; a process +/// that it spawns with one could not. pub(crate) fn spawn_detached( program: &Path, arguments: &[OsString], @@ -1446,4 +1461,47 @@ mod tests { assert!(!other_held, "a handle that was not allowed was inherited"); assert!(!allowed_inheritable && kept_inheritable); } + + #[test] + fn concurrent_spawns_that_allow_one_handle_all_inherit_it_and_restore_it() { + /// Threads that spawn at once. + const THREADS: usize = 4; + /// Spawns of each thread. + const SPAWNS: usize = 16; + let (_shared_reader, shared) = io::pipe().unwrap(); + assert!(!inheritable(&shared)); + let system = std::env::var_os("SystemRoot").unwrap(); + let ping = Path::new(&system).join("System32").join("PING.EXE"); + // Each spawn reads the handle's inheritability while others may have made it inheritable + // for their own spawns, and must neither take that for the handle's own nor find the + // handle no longer inheritable when it creates its process. + std::thread::scope(|scope| { + for _ in 0..THREADS { + scope.spawn(|| { + let value = shared.as_raw_handle() as HANDLE; + for _ in 0..SPAWNS { + let (stdin, _stdin_writer) = io::pipe().unwrap(); + let (_stdout_reader, stdout) = io::pipe().unwrap(); + let stderr = stdout.try_clone().unwrap(); + let child = spawn_detached( + &ping, + &["-n", "30", "127.0.0.1"].map(OsString::from), + &std::env::temp_dir(), + [stdin.into(), stdout.into(), stderr.into()], + &[Inherited::new(&shared)], + false, + ) + .unwrap(); + let held = holds(&child.process, value, &shared); + assert!(child.terminate(Duration::from_secs(10))); + assert!(held, "a spawn did not inherit the allowed handle"); + } + }); + } + }); + assert!( + !inheritable(&shared), + "the spawns left the handle inheritable" + ); + } } diff --git a/nvx-rs/src/host/supervisor/child.rs b/nvx-rs/src/host/supervisor/child.rs index 8a6127e38..5fab6aaf9 100644 --- a/nvx-rs/src/host/supervisor/child.rs +++ b/nvx-rs/src/host/supervisor/child.rs @@ -50,8 +50,9 @@ struct Borrowed<'a>(std::convert::Infallible, PhantomData<&'a ()>); /// `/proc/self/fd/N`, which `proc_path` returns. /// - On Windows, the handle joins the explicit list of handles that the VMM inherits, and is /// inheritable only while the VMM is created, after which the spawn restores its earlier -/// inheritability. A process that another thread creates in that window without such a list, -/// as `std::process::Command` does, can inherit it too. +/// inheritability. Spawns that allow handles take that window one at a time, so several may +/// allow the same handle. A process that another thread creates in that window without such a +/// list, as `std::process::Command` does, can inherit it too. /// /// Other hosts fail a spawn that passes any. #[derive(Debug, Clone, Copy)] From 9f768efdef31faf262a75fd7a24e99b9c593cc3c Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 20 Oct 2026 13:14:01 -0700 Subject: [PATCH 03/10] nvx-rs: say what an indeterminate reattach may have done Reattach signals no process that it has not verified, but the recovery that it runs may have terminated a verified VMM that has not exited yet, and removes a journal that a committed runtime record supersedes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03 --- nvx-rs/src/host/supervisor/mod.rs | 26 +++++++++++++++++--------- nvx-rs/tests/supervisor.rs | 4 ++-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/nvx-rs/src/host/supervisor/mod.rs b/nvx-rs/src/host/supervisor/mod.rs index 46f287f30..413f35e51 100644 --- a/nvx-rs/src/host/supervisor/mod.rs +++ b/nvx-rs/src/host/supervisor/mod.rs @@ -92,8 +92,9 @@ //! [`Process::open`]. A record from an earlier boot names a VMM that has exited. //! //! The supervisor signals only a process that it holds through a verified identity: the child -//! that it has not released, or a [`Process`] that matched its recorded reference. It never -//! signals a process that reuses a recorded ID. +//! that it has not released, a [`Process`] that matched its recorded reference, or, during the +//! recovery of a launch that did not record its VMM, the process that recovery verified to be that +//! VMM. It never signals a process that reuses a recorded ID. mod child; #[cfg(test)] @@ -537,19 +538,26 @@ pub enum Reattached { recovery: Recovery, }, /// Whether a VMM of the sandbox runs cannot be determined, for the reason that this gives: - /// an interrupted launch cannot be ended yet, or the recorded VMM cannot be examined. Nothing - /// is changed or signaled, and a later reattach may succeed. + /// an interrupted launch cannot be ended yet, or the recorded VMM cannot be examined. The + /// runtime record, and the journal of an interrupted launch, remain for a later reattach, + /// which may succeed. + /// + /// Reattach signals no process that it has not verified, but it is not free of effects: + /// [`LockedStore::recover`] may have terminated a verified VMM of the interrupted launch that + /// has not exited yet, and removes a journal that the runtime record of its committed launch + /// supersedes. Indeterminate(String), } /// Reattaches to the VM of the sandbox of `locked` after the orchestrator restarts within the /// same host boot, under the runtime identity `runtime`, which the caller would launch now. /// -/// It ends an interrupted launch with [`LockedStore::recover`], reads the runtime record, holds -/// the recorded VMM with [`Process::open`], which verifies its process ID, its start time, and -/// the host boot, and compares the record's runtime identity with `runtime`, as -/// [`RuntimeRecord::check_identity`] does; see [`Reattached`]. A record from an earlier boot -/// names a VMM that has exited. +/// It ends an interrupted launch with [`LockedStore::recover`], which signals only a VMM that it +/// verified, reads the runtime record, holds the recorded VMM with [`Process::open`], which +/// verifies its process ID, its start time, and the host boot, and compares the record's runtime +/// identity with `runtime`, as [`RuntimeRecord::check_identity`] does; see [`Reattached`]. A +/// record from an earlier boot names a VMM that has exited. Beyond that recovery, reattach +/// signals nothing: a VM that it reports as live or mismatched keeps running. /// /// Reattach does not replay the VM's start or its mappings, restore streams that the restart /// lost, or resolve images: the caller's own protocol resumes through the recorded endpoint, if it diff --git a/nvx-rs/tests/supervisor.rs b/nvx-rs/tests/supervisor.rs index 7f1bbacd1..2cd1244b9 100644 --- a/nvx-rs/tests/supervisor.rs +++ b/nvx-rs/tests/supervisor.rs @@ -1436,8 +1436,8 @@ fn a_process_that_this_user_may_not_query_has_an_unknown_state() { "the process was ended" ); - // A sandbox whose runtime record names it cannot tell whether its VMM runs: reattach changes - // nothing, and signals nothing. + // A sandbox whose runtime record names it cannot tell whether its VMM runs: reattach keeps + // the record, and signals nothing. let (_directory, store) = sandbox(); write_record(&store, &reference); let mut locked = store.lock(None).unwrap(); From 99299bf86c5cc8cf1212ea4926595af1cf07c88a Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 20 Oct 2026 13:29:01 -0700 Subject: [PATCH 04/10] nvx-rs: refuse inheritable descriptors in the VMM allowlist on Linux A descriptor that is not close-on-exec in the caller reaches every process that the caller spawns, not only the VMM, so a spawn that allows one could not keep the allowlist's promise that no other spawn inherits it. The spawn now refuses such a descriptor, before anything runs, and leaves its flag as it was. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03 --- nvx-rs/README.md | 2 +- nvx-rs/src/host/platform/linux.rs | 15 ++++++++++++++- nvx-rs/src/host/supervisor/child.rs | 30 ++++++++++++++++++++++------- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 6d83c2a36..9706d6468 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -62,7 +62,7 @@ nvx-rs/ | `nvx::host::instance` | `Instance`: a VM in a directory of its own. It starts OpenVMM with a fresh capability and waits until the guest is ready and has mounted the mapped paths, reconciles recorded state, recovers interrupted launches, runs workloads and pumps their output, and stops the VM with a forced fallback | | `nvx::host::nvxs` | `HostConnection`: the host's role of the NVXS session over a platform transport, on which `nvx::host::session` and other host applications run their protocols. It attaches with the launch capability, waiting while the broker asks it to, sends application bytes in as many DATA records as they take, and returns each DATA payload from the guest, with deadlines; a receive that times out keeps a partial record | | `nvx::host::process` | Verified process references, and the launch of OpenVMM for the CLI and `nvx::host::instance`, through `nvx::host::supervisor`. A `ProcessRef` names one process by its ID, its start time, and the `BootId` of the host boot during which it started. `Process::open` checks a reference and holds its process through a pidfd on Linux or a process handle on Windows, whether or not the caller started it, so that waits sleep until the process exits and termination never reaches a process that reuses its ID. A reference from an earlier boot names an exited process, and a process whose identity this user cannot check, such as another user's, makes `Process::open` fail rather than report an exit. Linux hosts without pidfds poll the reference instead, and cannot terminate the process | -| `nvx::host::supervisor` | The supervisor of a sandbox's VMM, beneath the CLI, `Instance`, and external orchestrators, which keep the configuration, the authorization, and the application protocol. `launch` runs the store's launch steps under the sandbox's lock: it writes the journal's marker, spawns OpenVMM with the claimed log, records its `ProcessRef` right after, and returns a `Starting` launch that the caller commits once the VMM serves its control endpoint, or abandons. A launch that fails never leaves an unrecorded VMM running. `VmmCommand::inherit` lists the descriptors (Linux) or handles (Windows) that the VMM inherits under their numbers, besides its standard streams, and it inherits nothing else of the caller's: on Linux, only the VMM's own process clears their close-on-exec flag, and on Windows, they join the VMM's explicit handle list. `Vmm::stop` runs one graceful step of the caller's and waits for the VMM's exit event until a deadline, and takes a bounded forced step through the VMM's verified identity only on request. `reattach` recovers an interrupted launch, and holds the recorded VMM again after the orchestrator restarts within the same host boot: `Live`, `Mismatched` with another runtime identity, `Exited`, or `Indeterminate` | +| `nvx::host::supervisor` | The supervisor of a sandbox's VMM, beneath the CLI, `Instance`, and external orchestrators, which keep the configuration, the authorization, and the application protocol. `launch` runs the store's launch steps under the sandbox's lock: it writes the journal's marker, spawns OpenVMM with the claimed log, records its `ProcessRef` right after, and returns a `Starting` launch that the caller commits once the VMM serves its control endpoint, or abandons. A launch that fails never leaves an unrecorded VMM running. `VmmCommand::inherit` lists the descriptors (Linux) or handles (Windows) that the VMM inherits under their numbers, besides its standard streams, and it inherits nothing else of the caller's: on Linux, they must be close-on-exec, a flag that only the VMM's own process clears; on Windows, they join the VMM's explicit handle list. `Vmm::stop` runs one graceful step of the caller's and waits for the VMM's exit event until a deadline, and takes a bounded forced step through the VMM's verified identity only on request. `reattach` recovers an interrupted launch, and holds the recorded VMM again after the orchestrator restarts within the same host boot: `Live`, `Mismatched` with another runtime identity, `Exited`, or `Indeterminate` | | `nvx::host::session`, `platform`, `report` | The authenticated control session, which carries NVX's control protocol, `NVXC`, one frame per DATA record of a `HostConnection`; Linux and Windows primitives; and OpenVMM's outcome report | | `nvx::host::sandbox` | The launch profile of `nvx.py sandbox`, ported from the one that `scripts/nvx_tools/sandbox.py` held: `SandboxLaunch`, with the EROFS distro `SandboxLayer`, the one layer that a sandbox takes, the ext4 scratch disk, the entrypoint, arguments, hostname, workload identity, and resource limits, and its `--microvm-sandbox-block` and `--microvm-workload-identity` arguments and kernel command line, within the 1024-byte budget that OpenVMM leaves it. `mount`: `SandboxMount` live shares of host directories and files, their guest targets and the reserved ones, their access policies and owner, the aggregate of several shares, and the checks of their host paths. `options`: the checks that `nvx.py sandbox` and `nvx.py run` apply to their options: the attribution of `--mount-deny`, `--mount-allow`, and `--mount-write` to shares, the systemd policy, the targets of `nvx.py run` shares, and the managed exec environment; see [Python parity](#python-parity) | | `nvx::testing::fake_openvmm` | A fake OpenVMM that serves the control console and runs scripted workloads without a VM, for direct launches and for the managed and one-shot sandboxes of `nvx-rs sandbox` and `nvx.py sandbox` | diff --git a/nvx-rs/src/host/platform/linux.rs b/nvx-rs/src/host/platform/linux.rs index 1ca3fff0d..a64494010 100644 --- a/nvx-rs/src/host/platform/linux.rs +++ b/nvx-rs/src/host/platform/linux.rs @@ -248,8 +248,11 @@ pub(crate) fn boot_id() -> io::Result<[u8; 16]> { /// Only the new process clears the close-on-exec flag of the descriptors in `inherit`, so no /// process that another thread spawns meanwhile inherits them. Fails with /// [`io::ErrorKind::InvalidInput`] for a descriptor of a standard stream, 0 to 2, which the -/// command's own standard streams replace in the new process. +/// command's own standard streams replace in the new process, and for a descriptor that is not +/// close-on-exec in the caller, which any process that the caller spawns would inherit. pub(crate) fn detach(command: &mut Command, inherit: &[Inherited<'_>]) -> io::Result<()> { + use nix::fcntl::{FcntlArg, FdFlag, fcntl}; + let mut descriptors: Vec = Vec::with_capacity(inherit.len()); for inherited in inherit { let descriptor = inherited.as_fd().as_raw_fd(); @@ -262,6 +265,16 @@ pub(crate) fn detach(command: &mut Command, inherit: &[Inherited<'_>]) -> io::Re ), )); } + let flags = FdFlag::from_bits_retain(fcntl(inherited.as_fd(), FcntlArg::F_GETFD)?); + if !flags.contains(FdFlag::FD_CLOEXEC) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "descriptor {descriptor} cannot be inherited by the VMM alone: it is not \ + close-on-exec, so any process that this process spawns inherits it" + ), + )); + } if !descriptors.contains(&descriptor) { descriptors.push(descriptor); } diff --git a/nvx-rs/src/host/supervisor/child.rs b/nvx-rs/src/host/supervisor/child.rs index 5fab6aaf9..cdc60f20c 100644 --- a/nvx-rs/src/host/supervisor/child.rs +++ b/nvx-rs/src/host/supervisor/child.rs @@ -44,10 +44,11 @@ struct Borrowed<'a>(std::convert::Infallible, PhantomData<&'a ()>); /// /// [`spawn`] passes the VMM these and its standard streams, and nothing else of the caller's: /// -/// - On Linux, the descriptor stays close-on-exec in the caller, as Rust opens its files, and -/// only the VMM's own process clears the flag, between its fork and its exec, so no process -/// that another thread spawns meanwhile inherits it. The VMM opens it again as -/// `/proc/self/fd/N`, which `proc_path` returns. +/// - On Linux, the descriptor must be close-on-exec in the caller, as Rust opens its files, and +/// stays so: only the VMM's own process clears the flag, between its fork and its exec, so no +/// process that another thread spawns meanwhile inherits it. A spawn refuses a descriptor that +/// is not close-on-exec, since any process that the caller spawns would inherit it. The VMM +/// opens it again as `/proc/self/fd/N`, which `proc_path` returns. /// - On Windows, the handle joins the explicit list of handles that the VMM inherits, and is /// inheritable only while the VMM is created, after which the spawn restores its earlier /// inheritability. Spawns that allow handles take that window one at a time, so several may @@ -162,9 +163,10 @@ impl VmmCommand<'_> { /// /// Record the VMM's identity right after the spawn, with /// [`Launch::record_process`](crate::host::store::Launch::record_process), as -/// [`launch`](super::launch) does. Fails with [`io::ErrorKind::InvalidInput`] for a descriptor of -/// a standard stream, 0 to 2, on Linux, and with [`io::ErrorKind::Unsupported`] for any descriptor -/// or handle on hosts other than Linux and Windows. +/// [`launch`](super::launch) does. Fails with [`io::ErrorKind::InvalidInput`] on Linux for a +/// descriptor of a standard stream, 0 to 2, or one that is not close-on-exec, and with +/// [`io::ErrorKind::Unsupported`] for any descriptor or handle on hosts other than Linux and +/// Windows. pub fn spawn( command: &VmmCommand<'_>, capability: &[u8; CAPABILITY_LEN], @@ -534,4 +536,18 @@ mod tests { assert_eq!(error.kind(), io::ErrorKind::InvalidInput); } } + + #[test] + fn descriptors_that_are_not_close_on_exec_are_refused() { + let directory = tempfile::tempdir().unwrap(); + // As a descriptor that the caller's parent or native code left without close-on-exec, + // which any process that the caller spawns would inherit, and not only the VMM. + let (_reader, writer) = io::pipe().unwrap(); + sys::clear_close_on_exec(&writer).unwrap(); + let error = spawn_sleeper(directory.path(), vec![Inherited::new(&writer)]).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + assert!(error.to_string().contains("close-on-exec"), "{error}"); + // The refusal, which comes before the spawn, leaves the descriptor as it was. + assert!(!close_on_exec(writer.as_fd())); + } } From 8bf8b9a1bab28d9c10c433853581c08850686508 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 20 Oct 2026 19:57:16 -0700 Subject: [PATCH 05/10] nvx-rs: wait briefly for a held log claim before recovery gives up On Linux, a process holds a copy of each descriptor of its parent from its fork until it executes its program. A spawn by another thread of a process that holds a launch's log, such as the launch of another sandbox's VMM, therefore holds that launch's claim on its log, and any sandbox lock that the process holds, for that moment. Recovery, and the abandon of a failed launch, then reported a launch that nothing of it held anymore as indeterminate, and kept its journal. Recovery now checks a held claim again for up to a second before it concludes that a process of the launch may still run. The store's tests that take a released lock with a deadline that has passed, or check a released claim directly, retry for a bounded time too. The supervisor's spawning tests run next to the store's tests, which made this race fail the library tests on CI, and in every one of 20 runs on a 24-thread host. With this change, 50 runs pass, 30 of them with 48 test threads. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03 --- nvx-rs/src/host/store/mod.rs | 39 ++++++++++++++++++++++++++++------ nvx-rs/src/host/store/tests.rs | 28 ++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/nvx-rs/src/host/store/mod.rs b/nvx-rs/src/host/store/mod.rs index ceb8c9b89..071061f0e 100644 --- a/nvx-rs/src/host/store/mod.rs +++ b/nvx-rs/src/host/store/mod.rs @@ -111,6 +111,11 @@ //! endpoint. The claim relies on the VMM keeping its standard output and error until it exits, as //! OpenVMM does, and on the directory being on a local file system. //! +//! On Linux, a process that another thread spawns while the log is open, such as another +//! sandbox's VMM, also holds the claim, from its fork until it executes its program, which closes +//! its copy. Recovery therefore checks a held claim again for up to a second before it concludes +//! that a process of the launch may still run. +//! //! # Recovery //! //! A caller that dies during a launch leaves its journal, and [`LockedStore::recover`] ends the @@ -204,6 +209,17 @@ pub const FILE_NAMES: [&str; 8] = [ /// Interval at which a lock with a deadline is tried again. const LOCK_RETRY_INTERVAL: Duration = Duration::from_millis(25); +/// How long recovery waits for a held claim on a launch's log to be released before it concludes +/// that a process of the launch may still run. +/// +/// On Linux, a process holds a copy of each descriptor of its parent from its fork until it +/// executes its program, so a spawn by another thread of the process that holds the log, such +/// as the launch of another sandbox's VMM, holds the claim for that moment. +const CLAIM_RELEASE_WAIT: Duration = Duration::from_secs(1); + +/// Interval at which a held claim is checked again while recovery waits for its release. +const CLAIM_RETRY_INTERVAL: Duration = Duration::from_millis(10); + /// Returns `dir` as an absolute path, which a relative one names from the current directory. fn absolute(dir: PathBuf) -> Result { std::path::absolute(&dir).map_err(io_error(format!( @@ -522,15 +538,26 @@ impl LockedStore { /// Returns whether a process still holds the claim of the launch of `journal` on its log, or /// why that cannot be determined. + /// + /// A held claim is checked again until [`CLAIM_RELEASE_WAIT`] passes, so that a process that + /// another thread spawned while the log was open, which holds the claim only until it + /// executes its program, is not taken for a process of the launch. fn claim_held(&self, journal: &LaunchJournal) -> std::result::Result { let path = self.log_path(); let identity = journal.log().identity; - platform::log_claim_held(&path, (identity.device, identity.index)).map_err(|error| { - format!( - "its claim on the log {} cannot be checked: {error}", - path.display() - ) - }) + let deadline = Instant::now() + CLAIM_RELEASE_WAIT; + loop { + match platform::log_claim_held(&path, (identity.device, identity.index)) { + Ok(true) if Instant::now() < deadline => thread::sleep(CLAIM_RETRY_INTERVAL), + Ok(held) => return Ok(held), + Err(error) => { + return Err(format!( + "its claim on the log {} cannot be checked: {error}", + path.display() + )); + } + } + } } /// Ends a launch through the recorded `reference` to its VMM, which `open`, [`Process::open`] diff --git a/nvx-rs/src/host/store/tests.rs b/nvx-rs/src/host/store/tests.rs index 6dd0d2564..455848d1c 100644 --- a/nvx-rs/src/host/store/tests.rs +++ b/nvx-rs/src/host/store/tests.rs @@ -44,6 +44,25 @@ fn write(store: &SandboxStore, name: &str, value: &serde_json::Value) { fs::write(store.path(name), serde_json::to_vec(value).unwrap()).unwrap(); } +/// How long a test waits for a lock or claim that it released to be free. +/// +/// A process that a concurrent test spawns holds a copy of each descriptor of this process from +/// its fork until it executes its program, and with it the lock or claim of a file that a test +/// had open, for that moment. +const RELEASE_WAIT: Duration = Duration::from_secs(10); + +/// Returns whether `free` holds before [`RELEASE_WAIT`] passes, checking it again until then. +fn released(mut free: impl FnMut() -> bool) -> bool { + let deadline = Instant::now() + RELEASE_WAIT; + while !free() { + if Instant::now() >= deadline { + return false; + } + thread::sleep(Duration::from_millis(10)); + } + true +} + #[test] fn artifact_identities_are_printable_tokens() { assert_eq!( @@ -291,7 +310,11 @@ fn the_lock_excludes_other_holders() { // A deadline that has passed still tries once. assert!(store.lock(Some(Instant::now())).is_err()); drop(held); - drop(store.lock(Some(Instant::now())).unwrap()); + assert!(released(|| match store.lock(Some(Instant::now())) { + Ok(_) => true, + Err(error) if error.kind() == ErrorKind::TimedOut => false, + Err(error) => panic!("{error}"), + })); // A waiter without a deadline takes the lock once its holder releases it. let held = store.lock(None).unwrap(); @@ -558,7 +581,8 @@ fn a_launch_writes_its_marker_before_its_vmm_runs() { drop(log); assert!(platform::log_claim_held(&store.log_path(), identity).unwrap()); drop(copy); - assert!(!platform::log_claim_held(&store.log_path(), identity).unwrap()); + let free = || !platform::log_claim_held(&store.log_path(), identity).unwrap(); + assert!(released(free)); // A log that is not the claimed file proves nothing. assert!(platform::log_claim_held(&store.log_path(), previous_log).is_err()); From c7b7c943ddf8292ded4b4c07f0ab4960aa246b14 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Tue, 20 Oct 2026 21:06:29 -0700 Subject: [PATCH 06/10] nvx-rs: let the instance lock test wait for a released lock The instance lock test took its lock again with a single try right after the holder released it. On Linux, a process that a concurrent test forks while the holder has the lock holds a copy of it until the process executes its program, so that try could still find the lock held. The supervisor's spawning tests run in the same test binary. The test now retries a lock that would block for up to ten seconds, as the store's tests do, and still fails at once on any other error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03 --- nvx-rs/src/host/instance.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/nvx-rs/src/host/instance.rs b/nvx-rs/src/host/instance.rs index 115350e2b..45fdb9d34 100644 --- a/nvx-rs/src/host/instance.rs +++ b/nvx-rs/src/host/instance.rs @@ -1137,7 +1137,18 @@ mod tests { let contender = OpenOptions::new().write(true).open(&path).unwrap(); assert!(contender.try_lock().is_err()); drop(guard); - contender.try_lock().unwrap(); + // A process that a concurrent test forked while the guard held the lock holds it until + // the process executes its program, so the released lock may take a moment to be free. + let deadline = Instant::now() + Duration::from_secs(10); + loop { + match contender.try_lock() { + Ok(()) => break, + Err(std::fs::TryLockError::WouldBlock) if Instant::now() < deadline => { + thread::sleep(Duration::from_millis(10)); + } + Err(error) => panic!("the released lock is not free: {error}"), + } + } drop(contender); // A waiter takes the lock once the holder releases it. let guard = lock(&path).unwrap(); From 3c9deeee32949e10e55df61172ad434d34b00aec Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Wed, 21 Oct 2026 09:39:50 -0700 Subject: [PATCH 07/10] nvx-rs: name the last standard stream descriptor The VMM allowlist refused descriptors 0 to 2, and the hook of a detached spawn marked the descriptors from 3 up close-on-exec, each through a bare number. A documented constant now names that boundary and says why it is there: a spawn gives the new process its own standard streams under those numbers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03 --- nvx-rs/src/host/platform/linux.rs | 5 +++-- nvx-rs/src/host/sys/mod.rs | 2 +- nvx-rs/src/host/sys/process.rs | 12 ++++++++++-- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/nvx-rs/src/host/platform/linux.rs b/nvx-rs/src/host/platform/linux.rs index a64494010..4244bec50 100644 --- a/nvx-rs/src/host/platform/linux.rs +++ b/nvx-rs/src/host/platform/linux.rs @@ -256,12 +256,13 @@ pub(crate) fn detach(command: &mut Command, inherit: &[Inherited<'_>]) -> io::Re let mut descriptors: Vec = Vec::with_capacity(inherit.len()); for inherited in inherit { let descriptor = inherited.as_fd().as_raw_fd(); - if descriptor <= 2 { + if descriptor <= sys::LAST_STANDARD_STREAM { return Err(io::Error::new( io::ErrorKind::InvalidInput, format!( "descriptor {descriptor} cannot be inherited: the VMM's own standard streams \ - replace descriptors 0 to 2" + replace descriptors 0 to {}", + sys::LAST_STANDARD_STREAM ), )); } diff --git a/nvx-rs/src/host/sys/mod.rs b/nvx-rs/src/host/sys/mod.rs index 601639f00..840135412 100644 --- a/nvx-rs/src/host/sys/mod.rs +++ b/nvx-rs/src/host/sys/mod.rs @@ -21,7 +21,7 @@ use nix::unistd::{getegid, geteuid}; #[cfg(feature = "cli")] pub(crate) use interrupt::{interrupted, watch_interrupts, write_interruptibly}; #[cfg(target_os = "linux")] -pub(crate) use process::{PidFd, detach, process_exists}; +pub(crate) use process::{LAST_STANDARD_STREAM, PidFd, detach, process_exists}; #[cfg(all(test, target_os = "linux"))] pub(crate) use process::{clear_close_on_exec, mark_listed_descriptors_on_spawn}; #[cfg(target_os = "linux")] diff --git a/nvx-rs/src/host/sys/process.rs b/nvx-rs/src/host/sys/process.rs index 17bcdbc15..c9d79afee 100644 --- a/nvx-rs/src/host/sys/process.rs +++ b/nvx-rs/src/host/sys/process.rs @@ -16,6 +16,14 @@ use nix::unistd::{Pid, setsid}; use super::Errno; +/// The highest descriptor of a standard stream: standard error. +/// +/// Standard input, output, and error are descriptors 0, 1, and 2 in every process, and a spawn +/// gives the new process its own standard streams under those numbers. A descriptor of the +/// caller's at or below this one therefore cannot reach the new process under its number, and +/// the descriptors that a detached spawn marks close-on-exec are the ones above it. +pub(crate) const LAST_STANDARD_STREAM: RawFd = libc::STDERR_FILENO; + /// A pidfd: an owned descriptor that refers to one process. Signals sent through it reach that /// process or fail, even once another process reuses its ID. pub(crate) struct PidFd(OwnedFd); @@ -156,7 +164,7 @@ pub(crate) fn clear_close_on_exec(descriptor: impl AsFd) -> io::Result<()> { /// pipe writer it holds, for example, keeps the reader from seeing end-of-file. Marking rather /// than closing keeps the descriptor through which `Command::spawn` reports a failed exec. fn mark_descriptors_close_on_exec() -> io::Result<()> { - let first: libc::c_uint = 3; + let first: libc::c_uint = LAST_STANDARD_STREAM.unsigned_abs() + 1; // SAFETY: close_range takes two descriptor bounds and flags and changes no memory. let marked = unsafe { libc::syscall( @@ -221,7 +229,7 @@ fn mark_directory_entries(directory: &OwnedFd) -> io::Result<()> { let (record, rest) = records.split_at_checked(length).ok_or_else(malformed)?; let name = record.get(19..).ok_or_else(malformed)?; if let Some(descriptor) = descriptor_number(name) - && descriptor > 2 + && descriptor > LAST_STANDARD_STREAM && descriptor != directory.as_raw_fd() { // SAFETY: the listing shows the descriptor open, and between fork and exec only From 4fcbeb4be9a05b4be1561e3bc8e274bc961bc8ea Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Wed, 21 Oct 2026 09:40:34 -0700 Subject: [PATCH 08/10] nvx-rs: recheck a held log claim at the store's lock retry interval Recovery checked a held claim on a launch's log again every 10 ms, an interval of its own. It now waits at the 25 ms interval at which the store already retries its lock, the interval of the crate's process and lock polls, and no longer sleeps past its deadline. The documentation of the one-second wait now says why it lasts that long, and which operations can wait. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03 --- nvx-rs/src/host/store/mod.rs | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/nvx-rs/src/host/store/mod.rs b/nvx-rs/src/host/store/mod.rs index 071061f0e..cf3ae3892 100644 --- a/nvx-rs/src/host/store/mod.rs +++ b/nvx-rs/src/host/store/mod.rs @@ -206,20 +206,25 @@ pub const FILE_NAMES: [&str; 8] = [ OUTCOME_NAME, ]; -/// Interval at which a lock with a deadline is tried again. +/// Interval at which a lock that another holder has is tried again: the sandbox's lock, by a +/// caller that waits for it until a deadline, and a launch's claim on its log, by a recovery that +/// waits for the claim's release. A release is noticed at most this late. const LOCK_RETRY_INTERVAL: Duration = Duration::from_millis(25); /// How long recovery waits for a held claim on a launch's log to be released before it concludes /// that a process of the launch may still run. /// /// On Linux, a process holds a copy of each descriptor of its parent from its fork until it -/// executes its program, so a spawn by another thread of the process that holds the log, such -/// as the launch of another sandbox's VMM, holds the claim for that moment. +/// executes its program. A spawn by another thread of the process that holds the log, such as +/// the launch of another sandbox's VMM, therefore holds the claim for that moment, which lasted +/// about 5 ms on a busy host; a second leaves a wide margin for a more heavily loaded one. +/// +/// Recovery, and the abandon of a failed launch, wait only while they find the claim held: a +/// launch that succeeds and a stop never check it. A claim that stays held is still reported held +/// when the wait ends, so the wait can delay them by up to a second, but never makes them end a +/// launch that may still run. const CLAIM_RELEASE_WAIT: Duration = Duration::from_secs(1); -/// Interval at which a held claim is checked again while recovery waits for its release. -const CLAIM_RETRY_INTERVAL: Duration = Duration::from_millis(10); - /// Returns `dir` as an absolute path, which a relative one names from the current directory. fn absolute(dir: PathBuf) -> Result { std::path::absolute(&dir).map_err(io_error(format!( @@ -539,17 +544,23 @@ impl LockedStore { /// Returns whether a process still holds the claim of the launch of `journal` on its log, or /// why that cannot be determined. /// - /// A held claim is checked again until [`CLAIM_RELEASE_WAIT`] passes, so that a process that - /// another thread spawned while the log was open, which holds the claim only until it - /// executes its program, is not taken for a process of the launch. + /// A held claim is checked again at every [`LOCK_RETRY_INTERVAL`] until [`CLAIM_RELEASE_WAIT`] + /// passes, so that a process that another thread spawned while the log was open, which holds + /// the claim only until it executes its program, is not taken for a process of the launch. fn claim_held(&self, journal: &LaunchJournal) -> std::result::Result { let path = self.log_path(); let identity = journal.log().identity; let deadline = Instant::now() + CLAIM_RELEASE_WAIT; loop { match platform::log_claim_held(&path, (identity.device, identity.index)) { - Ok(true) if Instant::now() < deadline => thread::sleep(CLAIM_RETRY_INTERVAL), - Ok(held) => return Ok(held), + Ok(true) => { + let left = deadline.saturating_duration_since(Instant::now()); + if left.is_zero() { + return Ok(true); + } + thread::sleep(left.min(LOCK_RETRY_INTERVAL)); + } + Ok(false) => return Ok(false), Err(error) => { return Err(format!( "its claim on the log {} cannot be checked: {error}", From 44d05a8364fee38ecf2c3fc2ed1bb35545b0cbd3 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Wed, 21 Oct 2026 09:41:31 -0700 Subject: [PATCH 09/10] nvx-rs: share the unit tests' wait for a released lock The store's tests and the instance lock test each waited for a lock or claim that they had released with a loop of their own and literal durations. They now share one helper, whose wait and retry interval are named and documented. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03 --- nvx-rs/src/host/instance.rs | 18 ++++++------------ nvx-rs/src/host/mod.rs | 2 ++ nvx-rs/src/host/store/tests.rs | 20 +------------------- nvx-rs/src/host/test_support.rs | 28 ++++++++++++++++++++++++++++ 4 files changed, 37 insertions(+), 31 deletions(-) create mode 100644 nvx-rs/src/host/test_support.rs diff --git a/nvx-rs/src/host/instance.rs b/nvx-rs/src/host/instance.rs index 45fdb9d34..0e96e9317 100644 --- a/nvx-rs/src/host/instance.rs +++ b/nvx-rs/src/host/instance.rs @@ -965,6 +965,7 @@ mod tests { use std::thread; use super::*; + use crate::host::test_support::released; /// A process ID that fits every platform's type and that no process has. const GONE_PID: u32 = 0x7fff_fff0; @@ -1137,18 +1138,11 @@ mod tests { let contender = OpenOptions::new().write(true).open(&path).unwrap(); assert!(contender.try_lock().is_err()); drop(guard); - // A process that a concurrent test forked while the guard held the lock holds it until - // the process executes its program, so the released lock may take a moment to be free. - let deadline = Instant::now() + Duration::from_secs(10); - loop { - match contender.try_lock() { - Ok(()) => break, - Err(std::fs::TryLockError::WouldBlock) if Instant::now() < deadline => { - thread::sleep(Duration::from_millis(10)); - } - Err(error) => panic!("the released lock is not free: {error}"), - } - } + assert!(released(|| match contender.try_lock() { + Ok(()) => true, + Err(std::fs::TryLockError::WouldBlock) => false, + Err(error) => panic!("{error}"), + })); drop(contender); // A waiter takes the lock once the holder releases it. let guard = lock(&path).unwrap(); diff --git a/nvx-rs/src/host/mod.rs b/nvx-rs/src/host/mod.rs index d9dfe5872..f6a326dee 100644 --- a/nvx-rs/src/host/mod.rs +++ b/nvx-rs/src/host/mod.rs @@ -58,3 +58,5 @@ pub mod supervisor; #[cfg(unix)] #[allow(unsafe_code)] mod sys; +#[cfg(test)] +mod test_support; diff --git a/nvx-rs/src/host/store/tests.rs b/nvx-rs/src/host/store/tests.rs index 455848d1c..af2b5b3da 100644 --- a/nvx-rs/src/host/store/tests.rs +++ b/nvx-rs/src/host/store/tests.rs @@ -9,6 +9,7 @@ use super::*; use crate::ErrorKind; use crate::contract; use crate::host::process::{BootId, ProcessRef}; +use crate::host::test_support::released; /// A launch capability. const CAPABILITY: [u8; CAPABILITY_LEN] = [7; CAPABILITY_LEN]; @@ -44,25 +45,6 @@ fn write(store: &SandboxStore, name: &str, value: &serde_json::Value) { fs::write(store.path(name), serde_json::to_vec(value).unwrap()).unwrap(); } -/// How long a test waits for a lock or claim that it released to be free. -/// -/// A process that a concurrent test spawns holds a copy of each descriptor of this process from -/// its fork until it executes its program, and with it the lock or claim of a file that a test -/// had open, for that moment. -const RELEASE_WAIT: Duration = Duration::from_secs(10); - -/// Returns whether `free` holds before [`RELEASE_WAIT`] passes, checking it again until then. -fn released(mut free: impl FnMut() -> bool) -> bool { - let deadline = Instant::now() + RELEASE_WAIT; - while !free() { - if Instant::now() >= deadline { - return false; - } - thread::sleep(Duration::from_millis(10)); - } - true -} - #[test] fn artifact_identities_are_printable_tokens() { assert_eq!( diff --git a/nvx-rs/src/host/test_support.rs b/nvx-rs/src/host/test_support.rs new file mode 100644 index 000000000..f40e6d5f8 --- /dev/null +++ b/nvx-rs/src/host/test_support.rs @@ -0,0 +1,28 @@ +//! Helpers that the host's unit tests share. + +use std::thread; +use std::time::{Duration, Instant}; + +/// How long a test waits for a lock or claim that it released to be free. +/// +/// A process that a concurrent test spawns holds a copy of each descriptor of the test process +/// from its fork until it executes its program, and with it the lock or claim of a file that a +/// test had open, for that moment. +const RELEASE_WAIT: Duration = Duration::from_secs(10); + +/// Interval at which a test checks a lock or claim that it released again, the interval at which +/// the host retries a lock. +const RELEASE_RETRY_INTERVAL: Duration = Duration::from_millis(25); + +/// Returns whether `free` holds before [`RELEASE_WAIT`] passes, checking it again at every +/// [`RELEASE_RETRY_INTERVAL`] until then. +pub(crate) fn released(mut free: impl FnMut() -> bool) -> bool { + let deadline = Instant::now() + RELEASE_WAIT; + while !free() { + if Instant::now() >= deadline { + return false; + } + thread::sleep(RELEASE_RETRY_INTERVAL); + } + true +} From 8b0bf90099e5a5c9ed70b8ee9833aadb6d35c27d Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Wed, 21 Oct 2026 09:42:06 -0700 Subject: [PATCH 10/10] nvx-rs: say what polls at the process poll interval The interval's documentation said only that process shutdown and identity are polled at it. It now names the waits that poll, because no exit event serves them, and what the interval costs them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d9910f1d-1e20-46ea-b2b3-a2ec40433c03 --- nvx-rs/src/host/process.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nvx-rs/src/host/process.rs b/nvx-rs/src/host/process.rs index 9cae954ec..b45186445 100644 --- a/nvx-rs/src/host/process.rs +++ b/nvx-rs/src/host/process.rs @@ -41,7 +41,11 @@ use crate::protocol::CAPABILITY_LEN; /// Longest wait for a terminated OpenVMM process to exit. pub(crate) const KILL_TIMEOUT: Duration = Duration::from_secs(10); -/// Interval at which process shutdown and identity are polled. +/// Interval at which the exit or identity of a process is polled where no exit event serves: the +/// exit of any process on Linux hosts without pidfds, and the exit of a child that a [`Launched`] +/// waits for with a timeout on Linux, since [`std::process::Child`] can wait only without one. +/// [`wait_for_exit`] also checks again at this interval an identity that it could not read. An +/// exit is noticed at most this late, for about 40 checks a second. pub(crate) const POLL_INTERVAL: Duration = Duration::from_millis(25); /// The identity of one boot of the host, which changes whenever the host restarts.