diff --git a/doc/usage.md b/doc/usage.md index a0e64ec5d..12eb06dd9 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -110,6 +110,7 @@ each code for callers of the `nvx` library. | `template-ineligible` | 1 | The restore window of a template's generation, which the time contract of its capture sets, excludes the host's time, so no clone may restore it now. | | `template-tombstoned` | 1 | A tombstone quarantines the template's generation, which takes no new clones. | | `exited` | 1 | OpenVMM exited, as when a start fails, whose message names the category and status of OpenVMM's outcome report when it has one. | +| `still-running` | 1 | A stop could not confirm the VM process's exit, so the caller keeps its verified process. | | `exec-refused` | 1 | The guest agent refused the workload, as for a working directory that it cannot use. | | `endpoint-closed` | 1 | OpenVMM closed the control endpoint, as it does when the VM shuts down. | | `protocol` | 1 | The control session failed otherwise, as when the guest breaks the protocol or the session is reset. | diff --git a/nvx-rs/README.md b/nvx-rs/README.md index cd9c6f7ca..f9e4f884a 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -124,7 +124,7 @@ changes only when the module's purpose does. | [`nvx::Hypervisor`](#nvxhypervisor) | KVM, MSHV, or WHP, with its serde form behind the `serde` feature | | [`nvx::exec`](#nvxexec) | The execution engine: the streams, credit, and outcome of an execution's two sides, without I/O, for any framing | | [`nvx::host::runtime`](#nvxhostruntime) | `Runtime`: what every launch shares, the runtime's files and their identity, the hypervisor and its probe, and the launch options | -| [`nvx::host::vm`](#nvxhostvm) | A private VM directory, cold preparation, verified detached launch, and caller-saved reattachment | +| [`nvx::host::vm`](#nvxhostvm) | A private VM directory, cold preparation, verified detached launch, confirmed stop, and caller-saved reattachment | | [`nvx::host::snapshot`](#nvxhostsnapshot) | Snapshot compatibility keys, strict manifests, restore-time eligibility, and the opaque input of a future VM restore | | [`nvx::host::config`](#nvxhostconfig) | `VmConfig` and `CaptureConfig`: pure configuration, cold-boot kernel/OpenVMM arguments, portable length checks, and capture identity | | [`nvx::host::restore`](#nvxhostrestore) | A caller-driven restore to Ready without I/O or a clock, and its peer-verified restore-ready listener | @@ -500,6 +500,17 @@ or reading runtime files. `Reattached` holds both the process and authenticated connection. Failures retain their original kinds and sources and leave the saved values unchanged. +`stop(process, outputs, graceful, grace, deadline)` waits through the verified +process, first after the caller's graceful request, then after forced termination +if needed. It returns `Stopped` only after confirmed exit. `self_exited` records +confirmation before a forced attempt, and `forced` records that attempt, not +proof from kill success. An unconfirmed exit is `StillRunning`, and the caller +keeps the process. The callback receives the bounded grace deadline and must +respect it; its boolean result is never exit proof. + +`Stopped::report` retains the outcome-report read's result separately, so report +failure never hides confirmed exit. + `vm::create(&runtime, &config, None, &dir)` creates an inspectable cold `Prepared` launch without starting a process or binding a listener. It requires `config.boot`, refuses a snapshot input, rechecks the directory's privacy, and @@ -959,13 +970,15 @@ compatibility wrapper. 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. +`Process::open` returns `Opened::Running(Process)` only after checking the +recorded boot, ID, and start time. It holds the process through a pidfd on +Linux or a process handle on Windows, whether or not the caller started it, +so that waits and termination never reach a process that reuses its ID. +`Opened::Exited(Exited)` proves that the recorded boot ended or no running +process matches both its ID and start time. A process whose identity this +user cannot check makes `Process::open` fail rather than report an exit. +Linux hosts without pidfds poll the reference instead, and cannot terminate +the process. On Linux and Windows, `Process::exit_handle()` returns `Result>`: an independently owned duplicate of the pidfd @@ -976,6 +989,10 @@ On Windows, the duplicate retains the original handle's access rights. Duplication failures have kind `Io` and retain their original I/O source. No watcher thread, future or callback is created by the library. +`Process::wait(deadline)` returns `Some(Exited)` only after verified exit, or +`None` when the deadline passes without confirmation. `Exited` proves exit, +not an exit status, and cannot be made by a caller. + `Process::bytes_read` reports how many bytes the process has read through the operating system's read calls, whether the host's cache served them or not: `rchar` of `/proc//io` on Linux, which it reads only while the process @@ -1422,6 +1439,11 @@ 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. +`report::read` returns `Ok(None)` for a missing report, and an `Io` error for an +unreadable or malformed report, retaining the underlying error. It keeps today's +summary of the outcome category, status code and teardown steps, not free-form +fields. + A `Transport` to an OpenVMM control endpoint reads and writes with deadlines, and tells, through `Transport::unread`, what the endpoint has not read yet of what it wrote, where the platform tells: on Linux, what `SIOCOUTQ` reports of @@ -3055,6 +3077,7 @@ optional cause. | `Unavailable` | A missing or unusable artifact, hypervisor, or release, a guest image without a required control feature, or an unsupported host | | `Unsupported` | A request that NVX refuses: a value that it does not accept, such as a malformed or out-of-range one, options that it cannot combine, or a request that it cannot represent or enforce, such as a policy beyond a limit of OpenVMM or the guest agent | | `Exited` | The VM's OpenVMM process exited | +| `StillRunning` | A stop could not confirm the VM process's exit, so the caller keeps its verified process | | `Io` | A host I/O failure, or state that NVX recorded on the host and cannot read | | `NotFound` | State that an operation names and that does not exist on the host, such as the store of a sandbox that was never created or that a removal deleted | | `Protocol` | A control session that failed, or a request that the guest agent refused | @@ -3087,6 +3110,7 @@ usage errors, which no library error has. | `unavailable` | `Unavailable` | A dependency is missing or unusable, other than as a finer code says: install metadata, the hypervisor, a release, a distro layer or scratch image that the request names, or a host that NVX does not support | | `unsupported` | `Unsupported` | NVX refuses the request: options that it cannot combine, a value that it does not accept, or a policy that it cannot represent or enforce. The message names the option or field | | `exited` | `Exited` | OpenVMM exited, as when a start fails, whose message names the category and status of OpenVMM's outcome report when it has one | +| `still-running` | `StillRunning` | A stop could not confirm the VM process's exit, so the caller keeps its verified process | | `io` | `Io` | A host I/O operation failed, or state that NVX recorded on the host is unreadable or malformed | | `not-found` | `NotFound` | Other state that the operation names does not exist on the host | | `protocol` | `Protocol` | The control session failed otherwise: the peer violated the protocol, reset the session, or failed authentication | diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index 6a422c570..edc5c8d46 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -58,6 +58,7 @@ variant nvx::ErrorCode::SandboxNotRunning variant nvx::ErrorCode::SandboxProvisioned variant nvx::ErrorCode::SandboxRunning variant nvx::ErrorCode::ScratchClaimed +variant nvx::ErrorCode::StillRunning variant nvx::ErrorCode::TemplateIneligible variant nvx::ErrorCode::TemplateTombstoned variant nvx::ErrorCode::TimedOut @@ -73,6 +74,7 @@ variant nvx::ErrorKind::NotFound variant nvx::ErrorKind::Other variant nvx::ErrorKind::Protocol variant nvx::ErrorKind::RuntimeMismatch +variant nvx::ErrorKind::StillRunning variant nvx::ErrorKind::TimedOut variant nvx::ErrorKind::Unavailable variant nvx::ErrorKind::Unsupported @@ -2445,6 +2447,12 @@ method nvx::host::process::BootId::as_bytes method nvx::host::process::BootId::current method nvx::host::process::BootId::from_bytes type nvx::host::process::ExitHandle +struct nvx::host::process::Exited +enum nvx::host::process::Opened +variant nvx::host::process::Opened::Exited +field nvx::host::process::Opened::Exited::0 +variant nvx::host::process::Opened::Running +field nvx::host::process::Opened::Running::0 struct nvx::host::process::Process method nvx::host::process::Process::bytes_read method nvx::host::process::Process::exit_handle @@ -3768,6 +3776,10 @@ field nvx::host::vm::SavedVm::endpoint field nvx::host::vm::SavedVm::process field nvx::host::vm::SavedVm::runtime field nvx::host::vm::SavedVm::vm_dir +struct nvx::host::vm::Stopped +field nvx::host::vm::Stopped::forced +field nvx::host::vm::Stopped::report +field nvx::host::vm::Stopped::self_exited struct nvx::host::vm::VmDir method nvx::host::vm::VmDir::open method nvx::host::vm::VmDir::path @@ -3780,6 +3792,7 @@ fn nvx::host::vm::create fn nvx::host::vm::create_private_dir fn nvx::host::vm::reattach fn nvx::host::vm::spawn +fn nvx::host::vm::stop fn nvx::host::vm::verify mod nvx::net struct nvx::net::Cidr diff --git a/nvx-rs/public-api/windows.txt b/nvx-rs/public-api/windows.txt index 9aa7bf0f8..e45f6cacf 100644 --- a/nvx-rs/public-api/windows.txt +++ b/nvx-rs/public-api/windows.txt @@ -58,6 +58,7 @@ variant nvx::ErrorCode::SandboxNotRunning variant nvx::ErrorCode::SandboxProvisioned variant nvx::ErrorCode::SandboxRunning variant nvx::ErrorCode::ScratchClaimed +variant nvx::ErrorCode::StillRunning variant nvx::ErrorCode::TemplateIneligible variant nvx::ErrorCode::TemplateTombstoned variant nvx::ErrorCode::TimedOut @@ -73,6 +74,7 @@ variant nvx::ErrorKind::NotFound variant nvx::ErrorKind::Other variant nvx::ErrorKind::Protocol variant nvx::ErrorKind::RuntimeMismatch +variant nvx::ErrorKind::StillRunning variant nvx::ErrorKind::TimedOut variant nvx::ErrorKind::Unavailable variant nvx::ErrorKind::Unsupported @@ -1031,6 +1033,12 @@ method nvx::host::process::BootId::as_bytes method nvx::host::process::BootId::current method nvx::host::process::BootId::from_bytes type nvx::host::process::ExitHandle +struct nvx::host::process::Exited +enum nvx::host::process::Opened +variant nvx::host::process::Opened::Exited +field nvx::host::process::Opened::Exited::0 +variant nvx::host::process::Opened::Running +field nvx::host::process::Opened::Running::0 struct nvx::host::process::Process method nvx::host::process::Process::bytes_read method nvx::host::process::Process::exit_handle @@ -2353,6 +2361,10 @@ field nvx::host::vm::SavedVm::endpoint field nvx::host::vm::SavedVm::process field nvx::host::vm::SavedVm::runtime field nvx::host::vm::SavedVm::vm_dir +struct nvx::host::vm::Stopped +field nvx::host::vm::Stopped::forced +field nvx::host::vm::Stopped::report +field nvx::host::vm::Stopped::self_exited struct nvx::host::vm::VmDir method nvx::host::vm::VmDir::open method nvx::host::vm::VmDir::path @@ -2365,6 +2377,7 @@ fn nvx::host::vm::create fn nvx::host::vm::create_private_dir fn nvx::host::vm::reattach fn nvx::host::vm::spawn +fn nvx::host::vm::stop fn nvx::host::vm::verify mod nvx::net struct nvx::net::Cidr diff --git a/nvx-rs/src/error.rs b/nvx-rs/src/error.rs index 989538bab..e98ae8830 100644 --- a/nvx-rs/src/error.rs +++ b/nvx-rs/src/error.rs @@ -19,6 +19,8 @@ pub enum ErrorKind { Unsupported, /// The OpenVMM process of a VM exited. Exited, + /// The exit of a VM's process could not be confirmed; it may still run. + StillRunning, /// A host I/O operation failed, or state that NVX recorded on the host is unreadable or /// malformed. Io, @@ -44,6 +46,7 @@ impl ErrorKind { Self::Unavailable => "unavailable", Self::Unsupported => "unsupported", Self::Exited => "exited", + Self::StillRunning => "still-running", Self::Io => "io", Self::NotFound => "not-found", Self::Protocol => "protocol", @@ -88,6 +91,8 @@ pub enum ErrorCode { Unsupported, /// `exited`: the OpenVMM process of a VM exited, as when a start fails. Exited, + /// `still-running`: the exit of a VM's process could not be confirmed. + StillRunning, /// `io`: a host I/O operation failed, or state that NVX recorded on the host is unreadable or /// malformed. Io, @@ -170,6 +175,7 @@ impl ErrorCode { Self::Unavailable, Self::Unsupported, Self::Exited, + Self::StillRunning, Self::Io, Self::NotFound, Self::Protocol, @@ -205,6 +211,7 @@ impl ErrorCode { Self::Unavailable => ErrorKind::Unavailable.as_str(), Self::Unsupported => ErrorKind::Unsupported.as_str(), Self::Exited => ErrorKind::Exited.as_str(), + Self::StillRunning => ErrorKind::StillRunning.as_str(), Self::Io => ErrorKind::Io.as_str(), Self::NotFound => ErrorKind::NotFound.as_str(), Self::Protocol => ErrorKind::Protocol.as_str(), @@ -248,6 +255,7 @@ impl ErrorCode { | Self::ArgumentMissing | Self::ArgumentInvalid => ErrorKind::Unsupported, Self::Exited | Self::SandboxExited => ErrorKind::Exited, + Self::StillRunning => ErrorKind::StillRunning, Self::Io | Self::FormatUnsupported => ErrorKind::Io, Self::NotFound | Self::SandboxNotProvisioned => ErrorKind::NotFound, Self::Protocol | Self::ExecRefused | Self::EndpointClosed => ErrorKind::Protocol, @@ -278,6 +286,7 @@ impl From for ErrorCode { ErrorKind::Unavailable => Self::Unavailable, ErrorKind::Unsupported => Self::Unsupported, ErrorKind::Exited => Self::Exited, + ErrorKind::StillRunning => Self::StillRunning, ErrorKind::Io => Self::Io, ErrorKind::NotFound => Self::NotFound, ErrorKind::Protocol => Self::Protocol, diff --git a/nvx-rs/src/error/tests.rs b/nvx-rs/src/error/tests.rs index eb87d7dc0..a893e72b7 100644 --- a/nvx-rs/src/error/tests.rs +++ b/nvx-rs/src/error/tests.rs @@ -52,6 +52,7 @@ fn every_kind_has_a_code_of_its_own_spelling() { ErrorKind::Unavailable, ErrorKind::Unsupported, ErrorKind::Exited, + ErrorKind::StillRunning, ErrorKind::Io, ErrorKind::NotFound, ErrorKind::Protocol, diff --git a/nvx-rs/src/host/cli/store/files.rs b/nvx-rs/src/host/cli/store/files.rs new file mode 100644 index 000000000..9050cf281 --- /dev/null +++ b/nvx-rs/src/host/cli/store/files.rs @@ -0,0 +1,51 @@ +//! Private, regular, non-link files, atomically replaced and synced under the state lock. + +use std::fs; +use std::io; + +use super::StateFile; +use super::locked::LockedState; +use crate::host::platform; +use crate::host::store::{files, regular}; +use crate::{Error, Result}; + +/// Bounds a fixed record's paths and identities without reading an unbounded corrupt file. +const MAX_RECORD_BYTES: u64 = 1024 * 1024; + +pub(super) fn read(state: &LockedState, name: StateFile) -> Result>> { + let path = state.path(name); + let failed = |error| Error::io(format!("cannot read {}", path.display())).with_source(error); + let file = match regular::open_regular(&path) { + Ok(file) => file, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(failed(error)), + }; + if let Some(problem) = platform::private_file_problem(&file).map_err(failed)? { + return Err(Error::io(format!( + "the sandbox file {} {problem}", + path.display() + ))); + } + regular::read_at_most(file, MAX_RECORD_BYTES, "a sandbox record") + .map(Some) + .map_err(failed) +} + +pub(super) fn replace(state: &LockedState, name: StateFile, bytes: &[u8]) -> Result<()> { + if bytes.len() as u64 > MAX_RECORD_BYTES { + return Err(Error::io("the sandbox record exceeds its size bound")); + } + // Refuse an existing unsafe entry rather than replacing it or following its target. + read(state, name)?; + files::write_private(&state.path(name), bytes)?; + files::sync_dir(state.dir()) +} + +pub(super) fn remove(state: &LockedState, name: StateFile) -> Result<()> { + read(state, name)?; + let path = state.path(name); + fs::remove_file(&path).map_err(|error| { + Error::io(format!("cannot remove {}", path.display())).with_source(error) + })?; + files::sync_dir(state.dir()) +} diff --git a/nvx-rs/src/host/cli/store/launch.rs b/nvx-rs/src/host/cli/store/launch.rs new file mode 100644 index 000000000..d0a5c04e6 --- /dev/null +++ b/nvx-rs/src/host/cli/store/launch.rs @@ -0,0 +1,80 @@ +//! Locked launch transitions, without spawning, recovery, or process termination. + +use super::StateFile; +use super::files; +use super::locked::LockedState; +use super::record::{LaunchJournal, RuntimeRecord}; +use crate::host::process::ProcessRef; +use crate::host::runtime::RuntimeIdentity; +use crate::host::vm::{Prepared, SavedVm}; +use crate::{Error, Result}; + +/// A launch whose lock stays borrowed until commit or until its journal is left behind. +#[derive(Debug)] +pub(crate) struct Launch<'a> { + locked: &'a mut LockedState, + journal: LaunchJournal, +} + +impl LockedState { + /// Durably publishes the capability and complete prepared journal before the caller spawns. + pub(crate) fn begin_launch( + &mut self, + prepared: &Prepared, + runtime: &RuntimeIdentity, + ) -> Result> { + if self.journal()?.is_some() || self.runtime()?.is_some() { + return Err(Error::other( + "the sandbox already has a runtime record or an interrupted launch", + )); + } + let journal = LaunchJournal::prepared(self, prepared, runtime)?; + files::replace(self, StateFile::Capability, &journal.capability)?; + self.write_record(StateFile::Journal, &journal)?; + Ok(Launch { + locked: self, + journal, + }) + } +} + +impl Launch<'_> { + /// Records only the caller's spawned process, durably, before changing the in-memory state. + pub(crate) fn record_process(&mut self, process: &ProcessRef) -> Result<()> { + if self.journal.process.is_some() { + return Err(Error::other("the launch has recorded its process already")); + } + let journal = LaunchJournal { + process: Some(process.clone()), + ..self.journal.clone() + }; + self.locked.write_record(StateFile::Journal, &journal)?; + self.journal = journal; + Ok(()) + } + + /// Publishes the exact saved VM, only after its matching process was journaled. + /// + /// The runtime record is the commit. A failed journal removal keeps that committed record + /// and reports the failure; it never rolls back the commit or signals the VM. + pub(crate) fn commit(self, saved: &SavedVm) -> Result<()> { + if self.journal.process.as_ref() != Some(&saved.process) + || self.journal.vm_dir != saved.vm_dir.path() + || self.journal.endpoint != saved.endpoint + || self.journal.capability != saved.capability + || self.journal.runtime != saved.runtime + { + return Err(Error::other( + "the saved VM does not match the launch and its journaled process", + )); + } + self.locked + .write_record(StateFile::Runtime, &RuntimeRecord::from(saved))?; + files::remove(self.locked, StateFile::Journal).map_err(|error| { + Error::io(format!( + "the VM runtime was committed but its launch journal could not be removed: {error}" + )) + .with_source(error) + }) + } +} diff --git a/nvx-rs/src/host/cli/store/liveness.rs b/nvx-rs/src/host/cli/store/liveness.rs new file mode 100644 index 000000000..4b584f836 --- /dev/null +++ b/nvx-rs/src/host/cli/store/liveness.rs @@ -0,0 +1,86 @@ +//! Bounded launch proofs, without recovery, record removal, or process termination. + +use std::io; + +use super::StateFile; +use super::locked::LockedState; +use super::record::LaunchJournal; +use crate::host::platform; +use crate::host::process::{BootId, Opened, Process, ProcessRef}; +use crate::{Error, Result}; + +impl LockedState { + /// Returns whether a launch process may still run. Only a released claim proves exit; + /// failure to examine it proves neither, and preserves the original error. + pub(crate) fn claim_held(&self, journal: &LaunchJournal) -> Result { + let path = self.path(StateFile::Log); + if journal.vm_dir != self.dir() || journal.log != path { + return Err(Error::io( + "the launch journal belongs to another sandbox directory", + )); + } + platform::log_claim_held(&path, journal.log_identity) + .map_err(|error| Error::io("cannot check the launch log claim").with_source(error)) + } + + /// Holds the server of the recorded endpoint, rechecks its ownership, and on Linux proves + /// that this very process also holds the launch's log. An error is not proof of exit. + pub(crate) fn endpoint_owner(&self, journal: &LaunchJournal) -> Result { + if !self.claim_held(journal)? { + return Err(Error::io("the launch log claim is released")); + } + let process = + hold_endpoint_server(|| platform::endpoint_server_pid(&journal.endpoint), find)?; + let reference = process.reference(); + let matches = + platform::serves_launch(reference.pid, reference.start_time, journal.log_identity) + .map_err(|error| { + Error::io("cannot verify the endpoint owner's log").with_source(error) + })?; + if !matches { + return Err(Error::io( + "the endpoint owner does not hold the launch log as its standard output", + )); + } + Ok(process) + } +} + +/// Opens only the process whose start time was just read, never another that reused its ID. +fn find(pid: u32) -> io::Result> { + let Some(start_time) = platform::process_start_time(pid)? else { + return Ok(None); + }; + Ok( + match Process::open(&ProcessRef { + pid, + start_time, + boot: BootId::current()?, + })? { + Opened::Running(process) => Some(process), + Opened::Exited(_) => None, + }, + ) +} + +/// Adapted from the old store's owner check: the second endpoint lookup happens only after +/// the process is held. The callbacks keep replacement races testable without test-only hooks. +pub(super) fn hold_endpoint_server( + mut server_pid: impl FnMut() -> io::Result>, + hold: impl FnOnce(u32) -> io::Result>, +) -> Result { + let pid = server_pid() + .map_err(|error| Error::io("cannot examine the control endpoint").with_source(error))? + .ok_or_else(|| Error::io("no process serves the control endpoint"))?; + let process = hold(pid) + .map_err(|error| Error::io("cannot hold the control endpoint owner").with_source(error))? + .ok_or_else(|| Error::io("the control endpoint owner exited before it could be held"))?; + let again = server_pid() + .map_err(|error| Error::io("cannot recheck the control endpoint").with_source(error))?; + if again != Some(pid) { + return Err(Error::io( + "the control endpoint changed while its owner was held", + )); + } + Ok(process) +} diff --git a/nvx-rs/src/host/cli/store/mod.rs b/nvx-rs/src/host/cli/store/mod.rs index 6f0c6479f..ac58708e0 100644 --- a/nvx-rs/src/host/cli/store/mod.rs +++ b/nvx-rs/src/host/cli/store/mod.rs @@ -26,6 +26,13 @@ //! and the Administrators group, as one under the user's profile does. An existing directory that //! others may access is refused rather than restricted, since they could have placed files in it, //! or still hold them open. The store creates its files readable only by the current user. +//! +//! A locked launch writes its prepared capability and outputs before spawn, records the spawned +//! process durably, and publishes the caller's exact saved VM only at commit. Dropping an +//! uncommitted launch keeps its journal; these transitions do not recover or signal a process. +//! Its journal also identifies the inherited log claim: held means a launch process may run, +//! and released proves exit. An endpoint owner is held and rechecked before it is returned; +//! Linux additionally verifies that this process holds that launch's log. #![cfg_attr( not(test), expect( @@ -34,8 +41,12 @@ ) )] +mod files; +mod launch; +mod liveness; mod locked; mod names; +mod record; use std::io; use std::path::{Path, PathBuf}; diff --git a/nvx-rs/src/host/cli/store/record.rs b/nvx-rs/src/host/cli/store/record.rs new file mode 100644 index 000000000..fa85fdf18 --- /dev/null +++ b/nvx-rs/src/host/cli/store/record.rs @@ -0,0 +1,167 @@ +//! The fixed launch journal and the caller-saved VM's runtime record. + +use std::fmt; +use std::path::PathBuf; + +use serde::de::DeserializeOwned; +use serde::{Deserialize, Serialize}; + +use super::StateFile; +use super::files; +use super::locked::LockedState; +use crate::host::platform; +use crate::host::process::ProcessRef; +use crate::host::runtime::RuntimeIdentity; +use crate::host::vm::{Prepared, SavedVm, VmDir, VmOutputs}; +use crate::protocol::CAPABILITY_LEN; +use crate::{Error, Result}; + +#[derive(Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub(crate) struct LaunchJournal { + pub(super) vm_dir: PathBuf, + pub(super) capability: [u8; CAPABILITY_LEN], + pub(super) log: PathBuf, + pub(super) log_identity: (u64, u64), + pub(super) outcome: PathBuf, + pub(super) endpoint: String, + pub(super) console: Option, + pub(super) runtime: RuntimeIdentity, + pub(super) process: Option, +} + +impl fmt::Debug for LaunchJournal { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("LaunchJournal") + .field("vm_dir", &self.vm_dir) + .field("log", &self.log) + .field("log_identity", &self.log_identity) + .field("outcome", &self.outcome) + .field("endpoint", &self.endpoint) + .field("console", &self.console) + .field("runtime", &self.runtime) + .field("process", &self.process) + .finish_non_exhaustive() + } +} + +impl LaunchJournal { + pub(super) fn prepared( + state: &LockedState, + prepared: &Prepared, + runtime: &RuntimeIdentity, + ) -> Result { + runtime.check()?; + let vm_dir = VmDir::open(state.dir())?; + let VmOutputs { + log, + outcome, + endpoint, + console, + } = prepared.outputs(); + if *log != vm_dir.path().join(StateFile::Log.name()) + || *outcome != vm_dir.path().join(StateFile::Outcome.name()) + { + return Err(Error::other( + "the prepared launch belongs to another sandbox directory", + )); + } + platform::claim_log(prepared.log()) + .map_err(|error| Error::io("cannot claim the prepared VM log").with_source(error))?; + let log_identity = platform::opened_file_identity(prepared.log()) + .map_err(|error| Error::io("cannot identify the prepared VM log").with_source(error))?; + Ok(Self { + vm_dir: vm_dir.path().to_path_buf(), + capability: *prepared.capability().as_bytes(), + log: log.clone(), + log_identity, + outcome: outcome.clone(), + endpoint: endpoint.clone(), + console: console.clone(), + runtime: runtime.clone(), + process: None, + }) + } + + pub(crate) fn process(&self) -> Option<&ProcessRef> { + self.process.as_ref() + } +} + +#[derive(Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub(super) struct RuntimeRecord { + vm_dir: PathBuf, + endpoint: String, + capability: [u8; CAPABILITY_LEN], + process: ProcessRef, + runtime: RuntimeIdentity, +} + +impl From<&SavedVm> for RuntimeRecord { + fn from(saved: &SavedVm) -> Self { + Self { + vm_dir: saved.vm_dir.path().to_path_buf(), + endpoint: saved.endpoint.clone(), + capability: saved.capability, + process: saved.process.clone(), + runtime: saved.runtime.clone(), + } + } +} + +impl LockedState { + pub(crate) fn journal(&self) -> Result> { + self.read_record(StateFile::Journal) + } + + pub(crate) fn runtime(&self) -> Result> { + let Some(record) = self.read_record::(StateFile::Runtime)? else { + return Ok(None); + }; + let vm_dir = VmDir::open(self.dir())?; + if record.vm_dir != vm_dir.path() { + return Err(Error::io( + "the runtime record belongs to another sandbox directory", + )); + } + Ok(Some(SavedVm { + vm_dir, + endpoint: record.endpoint, + capability: record.capability, + process: record.process, + runtime: record.runtime, + })) + } + + pub(crate) fn capability(&self) -> Result> { + files::read(self, StateFile::Capability)? + .map(|bytes| { + bytes.try_into().map_err(|_| { + Error::io(format!( + "the sandbox capability must hold {CAPABILITY_LEN} bytes" + )) + }) + }) + .transpose() + } + + fn read_record(&self, name: StateFile) -> Result> { + files::read(self, name)? + .map(|bytes| { + serde_json::from_slice(&bytes).map_err(|error| { + Error::io(format!("cannot parse {}", self.path(name).display())) + .with_source(error) + }) + }) + .transpose() + } + + pub(super) fn write_record(&self, name: StateFile, record: &impl Serialize) -> Result<()> { + let bytes = serde_json::to_vec(record).map_err(|error| { + Error::io(format!("cannot serialize {}", self.path(name).display())).with_source(error) + })?; + files::replace(self, name, &bytes) + } +} diff --git a/nvx-rs/src/host/cli/store/tests.rs b/nvx-rs/src/host/cli/store/tests.rs index bb36e8418..5db8c5adb 100644 --- a/nvx-rs/src/host/cli/store/tests.rs +++ b/nvx-rs/src/host/cli/store/tests.rs @@ -8,6 +8,9 @@ use super::*; use crate::host::cli::deadline::Deadline; use crate::host::store::{JOURNAL_NAME, RUNTIME_NAME, SandboxStore}; +mod launch; +mod liveness; + /// How long a test waits for a lock that it released to be free. /// /// A process that a concurrent test spawns holds a copy of each descriptor of the test process diff --git a/nvx-rs/src/host/cli/store/tests/launch.rs b/nvx-rs/src/host/cli/store/tests/launch.rs new file mode 100644 index 000000000..5fd38d31a --- /dev/null +++ b/nvx-rs/src/host/cli/store/tests/launch.rs @@ -0,0 +1,385 @@ +use std::error::Error as _; + +use super::*; +use crate::ErrorKind; +use crate::activation::{MachineIdMode, View}; +use crate::host::cli::store::{files, record}; +use crate::host::config::{Boot, ConsoleAttachment, RootConfig, VmConfig}; +use crate::host::process::{BootId, ProcessRef}; +use crate::host::runtime::{ArtifactIdentity, LaunchOptions, Runtime, RuntimeIdentity}; +use crate::host::vm::{self, Prepared, SavedVm, VmDir}; +use crate::hypervisor::Hypervisor; +use crate::protocol::CAPABILITY_LEN; + +pub(super) fn prepared( + state: &StateDir, + console: ConsoleAttachment, +) -> (Prepared, RuntimeIdentity) { + let identity = |text| ArtifactIdentity::new(text).unwrap(); + let runtime = Runtime { + openvmm: "missing-openvmm".into(), + kernel: "missing-kernel".into(), + initramfs: "missing-initramfs".into(), + identity: RuntimeIdentity { + openvmm: identity("vmm-revision"), + kernel: identity("kernel-revision"), + initramfs: identity("initramfs-revision"), + boot_contract: u64::MAX, + control_contract: "saved-control-revision".to_owned(), + control_session: u64::MAX - 1, + }, + hypervisor: if cfg!(windows) { + Hypervisor::Whp + } else { + Hypervisor::Mshv + }, + cpu_profile: None, + windows_breakaway: false, + options: LaunchOptions::default(), + }; + let config = VmConfig { + processors: 1, + memory_mib: 256, + root: RootConfig::Blockless, + view: View { + hostname: b"journal-test".to_vec(), + machine_id: MachineIdMode::Image, + }, + network: None, + filesystem: None, + console, + boot: Some(Boot { + command_line: "nvx_entrypoint=/bin/sh".to_owned(), + capture: None, + }), + }; + let vm_dir = VmDir::open(state.dir()).unwrap(); + ( + vm::create(&runtime, &config, None, &vm_dir).unwrap(), + runtime.identity, + ) +} + +fn saved(state: &StateDir, prepared: &Prepared, runtime: RuntimeIdentity) -> SavedVm { + SavedVm { + vm_dir: VmDir::open(state.dir()).unwrap(), + endpoint: prepared.outputs().endpoint.clone(), + capability: *prepared.capability().as_bytes(), + process: ProcessRef { + pid: u32::MAX, + start_time: u64::MAX, + boot: BootId::from_bytes([0xa5; 16]), + }, + runtime, + } +} + +fn assert_saved(actual: &SavedVm, expected: &SavedVm) { + assert_eq!(actual.vm_dir, expected.vm_dir); + assert_eq!(actual.endpoint, expected.endpoint); + assert_eq!(actual.capability, expected.capability); + assert_eq!(actual.process, expected.process); + assert_eq!(actual.runtime, expected.runtime); +} + +fn read_file(locked: &locked::LockedState, name: StateFile) -> crate::Result { + match name { + StateFile::Journal => locked.journal().map(|record| record.is_some()), + StateFile::Runtime => locked.runtime().map(|record| record.is_some()), + StateFile::Capability => locked.capability().map(|record| record.is_some()), + _ => unreachable!(), + } +} + +#[test] +fn the_pre_spawn_barrier_and_commit_preserve_every_prepared_and_saved_value() { + for console in [ConsoleAttachment::Listener, ConsoleAttachment::Disconnected] { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, console); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + let spawn_hook = || { + let bytes = fs::read(state.path(StateFile::Journal)).unwrap(); + let journal: record::LaunchJournal = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(journal.vm_dir, saved.vm_dir.path()); + assert_eq!(journal.capability, saved.capability); + assert_eq!(journal.log, prepared.outputs().log); + assert_eq!(journal.outcome, prepared.outputs().outcome); + assert_eq!(journal.endpoint, prepared.outputs().endpoint); + assert_eq!(journal.console, prepared.outputs().console); + assert_eq!(journal.runtime, runtime); + assert_eq!(journal.process(), None); + assert!(!format!("{journal:?}").contains(&format!("{:?}", saved.capability))); + assert_eq!( + fs::read(state.path(StateFile::Capability)).unwrap(), + saved.capability + ); + for name in [StateFile::Journal, StateFile::Capability] { + let path = state.path(name); + let metadata = fs::symlink_metadata(&path).unwrap(); + assert!(metadata.is_file()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(metadata.permissions().mode() & 0o7777, 0o600); + } + let file = fs::File::open(path).unwrap(); + assert_eq!(platform::private_file_problem(&file).unwrap(), None); + } + assert!(!state.path(StateFile::Runtime).exists()); + }; + spawn_hook(); + launch.record_process(&saved.process).unwrap(); + let journal: record::LaunchJournal = + serde_json::from_slice(&fs::read(state.path(StateFile::Journal)).unwrap()).unwrap(); + assert_eq!(journal.process(), Some(&saved.process)); + assert!(!state.path(StateFile::Runtime).exists()); + launch.commit(&saved).unwrap(); + assert!(!state.path(StateFile::Journal).exists()); + assert_saved(&locked.runtime().unwrap().unwrap(), &saved); + assert_eq!(locked.capability().unwrap(), Some(saved.capability)); + drop(locked); + drop(prepared); + let reopened = StateDir::open(state.dir()) + .unwrap() + .lock(Deadline::after(10.0)) + .unwrap(); + assert_saved(&reopened.runtime().unwrap().unwrap(), &saved); + for name in [StateFile::Runtime, StateFile::Capability] { + let path = state.path(name); + let metadata = fs::symlink_metadata(&path).unwrap(); + assert!(metadata.is_file()); + assert!(!crate::host::filesystem::is_reparse_point(&metadata)); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!(metadata.permissions().mode() & 0o7777, 0o600); + } + let file = fs::File::open(path).unwrap(); + assert_eq!(platform::private_file_problem(&file).unwrap(), None); + } + } +} + +#[test] +fn an_interruption_keeps_the_journal_and_capability_without_publishing_runtime() { + for spawned in [false, true] { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Listener); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + { + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + if spawned { + launch.record_process(&saved.process).unwrap(); + } + } + drop(locked); + let mut reopened = state.lock(Deadline::after(10.0)).unwrap(); + let journal = reopened.journal().unwrap().unwrap(); + assert_eq!(journal.process(), spawned.then_some(&saved.process)); + assert_eq!(journal.capability, saved.capability); + assert_eq!(reopened.capability().unwrap(), Some(saved.capability)); + assert!(reopened.runtime().unwrap().is_none()); + let before = fs::read(state.path(StateFile::Journal)).unwrap(); + assert!(reopened.begin_launch(&prepared, &runtime).is_err()); + assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), before); + } +} + +#[test] +fn a_commit_needs_the_same_saved_values_and_an_already_journaled_process() { + for mismatch in 0..6 { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let mut saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + if mismatch != 0 { + launch.record_process(&saved.process).unwrap(); + assert!(launch.record_process(&saved.process).is_err()); + } + let original = fs::read(state.path(StateFile::Journal)).unwrap(); + match mismatch { + 0 => {} + 1 => saved.process.start_time -= 1, + 2 => saved.endpoint.push('x'), + 3 => saved.capability[0] ^= 1, + 4 => saved.runtime.control_session -= 1, + 5 => { + let other = state.dir().join("other-vm"); + vm::create_private_dir(&other).unwrap(); + saved.vm_dir = VmDir::open(&other).unwrap(); + } + _ => unreachable!(), + } + assert!(launch.commit(&saved).is_err()); + assert!(!state.path(StateFile::Runtime).exists()); + assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), original); + } +} + +#[test] +fn preparation_for_another_directory_and_failed_record_writes_do_not_publish_runtime() { + let (_directory, state) = state(); + let (_other_directory, other) = super::state(); + let (other_prepared, runtime) = prepared(&other, ConsoleAttachment::Disconnected); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + assert!(locked.begin_launch(&other_prepared, &runtime).is_err()); + assert!(!state.path(StateFile::Journal).exists()); + assert!(!state.path(StateFile::Capability).exists()); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let saved = saved(&state, &prepared, runtime.clone()); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + let journal = state.path(StateFile::Journal); + let original = journal.with_extension("original"); + fs::rename(&journal, &original).unwrap(); + fs::create_dir(&journal).unwrap(); + let error = launch.record_process(&saved.process).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!(error.source().is_some()); + fs::remove_dir(&journal).unwrap(); + fs::rename(&original, &journal).unwrap(); + assert!(launch.commit(&saved).is_err()); + assert!(!state.path(StateFile::Runtime).exists()); + assert!(locked.journal().unwrap().unwrap().process().is_none()); +} + +#[test] +fn a_failed_runtime_publication_keeps_the_spawned_journal() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + launch.record_process(&saved.process).unwrap(); + let before = fs::read(state.path(StateFile::Journal)).unwrap(); + fs::create_dir(state.path(StateFile::Runtime)).unwrap(); + let error = launch.commit(&saved).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!(error.source().is_some()); + assert_eq!(fs::read(state.path(StateFile::Journal)).unwrap(), before); + assert_eq!( + fs::read_dir(state.path(StateFile::Runtime)) + .unwrap() + .count(), + 0 + ); +} + +#[test] +fn a_failed_journal_removal_reports_that_runtime_already_committed() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let saved = saved(&state, &prepared, runtime.clone()); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let mut launch = locked.begin_launch(&prepared, &runtime).unwrap(); + launch.record_process(&saved.process).unwrap(); + fs::remove_file(state.path(StateFile::Journal)).unwrap(); + fs::create_dir(state.path(StateFile::Journal)).unwrap(); + let error = launch.commit(&saved).unwrap_err(); + assert!(error.message().contains("runtime was committed"), "{error}"); + assert!(error.source().unwrap().source().is_some()); + assert_saved(&locked.runtime().unwrap().unwrap(), &saved); + assert!(state.path(StateFile::Journal).is_dir()); +} + +#[test] +fn malformed_records_and_capabilities_keep_their_parse_errors() { + let (_directory, state) = state(); + let locked = state.lock(Deadline::after(10.0)).unwrap(); + for name in [StateFile::Journal, StateFile::Runtime] { + files::replace(&locked, name, b"{malformed").unwrap(); + let error = read_file(&locked, name).unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!( + error + .source() + .unwrap() + .downcast_ref::() + .is_some(), + "{error}" + ); + } + files::replace(&locked, StateFile::Capability, &[0x5a; CAPABILITY_LEN - 1]).unwrap(); + assert_eq!(locked.capability().unwrap_err().kind(), ErrorKind::Io); +} + +#[test] +fn records_and_capabilities_refuse_non_regular_entries_and_links_before_read_or_replace() { + for name in [ + StateFile::Journal, + StateFile::Runtime, + StateFile::Capability, + ] { + let (directory, state) = state(); + let locked = state.lock(Deadline::after(10.0)).unwrap(); + let path = state.path(name); + fs::create_dir(&path).unwrap(); + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + assert_eq!(fs::read_dir(&path).unwrap().count(), 0); + fs::remove_dir(&path).unwrap(); + let target = directory.path().join("target"); + fs::write(&target, b"preserved target").unwrap(); + #[cfg(unix)] + let linked = std::os::unix::fs::symlink(&target, &path); + #[cfg(windows)] + let linked = std::os::windows::fs::symlink_file(&target, &path); + match linked { + Ok(()) => { + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + assert_eq!(fs::read(&target).unwrap(), b"preserved target"); + fs::remove_file(&path).unwrap(); + } + #[cfg(windows)] + Err(error) if error.raw_os_error() == Some(1314) => { + eprintln!("skipped the symbolic link without Developer Mode or privilege: {error}"); + } + Err(error) => panic!("cannot create the test symbolic link: {error}"), + } + #[cfg(target_os = "linux")] + { + use nix::sys::stat::Mode; + nix::unistd::mkfifo(&path, Mode::S_IRUSR | Mode::S_IWUSR).unwrap(); + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + } + #[cfg(windows)] + { + let target = directory.path().join("junction-target"); + fs::create_dir(&target).unwrap(); + junction(&path, &target); + assert!(read_file(&locked, name).is_err()); + assert!(files::replace(&locked, name, b"replacement").is_err()); + assert_eq!(fs::read_dir(target).unwrap().count(), 0); + } + } +} + +#[cfg(windows)] +#[test] +fn records_and_capabilities_that_other_users_may_access_are_refused_without_replacement() { + let (_directory, state) = state(); + let locked = state.lock(Deadline::after(10.0)).unwrap(); + for name in [ + StateFile::Journal, + StateFile::Runtime, + StateFile::Capability, + ] { + files::replace(&locked, name, b"preserved shared contents").unwrap(); + let path = state.path(name); + allow_everyone(&path, ""); + let before = fs::read(&path).unwrap(); + for error in [ + read_file(&locked, name).unwrap_err(), + files::replace(&locked, name, b"replacement").unwrap_err(), + ] { + assert_eq!(error.kind(), ErrorKind::Io); + assert!(error.message().contains("allows other users"), "{error}"); + } + assert_eq!(fs::read(path).unwrap(), before); + } +} diff --git a/nvx-rs/src/host/cli/store/tests/liveness.rs b/nvx-rs/src/host/cli/store/tests/liveness.rs new file mode 100644 index 000000000..e9bb3f683 --- /dev/null +++ b/nvx-rs/src/host/cli/store/tests/liveness.rs @@ -0,0 +1,271 @@ +use std::error::Error as _; +use std::io; +#[cfg(any(target_os = "linux", windows))] +use std::process::{Child, Command, Stdio}; + +use super::launch::prepared; +use super::*; +use crate::host::cli::store::liveness::hold_endpoint_server; +use crate::host::config::ConsoleAttachment; +use crate::host::process::{BootId, Opened, Process, ProcessRef}; +use crate::host::spawn::{self, VmmCommand}; + +const HELPER: &str = "host::cli::store::tests::liveness::log_holder_helper"; + +fn own_process() -> Process { + let pid = std::process::id(); + let Opened::Running(process) = Process::open(&ProcessRef { + pid, + start_time: platform::process_start_time(pid).unwrap().unwrap(), + boot: BootId::current().unwrap(), + }) + .unwrap() else { + panic!("this process runs"); + }; + process +} + +struct Running(Option); + +impl Drop for Running { + fn drop(&mut self) { + if let Some(child) = self.0.take() { + child.kill_and_confirm_exit().unwrap(); + } + } +} + +#[cfg(any(target_os = "linux", windows))] +struct Other(Child); + +#[cfg(any(target_os = "linux", windows))] +impl Drop for Other { + fn drop(&mut self) { + self.0.kill().unwrap(); + self.0.wait().unwrap(); + } +} + +#[cfg(any(target_os = "linux", windows))] +fn helper_command(state: &StateDir) -> Command { + let mut command = Command::new(std::env::current_exe().unwrap()); + command + .args(["--exact", HELPER, "--ignored", "--nocapture"]) + .current_dir(state.dir()) + .stdin(Stdio::null()); + command +} + +#[test] +fn a_claim_identifies_the_opened_log_and_last_close_proves_exit() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let identity = platform::opened_file_identity(prepared.log()).unwrap(); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + drop(locked.begin_launch(&prepared, &runtime).unwrap()); + let journal = locked.journal().unwrap().unwrap(); + assert_eq!(journal.log_identity, identity); + assert!(locked.claim_held(&journal).unwrap()); + let copy = prepared.log().try_clone().unwrap(); + drop(prepared); + assert!(locked.claim_held(&journal).unwrap()); + drop(copy); + assert!(released(|| !locked.claim_held(&journal).unwrap())); + assert!(locked.journal().unwrap().is_some()); + assert!(locked.capability().unwrap().is_some()); +} + +#[test] +fn replacing_the_log_or_sandbox_never_proves_the_launch_exited() { + let (_directory, state) = state(); + let (_other_directory, other) = super::state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + drop(locked.begin_launch(&prepared, &runtime).unwrap()); + let journal = locked.journal().unwrap().unwrap(); + let elsewhere = other.lock(Deadline::after(10.0)).unwrap(); + assert!(elsewhere.claim_held(&journal).is_err()); + drop(prepared); + fs::rename(state.path(StateFile::Log), state.dir().join("old-log")).unwrap(); + fs::write(state.path(StateFile::Log), b"another launch").unwrap(); + let error = locked.claim_held(&journal).unwrap_err(); + assert!(error.source().is_some()); + assert!( + error + .to_string() + .contains("cannot check the launch log claim") + ); + assert!(locked.endpoint_owner(&journal).is_err()); + assert!(locked.journal().unwrap().is_some()); +} + +#[test] +fn the_spawned_process_inherits_the_preparations_claim() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + drop(locked.begin_launch(&prepared, &runtime).unwrap()); + let journal = locked.journal().unwrap().unwrap(); + let command = VmmCommand::new( + std::env::current_exe().unwrap(), + ["--exact", HELPER, "--ignored", "--nocapture"] + .map(Into::into) + .to_vec(), + state.dir(), + ); + let child = Running(Some( + spawn::spawn_vm( + &command, + prepared.capability().as_bytes(), + prepared.log().try_clone().unwrap(), + ) + .unwrap(), + )); + drop(prepared); + assert!(released(|| state.dir().join("holder-ready").exists())); + assert!(locked.claim_held(&journal).unwrap()); + #[cfg(target_os = "linux")] + { + let owner = locked.endpoint_owner(&journal).unwrap(); + assert_eq!(owner.reference().pid, child.0.as_ref().unwrap().id()); + assert!(!owner.has_exited().unwrap()); + } + drop(child); + assert!(released(|| !locked.claim_held(&journal).unwrap())); +} + +#[cfg(target_os = "linux")] +#[test] +fn a_conflicting_lock_prevents_publishing_a_launch() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let other = fs::File::open(prepared.outputs().log.clone()).unwrap(); + other.try_lock().unwrap(); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + let error = locked.begin_launch(&prepared, &runtime).unwrap_err(); + assert!(error.source().is_some()); + assert!(locked.journal().unwrap().is_none()); + assert!(locked.capability().unwrap().is_none()); +} + +#[cfg(target_os = "linux")] +#[test] +fn an_endpoint_owner_without_the_claimed_log_is_rejected_without_signaling_it() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + drop(locked.begin_launch(&prepared, &runtime).unwrap()); + let journal = locked.journal().unwrap().unwrap(); + let mut other = Other( + helper_command(&state) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .unwrap(), + ); + assert!(released(|| state.dir().join("holder-ready").exists())); + assert!(locked.claim_held(&journal).unwrap()); + let error = locked.endpoint_owner(&journal).err().unwrap(); + assert!(error.to_string().contains("does not hold the launch log")); + assert!(other.0.try_wait().unwrap().is_none()); + drop(prepared); + assert!(released(|| !locked.claim_held(&journal).unwrap())); + assert!(locked.endpoint_owner(&journal).is_err()); + assert!(other.0.try_wait().unwrap().is_none()); +} + +#[cfg(windows)] +#[test] +fn without_a_log_job_inherited_handles_remain_the_claim() { + let (_directory, state) = state(); + let (prepared, runtime) = prepared(&state, ConsoleAttachment::Disconnected); + let mut locked = state.lock(Deadline::after(10.0)).unwrap(); + drop(locked.begin_launch(&prepared, &runtime).unwrap()); + let journal = locked.journal().unwrap().unwrap(); + let mut command = helper_command(&state); + command + .stdout(Stdio::from(prepared.log().try_clone().unwrap())) + .stderr(Stdio::from(prepared.log().try_clone().unwrap())); + let child = Other(spawn_locked(&mut command).unwrap()); + drop(command); + drop(prepared); + assert!(released(|| state.dir().join("holder-ready").exists())); + assert!(locked.claim_held(&journal).unwrap()); + drop(child); + assert!(released(|| !locked.claim_held(&journal).unwrap())); +} + +#[test] +fn endpoint_ownership_is_rechecked_only_after_the_server_is_held() { + let pid = std::process::id(); + let held = std::cell::Cell::new(false); + let mut lookups = 0; + let owner = hold_endpoint_server( + || { + lookups += 1; + assert_eq!(held.get(), lookups == 2); + Ok(Some(pid)) + }, + |found| { + assert_eq!(found, pid); + held.set(true); + Ok(Some(own_process())) + }, + ) + .unwrap(); + assert_eq!(owner.reference().pid, pid); + assert_eq!(lookups, 2); + for replacement in [None, Some(pid.wrapping_add(1))] { + let mut lookups = 0; + let error = hold_endpoint_server( + || { + lookups += 1; + Ok(if lookups == 1 { Some(pid) } else { replacement }) + }, + |_| Ok(Some(own_process())), + ) + .err() + .unwrap(); + assert!(error.to_string().contains("endpoint changed")); + assert_eq!(lookups, 2); + } +} + +#[test] +fn missing_owners_and_lookup_or_hold_errors_are_not_exit_proofs() { + assert!(hold_endpoint_server(|| Ok(None), |_| panic!("no owner was found")).is_err()); + assert!(hold_endpoint_server(|| Ok(Some(1)), |_| Ok(None)).is_err()); + for phase in 0..3 { + let mut lookups = 0; + let error = hold_endpoint_server( + || { + lookups += 1; + if (phase == 0 && lookups == 1) || (phase == 2 && lookups == 2) { + Err(io::Error::from(io::ErrorKind::PermissionDenied)) + } else { + Ok(Some(std::process::id())) + } + }, + |_| { + if phase == 1 { + Err(io::Error::from(io::ErrorKind::PermissionDenied)) + } else { + Ok(Some(own_process())) + } + }, + ) + .err() + .unwrap(); + let source = error.source().unwrap().downcast_ref::().unwrap(); + assert_eq!(source.kind(), io::ErrorKind::PermissionDenied); + } +} + +#[test] +#[ignore = "invoked as an owned subprocess by the launch-claim tests"] +fn log_holder_helper() { + #[cfg(target_os = "linux")] + let _listener = std::os::unix::net::UnixListener::bind("control.sock").unwrap(); + fs::write("holder-ready", b"").unwrap(); + thread::sleep(Duration::from_secs(30)); +} diff --git a/nvx-rs/src/host/instance/locked.rs b/nvx-rs/src/host/instance/locked.rs index fca2a638b..2cbee7f48 100644 --- a/nvx-rs/src/host/instance/locked.rs +++ b/nvx-rs/src/host/instance/locked.rs @@ -216,7 +216,8 @@ impl LockedInstance { }; Ok(Stopped { forced, - report: report::read(&self.instance.report_path()), + // This legacy wrapper keeps its best-effort report policy. + report: report::read(&self.instance.report_path()).ok().flatten(), }) } diff --git a/nvx-rs/src/host/platform/windows/tests.rs b/nvx-rs/src/host/platform/windows/tests.rs index 44dd7359a..7da84f781 100644 --- a/nvx-rs/src/host/platform/windows/tests.rs +++ b/nvx-rs/src/host/platform/windows/tests.rs @@ -16,7 +16,8 @@ use windows_sys::Win32::Foundation::{ use windows_sys::Win32::Security::SECURITY_ATTRIBUTES; use windows_sys::Win32::System::JobObjects::CreateJobObjectW; use windows_sys::Win32::System::Threading::{ - CreateEventW, GetCurrentProcess, INFINITE, OpenProcess, PROCESS_DUP_HANDLE, TerminateProcess, + CreateEventW, GetCurrentProcess, INFINITE, OpenProcess, PROCESS_DUP_HANDLE, + PROCESS_QUERY_LIMITED_INFORMATION, PROCESS_SYNCHRONIZE, TerminateProcess, }; pub(crate) use super::endpoint::tests::{accept_control, control_server}; @@ -38,6 +39,16 @@ pub(crate) fn wait_exit_handle(process: OwnedHandle, deadline: Instant) -> io::R .map(|exited| exited.expect("Windows process handles always provide an exit event")) } +impl ProcessHandle { + pub(crate) fn wait_only(pid: u32) -> Self { + Self { + process: open_handle(pid, PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_SYNCHRONIZE) + .unwrap(), + terminate: false, + } + } +} + thread_local! { /// What a spawn of this thread runs in its window, once its handles are inheritable and /// before it creates its process. @@ -1084,7 +1095,7 @@ fn a_launch_whose_vmm_exited_ends_while_a_process_spawned_in_its_window_holds_it let vmm = launch.record_process(&launched).unwrap(); launched.detach(); assert!( - vmm.wait(Instant::now() + WINDOW_TIMEOUT).unwrap(), + vmm.wait(Instant::now() + WINDOW_TIMEOUT).unwrap().is_some(), "the stand-in did not exit" ); // The VMM has exited, so the launch ends, whatever the other process holds. diff --git a/nvx-rs/src/host/process.rs b/nvx-rs/src/host/process.rs index 32047f8d6..a24ed8776 100644 --- a/nvx-rs/src/host/process.rs +++ b/nvx-rs/src/host/process.rs @@ -2,10 +2,11 @@ //! //! 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`] -//! checks a reference against the host and holds the process that it names through a handle of -//! the operating system: a pidfd on Linux, and a process handle on Windows. Waits and termination -//! go through that handle, so they reach the referenced process, whether or not the caller -//! started it, and never another process that reuses its ID. A process that does not match its +//! checks a reference against the host and returns [`Opened::Running`] with the process held +//! through a handle of the operating system, or [`Opened::Exited`] with proof that the recorded +//! process exited. The handle is a pidfd on Linux, and a process handle on Windows. Waits and +//! termination go through that handle, so they reach the referenced process, whether or not the +//! caller started it, and never another process that reuses its ID. A process that does not match its //! reference is never signaled, and neither is one that a reference from an earlier boot names: //! every process of that boot has exited. //! @@ -188,8 +189,23 @@ pub struct Process { handle: platform::ProcessHandle, } +/// The result of checking a recorded process reference against this host. +#[derive(Debug)] +pub enum Opened { + /// The process matched the recorded boot, ID, and start time; the caller owns its `Process`. + Running(Process), + /// The recorded process exited: its boot ended, or no running process matches its identity. + Exited(Exited), +} + +/// Proof that a verified process exited, produced only by [`Process::wait`] or [`Process::open`]. +/// +/// It proves exit, not the process's exit status, and cannot be constructed by a caller. +#[derive(Debug)] +pub struct Exited(()); + impl Process { - /// Finds the process that `reference` names, and returns `None` if it has exited. + /// Finds the process that `reference` names, or returns proof that it has exited. /// /// The process has exited when no process has its ID, when the process that has it started /// at another time, and when the reference comes from another boot. @@ -201,12 +217,15 @@ impl Process { /// other users. Fails with [`io::ErrorKind::InvalidInput`] for a process ID that the host /// never assigns, such as 0, and with [`io::ErrorKind::Unsupported`] on hosts other than /// Linux and Windows. - pub fn open(reference: &ProcessRef) -> io::Result> { + pub fn open(reference: &ProcessRef) -> io::Result { let handle = open_reference(reference, BootId::current, platform::open_process)?; - Ok(handle.map(|handle| Self { - reference: reference.clone(), - handle, - })) + Ok(match handle { + Some(handle) => Opened::Running(Self { + reference: reference.clone(), + handle, + }), + None => Opened::Exited(Exited(())), + }) } /// Returns the process that `handle` holds, whose identity the caller verified to be @@ -241,13 +260,17 @@ impl Process { }) } - /// Waits until the process exits or `deadline` passes, and returns whether it exited. + /// Waits until the process exits or `deadline` passes, returning exit proof or `None`. /// /// A process exits with its last thread. The wait sleeps until the exit event of the /// process, or with [`WaitMethod::Polling`], checks its identity at a short interval. An /// error means that the wait cannot determine whether the process exited. - pub fn wait(&self, deadline: Instant) -> io::Result { - self.wait_bool(deadline) + pub fn wait(&self, deadline: Instant) -> io::Result> { + Ok(if self.wait_bool(deadline)? { + Some(Exited(())) + } else { + None + }) } /// Keeps the boolean wait result for the legacy lifecycle callers. @@ -262,7 +285,7 @@ impl Process { /// Returns whether the process has exited, without waiting. pub fn has_exited(&self) -> io::Result { - self.wait(Instant::now()) + self.wait(Instant::now()).map(|exited| exited.is_some()) } /// Ends the process at once, unless it has exited or is exiting: with `SIGKILL` on Linux, and diff --git a/nvx-rs/src/host/process/tests.rs b/nvx-rs/src/host/process/tests.rs index ebc305ffa..85f4b54d8 100644 --- a/nvx-rs/src/host/process/tests.rs +++ b/nvx-rs/src/host/process/tests.rs @@ -182,6 +182,21 @@ fn sleeper() -> std::process::Command { #[cfg(any(target_os = "linux", windows))] struct ExitChild(std::process::Child); +#[cfg(any(target_os = "linux", windows))] +impl ExitChild { + fn spawn() -> Self { + Self(crate::host::tests::spawn_locked(&mut sleeper()).unwrap()) + } + + fn reference(&self) -> ProcessRef { + ProcessRef { + pid: self.0.id(), + start_time: platform::process_start_time(self.0.id()).unwrap().unwrap(), + boot: BootId::current().unwrap(), + } + } +} + #[cfg(any(target_os = "linux", windows))] impl Drop for ExitChild { fn drop(&mut self) { @@ -224,7 +239,9 @@ fn exit_handles_are_independently_owned_and_exist_only_for_exit_events() { start_time: platform::process_start_time(child.0.id()).unwrap().unwrap(), boot: BootId::current().unwrap(), }; - let process = Process::open(&reference).unwrap().unwrap(); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the child runs"); + }; let handle = process.exit_handle().unwrap(); assert_eq!(handle.is_some(), process.wait_method() == WaitMethod::Event); if handle.is_some() { @@ -239,6 +256,47 @@ fn exit_handles_are_independently_owned_and_exist_only_for_exit_events() { } } +#[cfg(any(target_os = "linux", windows))] +#[test] +fn opening_a_recorded_child_returns_running_only_for_its_identity() { + let mut child = ExitChild::spawn(); + let reference = child.reference(); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the recorded child runs"); + }; + assert_eq!(process.reference(), &reference); + assert!(!process.has_exited().unwrap()); + let mut other_boot = *reference.boot.as_bytes(); + other_boot[0] ^= 1; + for exited in [ + ProcessRef { + boot: BootId::from_bytes(other_boot), + ..reference.clone() + }, + ProcessRef { + start_time: reference.start_time + 1, + ..reference.clone() + }, + ] { + assert!(matches!(Process::open(&exited).unwrap(), Opened::Exited(_))); + } + assert!(child.0.try_wait().unwrap().is_none()); + assert!(!process.has_exited().unwrap()); +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn opening_a_reaped_recorded_child_proves_exit() { + let mut child = ExitChild::spawn(); + let reference = child.reference(); + child.0.kill().unwrap(); + child.0.wait().unwrap(); + assert!(matches!( + Process::open(&reference).unwrap(), + Opened::Exited(_) + )); +} + #[cfg(any(target_os = "linux", windows))] #[test] fn polling_follows_a_real_process_until_it_exits() { @@ -275,7 +333,7 @@ fn hosts_without_pidfds_poll_and_never_signal() { let still_running = process.has_exited(); child.kill().unwrap(); child.wait().unwrap(); - assert!(!running.unwrap()); + assert!(running.unwrap().is_none()); assert_eq!(refused.unwrap_err().kind(), io::ErrorKind::Unsupported); assert!(!still_running.unwrap()); // Once the child is reaped, its ID names no process, or another one. @@ -283,6 +341,7 @@ fn hosts_without_pidfds_poll_and_never_signal() { process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); } @@ -346,7 +405,9 @@ fn processes_count_the_bytes_that_their_read_calls_read() { start_time: platform::process_start_time(child.id()).unwrap().unwrap(), boot: BootId::current().unwrap(), }; - let process = Process::open(&reference).unwrap().unwrap(); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the reader runs"); + }; let counted = process.bytes_read(); let mut stdin = child.stdin.take().unwrap(); stdin.write_all(b"x\n").unwrap(); diff --git a/nvx-rs/src/host/process/tests/linux.rs b/nvx-rs/src/host/process/tests/linux.rs index 4c9177b53..50bb942b0 100644 --- a/nvx-rs/src/host/process/tests/linux.rs +++ b/nvx-rs/src/host/process/tests/linux.rs @@ -144,21 +144,23 @@ fn a_process_runs_until_its_last_thread_exits() { thread::sleep(POLL_INTERVAL); } assert_eq!(platform::process_start_time(pid).unwrap(), Some(start_time)); - let process = Process::open(&ProcessRef { + let Opened::Running(process) = Process::open(&ProcessRef { pid, start_time, boot: BootId::current().unwrap(), }) - .unwrap() - .expect("a process whose main thread exited still runs"); + .unwrap() else { + panic!("a process whose main thread exited still runs"); + }; assert!( - !process + process .wait(Instant::now() + Duration::from_millis(100)) .unwrap() + .is_none() ); child.0.kill().unwrap(); child.0.wait().unwrap(); - assert!(process.wait(Instant::now()).unwrap()); + assert!(process.wait(Instant::now()).unwrap().is_some()); } #[test] @@ -196,13 +198,16 @@ fn a_process_whose_command_name_is_not_utf8_can_be_examined() { start_time: platform::process_start_time(pid).unwrap().unwrap(), boot: BootId::current().unwrap(), }; - assert!(Process::open(&reference).unwrap().is_some()); + assert!(matches!( + Process::open(&reference).unwrap(), + Opened::Running(_) + )); // A reference to a process that exited and whose ID this one reused names no process. let stale = ProcessRef { start_time: reference.start_time + 1, ..reference }; - assert!(Process::open(&stale).unwrap().is_none()); + assert!(matches!(Process::open(&stale).unwrap(), Opened::Exited(_))); child.0.kill().unwrap(); child.0.wait().unwrap(); } diff --git a/nvx-rs/src/host/report.rs b/nvx-rs/src/host/report.rs index 48f86fec4..f07bf2ee4 100644 --- a/nvx-rs/src/host/report.rs +++ b/nvx-rs/src/host/report.rs @@ -9,6 +9,8 @@ use std::path::Path; use serde_json::Value; +use crate::{Error, Result}; + /// Summary of an outcome report. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct Report { @@ -20,9 +22,24 @@ pub struct Report { pub teardown_complete: Option, } -/// Reads the report at `path`, or returns `None` if it is missing or not JSON. -pub fn read(path: &Path) -> Option { - let report: Value = serde_json::from_slice(&fs::read(path).ok()?).ok()?; +/// Reads the report at `path`, or returns `Ok(None)` if it is missing. +/// +/// An unreadable or malformed report fails with [`ErrorKind::Io`](crate::ErrorKind::Io), +/// retaining the underlying filesystem or JSON error. +pub fn read(path: &Path) -> Result> { + let bytes = match fs::read(path) { + Ok(bytes) => bytes, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) => { + return Err( + Error::io(format!("cannot read outcome report {}", path.display())) + .with_source(error), + ); + } + }; + let report: Value = serde_json::from_slice(&bytes).map_err(|error| { + Error::io(format!("cannot parse outcome report {}", path.display())).with_source(error) + })?; let mut summary = Report::default(); if let Some(outcome) = report.get("outcome") { summary.category = outcome @@ -35,7 +52,7 @@ pub fn read(path: &Path) -> Option { let complete = teardown.values().all(|step| step.as_bool() == Some(true)); summary.teardown_complete = Some(complete); } - Some(summary) + Ok(Some(summary)) } #[cfg(test)] diff --git a/nvx-rs/src/host/report/tests.rs b/nvx-rs/src/host/report/tests.rs index 127fd5b2e..961377aa4 100644 --- a/nvx-rs/src/host/report/tests.rs +++ b/nvx-rs/src/host/report/tests.rs @@ -11,7 +11,7 @@ fn reports_are_summarized_without_free_form_fields() { ) .unwrap(); assert_eq!( - read(&path).unwrap(), + read(&path).unwrap().unwrap(), Report { category: Some("success".to_owned()), status_code: Some(0), @@ -20,13 +20,60 @@ fn reports_are_summarized_without_free_form_fields() { ); fs::write(&path, r#"{"teardown":{"workers":true,"memory":false}}"#).unwrap(); assert_eq!( - read(&path).unwrap(), + read(&path).unwrap().unwrap(), Report { teardown_complete: Some(false), ..Report::default() } ); - assert!(read(&directory.path().join("missing.json")).is_none()); +} + +#[test] +fn missing_reports_are_distinct_from_malformed_or_unreadable_reports() { + let directory = tempfile::tempdir().unwrap(); + assert!( + read(&directory.path().join("missing.json")) + .unwrap() + .is_none() + ); + let path = directory.path().join("outcome.json"); fs::write(&path, b"{ truncated").unwrap(); - assert!(read(&path).is_none()); + let malformed = read(&path).unwrap_err(); + assert_eq!(malformed.kind(), crate::ErrorKind::Io); + assert!( + std::error::Error::source(&malformed) + .unwrap() + .is::() + ); + let unreadable = read(directory.path()).unwrap_err(); + assert_eq!(unreadable.kind(), crate::ErrorKind::Io); + assert!( + std::error::Error::source(&unreadable) + .unwrap() + .is::() + ); +} + +#[cfg(windows)] +#[test] +fn a_report_opened_without_read_sharing_returns_its_io_error() { + use std::os::windows::fs::OpenOptionsExt; + + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("outcome.json"); + fs::write(&path, b"{}").unwrap(); + let held = fs::OpenOptions::new() + .read(true) + .share_mode(0) + .open(&path) + .unwrap(); + let error = read(&path).unwrap_err(); + assert_eq!(error.kind(), crate::ErrorKind::Io); + assert!( + std::error::Error::source(&error) + .unwrap() + .is::() + ); + drop(held); + assert_eq!(read(&path).unwrap(), Some(Report::default())); } diff --git a/nvx-rs/src/host/restore/tests.rs b/nvx-rs/src/host/restore/tests.rs index b302590b0..f163d92da 100644 --- a/nvx-rs/src/host/restore/tests.rs +++ b/nvx-rs/src/host/restore/tests.rs @@ -12,7 +12,7 @@ use crate::activation::{ OverlayMode, ReadyActivation, RestoreSection, TransactionId, Upper, View, }; use crate::host::platform::inherit::spawn_command; -use crate::host::process::BootId; +use crate::host::process::{BootId, Opened}; use crate::host::vm::create_private_dir; use crate::protocol::restore_ready::parse_ready_event; @@ -25,13 +25,15 @@ fn vm_directory() -> (tempfile::TempDir, VmDir) { } fn held_process(pid: u32) -> Process { - Process::open(&ProcessRef { + let Opened::Running(process) = Process::open(&ProcessRef { pid, start_time: platform::process_start_time(pid).unwrap().unwrap(), boot: BootId::current().unwrap(), }) - .unwrap() - .unwrap() + .unwrap() else { + panic!("the fixture process runs"); + }; + process } /// A separate process, held before it may connect, and alive until the test ends it. diff --git a/nvx-rs/src/host/store/locked.rs b/nvx-rs/src/host/store/locked.rs index 91d1dd322..67e6d269a 100644 --- a/nvx-rs/src/host/store/locked.rs +++ b/nvx-rs/src/host/store/locked.rs @@ -20,7 +20,7 @@ use super::{CLAIM_RELEASE_WAIT, LOCK_RETRY_INTERVAL, SandboxStore, record, store use crate::error::{Error, ErrorCode, Result}; use crate::host::filesystem::FileIdentity; use crate::host::platform; -use crate::host::process::{BootId, Process, ProcessRef}; +use crate::host::process::{BootId, Opened, Process, ProcessRef}; use crate::protocol::CAPABILITY_LEN; /// The store of a sandbox whose lock this holds, through which its lifecycle transitions change @@ -357,7 +357,12 @@ impl LockedStore { /// Ends the launch that `journal` records, and removes its files once nothing of it runs. pub(super) fn end_launch(&self, journal: &LaunchJournal) -> Result { let recovery = match journal.process() { - Some(process) => self.end_identified(journal, process, Process::open), + Some(process) => self.end_identified(journal, process, |reference| { + Ok(match Process::open(reference)? { + Opened::Running(process) => Some(process), + Opened::Exited(_) => None, + }) + }), None => self.end_unidentified(journal), }; if !matches!(recovery, Recovery::Indeterminate(_)) { diff --git a/nvx-rs/src/host/store/vmm.rs b/nvx-rs/src/host/store/vmm.rs index 4453d2fa6..f4a66918c 100644 --- a/nvx-rs/src/host/store/vmm.rs +++ b/nvx-rs/src/host/store/vmm.rs @@ -5,7 +5,7 @@ use std::io; use std::time::Instant; use crate::host::platform; -use crate::host::process::{BootId, KILL_TIMEOUT, Process, ProcessRef}; +use crate::host::process::{BootId, KILL_TIMEOUT, Opened, Process, ProcessRef}; /// Finds the live process `pid` of the current boot and holds it, or returns `None` if it has /// exited. @@ -16,10 +16,14 @@ pub(super) fn find(pid: u32) -> io::Result> { let Some(start_time) = platform::process_start_time(pid)? else { return Ok(None); }; - Process::open(&ProcessRef { + let opened = Process::open(&ProcessRef { pid, start_time, boot: BootId::current()?, + })?; + Ok(match opened { + Opened::Running(process) => Some(process), + Opened::Exited(_) => None, }) } diff --git a/nvx-rs/src/host/supervisor/reattach.rs b/nvx-rs/src/host/supervisor/reattach.rs index 5563c4269..c04037bf9 100644 --- a/nvx-rs/src/host/supervisor/reattach.rs +++ b/nvx-rs/src/host/supervisor/reattach.rs @@ -5,7 +5,7 @@ use std::io; use super::vmm::Vmm; use crate::error::Result; -use crate::host::process::{Process, ProcessRef}; +use crate::host::process::{Opened, Process, ProcessRef}; use crate::host::store::{ IdentityCheck, IdentityPart, LockedStore, Recovery, RuntimeIdentity, RuntimeRecord, SandboxStore, @@ -65,7 +65,7 @@ pub enum Reattached { /// 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, |_| Ok(())) + reattach_reporting(locked, runtime, |_| Ok(())) } /// Reattaches as [`reattach`] does, and calls `exited` with the runtime record of a VMM that it @@ -77,7 +77,17 @@ pub(crate) fn reattach_reporting( runtime: &RuntimeIdentity, exited: impl FnOnce(&RuntimeRecord) -> Result<()>, ) -> Result { - reattach_with(locked, runtime, Process::open, exited) + reattach_with( + locked, + runtime, + |reference| { + Ok(match Process::open(reference)? { + Opened::Running(process) => Some(process), + Opened::Exited(_) => None, + }) + }, + exited, + ) } /// Reattaches as [`reattach_reporting`] does, holding the recorded VMM with `open`, which is diff --git a/nvx-rs/src/host/template/clone/records.rs b/nvx-rs/src/host/template/clone/records.rs index ce9752576..dc5fbc696 100644 --- a/nvx-rs/src/host/template/clone/records.rs +++ b/nvx-rs/src/host/template/clone/records.rs @@ -296,7 +296,7 @@ impl Recorder { /// report only once the clone's VM ended, or its teardown did. pub(super) fn observed(&self) -> Option<&Report> { self.observed - .get_or_init(|| report::read(&self.report)) + .get_or_init(|| report::read(&self.report).ok().flatten()) .as_ref() } @@ -1119,7 +1119,7 @@ impl CloneRecords { // The launch removed the report of an earlier VM of the sandbox before it wrote the first // of the clone's records, which a progress past the plan shows. let report = (progress > Progress::Planned) - .then(|| report::read(&sandbox.outcome_path())) + .then(|| report::read(&sandbox.outcome_path()).ok().flatten()) .flatten(); Ok(Some(Self { lease, diff --git a/nvx-rs/src/host/template/clone/tests/launch.rs b/nvx-rs/src/host/template/clone/tests/launch.rs index c58043a4c..ccd088aa8 100644 --- a/nvx-rs/src/host/template/clone/tests/launch.rs +++ b/nvx-rs/src/host/template/clone/tests/launch.rs @@ -18,7 +18,7 @@ use super::*; use crate::activation::{ReadyActivation, StoreGeneration, TransactionId}; use crate::host::artifacts; use crate::host::platform::Transport; -use crate::host::process::{BootId, Process, ProcessRef}; +use crate::host::process::{BootId, Opened, Process, ProcessRef}; use crate::host::snapshot::tests::artifact; use crate::host::store::files::{sync_dir, write_private}; use crate::host::store::{ @@ -361,18 +361,18 @@ impl VmmDir { /// before it asks, since a process that has exited no longer opens. pub(super) fn end_and_wait(&self) { let reference = self.reference.as_ref().expect("the test knows the VMM"); - let process = Process::open(reference) - .unwrap() - .expect("the VMM runs until the test ends it"); + let Opened::Running(process) = Process::open(reference).unwrap() else { + panic!("the VMM runs until the test ends it"); + }; self.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } /// Waits until the VMM, which exits on its own, as its [plan](Self::plan) asks, has exited. pub(super) fn wait_exit(&self) { let reference = self.reference.as_ref().expect("the test knows the VMM"); - if let Some(process) = Process::open(reference).unwrap() { - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + if let Opened::Running(process) = Process::open(reference).unwrap() { + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } } @@ -412,8 +412,10 @@ impl Drop for VmmDir { fn drop(&mut self) { let _ = fs::write(self.path().join(EXIT_MARKER), b""); if let Some(reference) = &self.reference - && let Ok(Some(process)) = Process::open(reference) - && !process.wait(Instant::now() + TIMEOUT).unwrap_or(false) + && let Ok(Opened::Running(process)) = Process::open(reference) + && !process + .wait(Instant::now() + TIMEOUT) + .is_ok_and(|exited| exited.is_some()) { let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); @@ -1567,7 +1569,7 @@ fn a_clone_whose_launch_failed_before_it_began_launches_no_more_until_its_releas // clone's: an inspection takes none, and the launch removes it before it records anything. let report = acquired.locked.outcome_path(); fs::write(&report, guest_exit(195)).unwrap(); - assert!(crate::host::report::read(&report).is_some()); + assert!(crate::host::report::read(&report).unwrap().is_some()); let records = CloneRecords::read(&acquired.locked).unwrap().unwrap(); assert_eq!( (records.progress, records.report), @@ -2299,7 +2301,9 @@ fn a_reattach_takes_the_report_of_a_vmm_whose_launch_a_recovery_of_the_sandbox_e fn a_launch_whose_caller_crashes_once_its_vmm_started_ends_the_vmm_as_reattach_recovers_it() { let crashed = Crashed::at("launched"); let reference = crashed.vmm.reference.clone().unwrap(); - let process = Process::open(&reference).unwrap().unwrap(); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the crashed caller's VMM runs"); + }; // The VMM listens beside the snapshot on Linux, as it has once it tried to send its event. crashed.vmm.started(); assert_eq!(exists(&crashed.socket()), cfg!(target_os = "linux")); @@ -2323,7 +2327,7 @@ fn a_launch_whose_caller_crashes_once_its_vmm_started_ends_the_vmm_as_reattach_r } => assert_eq!(terminated, reference), other => panic!("unexpected reattach: {other:?}"), } - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(!exists(&crashed.socket())); crashed.releases(&mut locked); } @@ -2336,7 +2340,9 @@ fn a_restore_whose_caller_crashes_before_its_commit_ends_its_vmm_as_reattach_rec for step in ["activating", "activated", "starting", "started"] { let crashed = Crashed::at(step); let reference = crashed.vmm.reference.clone().unwrap(); - let process = Process::open(&reference).unwrap().unwrap(); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the crashed caller's VMM runs"); + }; // The caller read the VMM's restore-ready event, whose flush waited for that on Windows. assert!(crashed.vmm.started().ends_with(" true"), "{step}"); assert!(crashed.vmm.flushed(), "{step}"); @@ -2356,7 +2362,10 @@ fn a_restore_whose_caller_crashes_before_its_commit_ends_its_vmm_as_reattach_rec } => assert_eq!(terminated, reference, "{step}"), other => panic!("unexpected reattach at {step}: {other:?}"), } - assert!(process.wait(Instant::now() + TIMEOUT).unwrap(), "{step}"); + assert!( + process.wait(Instant::now() + TIMEOUT).unwrap().is_some(), + "{step}" + ); assert!(!exists(&crashed.socket()), "{step}"); crashed.releases(&mut locked); } @@ -3658,7 +3667,10 @@ fn a_reattach_that_cannot_record_a_failure_ends_and_forgets_nothing_of_the_clone .journal() .unwrap() .expect("the reattach kept the journal"); - assert!(Process::open(journal.process().unwrap()).unwrap().is_some()); + assert!(matches!( + Process::open(journal.process().unwrap()).unwrap(), + Opened::Running(_) + )); crashed.refuses_release(&mut locked); // Once the record can be written, the reattach ends the launch, and records its failure. fs::remove_dir(&path).unwrap(); @@ -3714,7 +3726,7 @@ fn a_reattach_that_cannot_record_a_failure_ends_and_forgets_nothing_of_the_clone .unwrap() .expect("the reattach kept the record"); assert!( - Process::open(record.process()).unwrap().is_some(), + matches!(Process::open(record.process()).unwrap(), Opened::Running(_)), "{evidence:?}" ); crashed.refuses_release(&mut locked); @@ -3770,7 +3782,10 @@ fn a_reattach_makes_the_record_of_an_earlier_failure_durable_before_it_ends_the_ .journal() .unwrap() .expect("the reattach kept the journal"); - assert!(Process::open(journal.process().unwrap()).unwrap().is_some()); + assert!(matches!( + Process::open(journal.process().unwrap()).unwrap(), + Opened::Running(_) + )); crashed.refuses_release(&mut locked); // One that can syncs the sandbox's directory before it ends the launch. let synced = RefCell::new(Vec::new()); diff --git a/nvx-rs/src/host/template/clone/tests/restore.rs b/nvx-rs/src/host/template/clone/tests/restore.rs index 0a33cb1dd..54d8f45eb 100644 --- a/nvx-rs/src/host/template/clone/tests/restore.rs +++ b/nvx-rs/src/host/template/clone/tests/restore.rs @@ -20,7 +20,7 @@ use super::launch::{ use super::*; use crate::activation::{ReadyActivation, RestoreSection, TransactionId}; use crate::host::platform::Transport; -use crate::host::process::{BootId, Process}; +use crate::host::process::{BootId, Opened, Process}; use crate::host::store::{CLONE_ACTIVATION_NAME, Recovery}; use crate::host::supervisor::{Starting, StopOutcome}; use crate::host::template::clone::launch::Hooks; @@ -421,8 +421,8 @@ fn vmm_with(plan: &str) -> VmmDir { /// Checks that the VMM of `vmm` has exited. fn assert_vmm_ended(vmm: &VmmDir) { let reference = vmm.reference.as_ref().unwrap(); - if let Some(process) = Process::open(reference).unwrap() { - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + if let Opened::Running(process) = Process::open(reference).unwrap() { + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } } @@ -1193,7 +1193,10 @@ fn a_launch_that_cannot_be_ended_keeps_its_records_and_its_lease_until_a_reattac assert_eq!(refusal.code(), ErrorCode::SandboxRunning, "{refusal}"); let records = CloneRecords::read(locked).unwrap().unwrap(); assert_eq!(records.journal.unwrap().process(), Some(&reference)); - assert!(Process::open(&reference).unwrap().is_some()); + assert!(matches!( + Process::open(&reference).unwrap(), + Opened::Running(_) + )); // A reattach ends the launch through its VMM's verified identity: the record keeps the // failure's stage, evidence, and original outcome, with the teardown that resolved it. let mut control = FakeControl::default(); diff --git a/nvx-rs/src/host/template/store/leases/tests.rs b/nvx-rs/src/host/template/store/leases/tests.rs index 1205e4ba3..2a8b6898d 100644 --- a/nvx-rs/src/host/template/store/leases/tests.rs +++ b/nvx-rs/src/host/template/store/leases/tests.rs @@ -15,7 +15,7 @@ use serde_json::{Value, json}; use super::*; use crate::activation::TemplateReference; -use crate::host::process::{Process, ProcessRef}; +use crate::host::process::{Opened, Process, ProcessRef}; use crate::host::snapshot::tests::{ManifestSpec, artifact, network_class}; use crate::host::store::files::write_private; use crate::host::store::{ @@ -2459,8 +2459,10 @@ impl Drop for VmmDir { fn drop(&mut self) { let _ = fs::write(self.path().join(EXIT_MARKER), b""); if let Some(reference) = &self.reference - && let Ok(Some(process)) = Process::open(reference) - && !process.wait(Instant::now() + TIMEOUT).unwrap_or(false) + && let Ok(Opened::Running(process)) = Process::open(reference) + && !process + .wait(Instant::now() + TIMEOUT) + .is_ok_and(|exited| exited.is_some()) { let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); @@ -2623,7 +2625,9 @@ fn a_clone_that_crashes_before_its_spawn_keeps_its_lease_until_its_launch_ends() fn a_clone_that_crashes_after_its_spawn_keeps_its_lease_until_its_unrecorded_vmm_exits() { let crashed = Crashed::after("after-spawn"); let reference = crashed.vmm.reference.clone().unwrap(); - let process = Process::open(&reference).unwrap().unwrap(); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the unrecorded VMM runs"); + }; let mut locked = crashed.lock(); assert_eq!( locked.journal().unwrap().unwrap().state(), @@ -2639,7 +2643,7 @@ fn a_clone_that_crashes_after_its_spawn_keeps_its_lease_until_its_unrecorded_vmm crashed.refuses_release(&mut locked); // Once the VMM exits, the claim that it released proves it. crashed.vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); match reattach_until_determinate(&mut locked) { Reattached::Exited { record: None, @@ -2654,7 +2658,9 @@ fn a_clone_that_crashes_after_its_spawn_keeps_its_lease_until_its_unrecorded_vmm fn a_clone_that_crashes_after_recording_its_vmm_keeps_its_lease_until_reattach_ends_it() { let crashed = Crashed::after("after-identity"); let reference = crashed.vmm.reference.clone().unwrap(); - let process = Process::open(&reference).unwrap().unwrap(); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the recorded VMM runs"); + }; let mut locked = crashed.lock(); let journal = locked.journal().unwrap().unwrap(); assert_eq!(journal.launch(), crashed.lease.launch()); @@ -2669,7 +2675,7 @@ fn a_clone_that_crashes_after_recording_its_vmm_keeps_its_lease_until_reattach_e } => assert_eq!(terminated, reference), other => panic!("unexpected reattach: {other:?}"), } - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); crashed.releases(&mut locked); } diff --git a/nvx-rs/src/host/transport.rs b/nvx-rs/src/host/transport.rs index 5ab40b105..bd171ded9 100644 --- a/nvx-rs/src/host/transport.rs +++ b/nvx-rs/src/host/transport.rs @@ -47,7 +47,7 @@ use std::thread; use std::time::{Duration, Instant}; use super::platform; -use super::process::{Process, ProcessRef}; +use super::process::{Opened, Process, ProcessRef}; use crate::protocol::CAPABILITY_LEN; use crate::protocol::nvxs::{ ErrorCode, HostEvent, HostSession, Identity, RecordType, SessionError, @@ -76,9 +76,12 @@ pub fn connect_verified( reference: &ProcessRef, deadline: Instant, ) -> Result> { - let process = Process::open(reference) + let process = match Process::open(reference) .map_err(|error| Error::io("cannot verify the endpoint's VM process").with_source(error))? - .ok_or_else(exited)?; + { + Opened::Running(process) => process, + Opened::Exited(_) => return Err(exited()), + }; let mut unavailable = None; loop { check_running(&process)?; diff --git a/nvx-rs/src/host/transport/tests.rs b/nvx-rs/src/host/transport/tests.rs index 2136f8fbb..eb449723e 100644 --- a/nvx-rs/src/host/transport/tests.rs +++ b/nvx-rs/src/host/transport/tests.rs @@ -166,13 +166,10 @@ fn another_process_serving_the_endpoint_is_refused() { assert!(source.to_string().contains(&std::process::id().to_string())); assert!(source.to_string().contains(&process.pid.to_string())); } - assert!( - !Process::open(&process) - .unwrap() - .unwrap() - .has_exited() - .unwrap() - ); + let Opened::Running(held) = Process::open(&process).unwrap() else { + panic!("the unrelated VM process still runs"); + }; + assert!(!held.has_exited().unwrap()); } } diff --git a/nvx-rs/src/host/vm.rs b/nvx-rs/src/host/vm.rs index 1429a7a26..142ca7c0c 100644 --- a/nvx-rs/src/host/vm.rs +++ b/nvx-rs/src/host/vm.rs @@ -16,6 +16,7 @@ use std::fmt; use std::fs::{self, File, OpenOptions}; use std::io; use std::path::{Path, PathBuf}; +use std::time::Instant; use crate::host::config::{ConsoleAttachment, RootConfig, UpperConfig, VmConfig}; use crate::host::filesystem; @@ -25,11 +26,15 @@ use crate::host::platform; use crate::host::platform::{check_store_dir as check_dir, create_store_dir as create_dir}; #[cfg(windows)] use crate::host::platform::{check_vm_dir as check_dir, create_vm_dir as create_dir}; +use crate::host::report::{self, Report}; use crate::host::runtime::{Runtime, RuntimeIdentity}; use crate::host::snapshot::Snapshot; use crate::host::spawn::{self, VmmCommand}; use crate::host::transport::{self, HostConnection}; -use crate::host::{platform::inherit::Inherited, process::Process, process::ProcessRef}; +use crate::host::{ + platform::inherit::Inherited, + process::{Opened, Process, ProcessRef}, +}; use crate::protocol::CAPABILITY_LEN; use crate::{Error, Result}; @@ -250,12 +255,16 @@ pub fn verify(saved: &SavedVm, runtime: &RuntimeIdentity) -> Result { ), )); } - Process::open(&saved.process) + match Process::open(&saved.process) .map_err(|error| Error::io("cannot verify the saved VM process").with_source(error))? - .ok_or_else(|| { + { + Opened::Running(process) => Ok(process), + Opened::Exited(_) => { let message = "the saved VM process exited"; - Error::exited(message).with_source(io::Error::new(io::ErrorKind::NotFound, message)) - }) + Err(Error::exited(message) + .with_source(io::Error::new(io::ErrorKind::NotFound, message))) + } + } } /// A verified held VM process and its authenticated control connection. @@ -522,6 +531,78 @@ pub fn spawn(prepared: Prepared) -> Result { spawn_verified(prepared, spawn::Launched::process) } +/// A confirmed VM exit, with the outcome-report read kept separate from exit proof. +#[derive(Debug)] +pub struct Stopped { + /// Exit was confirmed before any forced termination was attempted. + pub self_exited: bool, + /// Forced termination through the verified process was attempted. + pub forced: bool, + /// The outcome summary, missing report, or report-read error after confirmed exit. + pub report: Result>, +} + +/// Stops a VM, returning only after its verified process has exited. +/// +/// Unless exit is already confirmed, calls `graceful` with `grace`, bounded by `deadline`, +/// then waits until that grace deadline. The callback must respect the supplied deadline; +/// its boolean request result is not exit proof. If exit is still unconfirmed, attempts +/// forced termination and waits until `deadline`. A failed termination is harmless only +/// when that final wait confirms exit. +/// +/// An unconfirmed exit fails with [`ErrorKind::StillRunning`](crate::ErrorKind::StillRunning), +/// retaining host errors. The caller keeps the process in that case. +/// Report-read errors have kind [`ErrorKind::Io`](crate::ErrorKind::Io) +/// inside [`Stopped::report`], never hiding confirmed exit. +pub fn stop( + process: &Process, + outputs: &VmOutputs, + graceful: impl FnOnce(Instant) -> bool, + grace: Instant, + deadline: Instant, +) -> Result { + let failure = |kill_error: Option<&io::Error>| { + let mut message = format!( + "cannot confirm exit of VM process {}", + process.reference().pid + ); + if let Some(error) = kill_error { + message.push_str(&format!("; forced termination failed: {error}")); + } + Error::new(crate::ErrorKind::StillRunning, message) + }; + let wait = |until| { + process + .wait(until) + .map_err(|error| failure(None).with_source(error)) + }; + let self_exited = if wait(Instant::now())?.is_some() { + true + } else { + let grace = grace.min(deadline); + graceful(grace); + wait(grace)?.is_some() + }; + if !self_exited { + let kill_error = process.kill().err(); + let exited = process + .wait(deadline) + .map_err(|error| failure(kill_error.as_ref()).with_source(error))?; + if exited.is_none() { + let error = failure(kill_error.as_ref()); + return Err(match kill_error { + Some(source) => error.with_source(source), + None => error, + }); + } + } + Ok(Stopped { + self_exited, + forced: !self_exited, + report: report::read(&outputs.outcome), + }) +} + /// Keeps the child owned until its process identity is verified, or its exit is confirmed. fn spawn_verified( prepared: Prepared, diff --git a/nvx-rs/src/host/vm/tests.rs b/nvx-rs/src/host/vm/tests.rs index 05af49bd0..8903967cb 100644 --- a/nvx-rs/src/host/vm/tests.rs +++ b/nvx-rs/src/host/vm/tests.rs @@ -11,6 +11,8 @@ use crate::host::config::{ Boot, LayeredRoot, PROCESS_COMMAND_LINE_LIMIT, RootConfig, UpperConfig, process_command_line_units, }; +#[cfg(any(target_os = "linux", windows))] +use crate::host::process::Opened; use crate::host::runtime::{ArtifactIdentity, LaunchOptions, RuntimeIdentity}; use crate::hypervisor::Hypervisor; use crate::protocol::root::RootImage; @@ -901,7 +903,9 @@ fn spawn_returns_the_same_outputs_and_capability_with_a_verified_process() { assert_eq!(launched.0.capability.as_bytes(), &capability); let reference = launched.0.process.reference(); assert_ne!(reference.pid, std::process::id()); - let reopened = Process::open(reference).unwrap().unwrap(); + let Opened::Running(reopened) = Process::open(reference).unwrap() else { + panic!("the recorded VMM runs"); + }; assert_eq!(reopened.reference(), reference); assert!(!reopened.has_exited().unwrap()); assert!(!format!("{:?}", launched.0).contains(&format!("{capability:?}"))); @@ -1073,7 +1077,9 @@ fn a_library_vm_outlives_the_caller_that_spawned_it_without_retaining_its_output ); let reference = serde_json::from_slice(&fs::read(vm.join("caller-process.json")).unwrap()).unwrap(); - let process = Process::open(&reference).unwrap().unwrap(); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the caller's recorded VMM runs"); + }; let launched = RunningVm(Launched { process, outputs: VmDir::open(&vm).unwrap().outputs(false).unwrap(), @@ -1098,6 +1104,7 @@ fn spawn_caller_helper() { .process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); panic!("cannot record the helper's child: {error}"); } @@ -1181,6 +1188,7 @@ fn an_unrelated_process_started_in_the_library_spawn_window_gets_no_vm_claim() { .process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); assert!( platform::tests::claim_released_within(&log, identity, Duration::from_secs(10)), @@ -1231,7 +1239,9 @@ fn current_cold_spawn_boots_a_guest_accepts_shell_input_and_exits_cleanly() { let launched = RunningVm(spawn(prepared).unwrap()); assert_eq!(launched.0.capability.as_bytes(), &capability); let reference = launched.0.process.reference(); - let reopened = Process::open(reference).unwrap().unwrap(); + let Opened::Running(reopened) = Process::open(reference).unwrap() else { + panic!("the recorded VMM runs"); + }; assert_eq!(reopened.reference(), reference); let deadline = Instant::now() + Duration::from_secs(60); let mut console = loop { @@ -1282,10 +1292,195 @@ fn current_cold_spawn_boots_a_guest_accepts_shell_input_and_exits_cleanly() { ) .unwrap(); read_until(&mut *console, b"NVX-SPAWN-COLD-OK"); - assert!(launched.0.process.wait(deadline).unwrap()); + assert!(launched.0.process.wait(deadline).unwrap().is_some()); assert!(reopened.has_exited().unwrap()); - let outcome = crate::host::report::read(&launched.0.outputs.outcome).unwrap(); + let outcome = crate::host::report::read(&launched.0.outputs.outcome) + .unwrap() + .unwrap(); assert_eq!(outcome.category.as_deref(), Some("success")); assert_eq!(outcome.status_code, Some(0)); println!("NVX-LIBRARY-COLD-SPAWN-EXIT-OK: {}", reference.pid); } + +#[cfg(any(target_os = "linux", windows))] +struct StopChild { + child: std::process::Child, + process: Process, +} + +#[cfg(any(target_os = "linux", windows))] +impl StopChild { + fn start() -> Self { + use std::io::{BufRead, BufReader}; + + #[cfg(windows)] + let mut command = { + let mut command = Command::new( + PathBuf::from(std::env::var_os("SystemRoot").unwrap()).join("System32\\cmd.exe"), + ); + command.args(["/d", "/q", "/c", "echo ready & set /p wait="]); + command + }; + #[cfg(target_os = "linux")] + let mut command = { + let mut command = Command::new("sh"); + command.args(["-c", "printf 'ready\\n'; read -r wait"]); + command + }; + command + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + let mut child = crate::host::tests::spawn_locked(&mut command).unwrap(); + let mut ready = String::new(); + BufReader::new(child.stdout.take().unwrap()) + .read_line(&mut ready) + .unwrap(); + assert_eq!(ready.trim_end(), "ready"); + let reference = crate::host::process::ProcessRef { + pid: child.id(), + start_time: platform::process_start_time(child.id()).unwrap().unwrap(), + boot: crate::host::process::BootId::current().unwrap(), + }; + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the owned child runs"); + }; + Self { child, process } + } +} + +#[cfg(any(target_os = "linux", windows))] +impl Drop for StopChild { + fn drop(&mut self) { + if self.child.try_wait().unwrap().is_none() { + self.child.kill().unwrap(); + } + self.child.wait().unwrap(); + } +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn stop_preserves_report_results_after_a_real_self_exit() { + for report in ["missing", "valid", "malformed", "unreadable"] { + let (_directory, vm) = vm_dir(); + let outputs = vm.outputs(false).unwrap(); + match report { + "missing" => {} + "valid" => fs::write( + &outputs.outcome, + br#"{"outcome":{"category":"success","status_code":0},"teardown":{"workers":true}}"#, + ).unwrap(), + "malformed" => fs::write(&outputs.outcome, b"{ truncated").unwrap(), + "unreadable" => fs::create_dir(&outputs.outcome).unwrap(), + _ => unreachable!(), + } + let mut child = StopChild::start(); + let deadline = Instant::now() + Duration::from_secs(10); + let stopped = stop( + &child.process, + &outputs, + |supplied| { + assert_eq!(supplied, deadline); + drop(child.child.stdin.take()); + false + }, + deadline, + deadline, + ) + .unwrap(); + assert!(stopped.self_exited); + assert!(!stopped.forced); + assert!(child.process.wait(Instant::now()).unwrap().is_some()); + assert!(child.child.try_wait().unwrap().is_some()); + let already_exited = stop( + &child.process, + &outputs, + |_| panic!("an exited process needs no graceful request"), + Instant::now(), + Instant::now(), + ) + .unwrap(); + assert!(already_exited.self_exited); + assert!(!already_exited.forced); + match report { + "missing" => assert!(stopped.report.unwrap().is_none()), + "valid" => assert_eq!( + stopped.report.unwrap(), + Some(Report { + category: Some("success".to_owned()), + status_code: Some(0), + teardown_complete: Some(true), + }) + ), + _ => { + let error = stopped.report.unwrap_err(); + assert_eq!(error.kind(), ErrorKind::Io); + assert!(std::error::Error::source(&error).is_some()); + } + } + } +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn stop_forces_only_its_verified_process_after_grace() { + let (_directory, vm) = vm_dir(); + let outputs = vm.outputs(false).unwrap(); + let mut child = StopChild::start(); + let mut other = StopChild::start(); + let grace = Instant::now() + Duration::from_millis(80); + let deadline = grace + Duration::from_secs(10); + let stopped = stop( + &child.process, + &outputs, + |supplied| { + assert_eq!(supplied, grace); + true + }, + grace, + deadline, + ) + .unwrap(); + assert!(!stopped.self_exited); + assert!(stopped.forced); + assert!(stopped.report.unwrap().is_none()); + assert!(child.process.wait(Instant::now()).unwrap().is_some()); + assert!(child.child.try_wait().unwrap().is_some()); + assert!(other.process.wait(Instant::now()).unwrap().is_none()); + assert!(other.child.try_wait().unwrap().is_none()); +} + +#[cfg(any(target_os = "linux", windows))] +#[test] +fn stop_returns_still_running_without_exit_confirmation_at_its_deadline() { + let (_directory, vm) = vm_dir(); + let outputs = vm.outputs(false).unwrap(); + fs::write(&outputs.outcome, b"{ truncated").unwrap(); + let mut child = StopChild::start(); + #[cfg(target_os = "linux")] + let handle = platform::ProcessHandle::without_pidfd(); + #[cfg(windows)] + let handle = platform::ProcessHandle::wait_only(child.child.id()); + let process = Process::held(child.process.reference().clone(), handle); + let grace = Instant::now(); + let deadline = grace + Duration::from_millis(80); + let error = stop(&process, &outputs, |_| false, grace, deadline).unwrap_err(); + assert!(Instant::now() >= deadline); + assert_eq!(error.kind(), ErrorKind::StillRunning); + assert_eq!(error.code().as_str(), "still-running"); + let cause = std::error::Error::source(&error) + .unwrap() + .downcast_ref::() + .unwrap(); + assert_eq!( + cause.kind(), + if cfg!(windows) { + io::ErrorKind::PermissionDenied + } else { + io::ErrorKind::Unsupported + } + ); + assert!(process.wait(Instant::now()).unwrap().is_none()); + assert!(child.child.try_wait().unwrap().is_none()); +} diff --git a/nvx-rs/src/host/vm/tests/reattach.rs b/nvx-rs/src/host/vm/tests/reattach.rs index 88a95d005..0611675b6 100644 --- a/nvx-rs/src/host/vm/tests/reattach.rs +++ b/nvx-rs/src/host/vm/tests/reattach.rs @@ -244,7 +244,7 @@ fn reattaching_returns_the_held_owned_child_and_authenticated_connection() { b"response" ); owner.wait(); - assert!(attached.process.wait(deadline).unwrap()); + assert!(attached.process.wait(deadline).unwrap().is_some()); } #[test] diff --git a/nvx-rs/tests/checked_handoff.rs b/nvx-rs/tests/checked_handoff.rs index b07a9e340..8f1c7b07b 100644 --- a/nvx-rs/tests/checked_handoff.rs +++ b/nvx-rs/tests/checked_handoff.rs @@ -703,6 +703,7 @@ fn a_prepared_launch_hands_the_vmm_its_checked_images_alone() { .unwrap() .wait(Instant::now() + TIMEOUT) .unwrap() + .is_some() ); launched.kill_and_wait(); let scratch = fs::read_to_string(staged.scratch()).unwrap(); @@ -788,6 +789,7 @@ fn a_vmm_keeps_the_scratch_images_claim_once_its_launcher_is_gone() { .unwrap() .wait(Instant::now() + TIMEOUT) .unwrap() + .is_some() ); launched.kill_and_wait(); drop(check_written().unwrap()); diff --git a/nvx-rs/tests/cli.rs b/nvx-rs/tests/cli.rs index f3b64de5b..0c32142f2 100644 --- a/nvx-rs/tests/cli.rs +++ b/nvx-rs/tests/cli.rs @@ -51,7 +51,7 @@ use std::process::{Command, Output, Stdio}; use std::thread; use std::time::{Duration, Instant}; -use nvx::host::process::{Process, ProcessRef}; +use nvx::host::process::{Opened, Process, ProcessRef}; use serde_json::{Value, json}; use self::harness::{FAKE_OPENVMM, NVX_RS, Root, canonical}; @@ -800,7 +800,10 @@ fn runtime_record(root: &Root, state: &str) -> Option { fn recorded_process(root: &Root, state: &str) -> Option { let record = runtime_record(root, state)?; let reference: ProcessRef = serde_json::from_value(record["process"].clone()).unwrap(); - Process::open(&reference).unwrap() + match Process::open(&reference).unwrap() { + Opened::Running(process) => Some(process), + Opened::Exited(_) => None, + } } /// Returns the names in the state directory `state` below the root, sorted. @@ -1007,9 +1010,10 @@ fn a_stop_under_another_identity_never_ends_a_process_that_its_record_does_not_n ); assert_eq!(runtime_record(&root, "state"), None); assert!( - !process + process .wait(Instant::now() + Duration::from_millis(200)) - .unwrap(), + .unwrap() + .is_none(), "{field}" ); process.kill().unwrap(); @@ -1017,6 +1021,7 @@ fn a_stop_under_another_identity_never_ends_a_process_that_its_record_does_not_n process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); } assert_eq!(transition(&root, "deprovision", "state", &[]), succeeded()); @@ -1055,9 +1060,9 @@ fn a_start_that_was_interrupted_is_ended_under_another_identity_too() { // The CLI dies, as a crash would, and leaves its OpenVMM running. start.kill().unwrap(); start.wait().unwrap(); - let process = Process::open(&reference) - .unwrap() - .expect("the OpenVMM of the start runs"); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the OpenVMM of the start runs"); + }; // Under another runtime identity, the next transition ends the start, whose OpenVMM it // verifies through the journal. @@ -1153,6 +1158,7 @@ fn stop_reports_its_timeout_and_leaves_the_vm_running() { process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); assert_eq!( transition(&root, "deprovision", "unanswered", &[]), @@ -1173,6 +1179,7 @@ fn stop_reports_its_timeout_and_leaves_the_vm_running() { process .wait(Instant::now() + Duration::from_secs(30)) .unwrap() + .is_some() ); assert_eq!( transition(&root, "stop", "slow", &[]), @@ -1194,6 +1201,7 @@ fn a_sandbox_whose_openvmm_exited_reports_it_once_and_starts_again() { process .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); }; crash(); @@ -2054,9 +2062,9 @@ fn a_start_that_ends_before_openvmm_opens_the_images_leaves_it_the_checked_ones( // The CLI dies, as a crash would, before OpenVMM has opened its images, and leaves it running. start.kill().unwrap(); start.wait().unwrap(); - let process = Process::open(&reference) - .unwrap() - .expect("the OpenVMM of the start runs"); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the OpenVMM of the start runs"); + }; let blocks = root .path .join("build") diff --git a/nvx-rs/tests/cli/download.rs b/nvx-rs/tests/cli/download.rs index be959f532..f9d758cea 100644 --- a/nvx-rs/tests/cli/download.rs +++ b/nvx-rs/tests/cli/download.rs @@ -171,6 +171,22 @@ fn provision_and_deprovision_need_no_artifact_root() { let executable = elsewhere .path() .join(Path::new(NVX_RS).file_name().unwrap()); + #[cfg(unix)] + { + // Concurrent forks of this test process must inherit no writable executable. + let copied = Command::new("cp") + .arg("-p") + .arg(NVX_RS) + .arg(&executable) + .output() + .unwrap(); + assert!( + copied.status.success(), + "cannot copy the executable: {}", + String::from_utf8_lossy(&copied.stderr) + ); + } + #[cfg(windows)] fs::copy(NVX_RS, &executable).unwrap(); let nvx_rs = |arguments: &[String]| { let output = Command::new(&executable) diff --git a/nvx-rs/tests/cli/lifecycle.rs b/nvx-rs/tests/cli/lifecycle.rs index 7078aba9e..ce2ac6b58 100644 --- a/nvx-rs/tests/cli/lifecycle.rs +++ b/nvx-rs/tests/cli/lifecycle.rs @@ -11,7 +11,7 @@ use std::time::{Duration, Instant}; use nvx::contract::{ BOOT_CONTRACT_VERSION, CONTROL_CONTRACT_REVISION, CONTROL_SESSION_PROTOCOL_VERSION, }; -use nvx::host::process::{Process, ProcessRef}; +use nvx::host::process::{Opened, Process, ProcessRef}; use nvx::host::sandbox::distro::DistroBinding; use nvx::host::sandbox::scratch::ScratchBinding; use nvx::host::sandbox::{LayerRole, SandboxLayer}; @@ -153,9 +153,9 @@ fn assert_runtime_record(root: &Root, state: &str, record: &Value) -> (String, P "{record}" ); let reference: ProcessRef = serde_json::from_value(record["process"].clone()).unwrap(); - let process = Process::open(&reference) - .unwrap() - .unwrap_or_else(|| panic!("the OpenVMM of the record does not run: {record}")); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the OpenVMM of the record does not run: {record}"); + }; let endpoint = record["endpoint"].as_str().unwrap().to_owned(); assert_endpoint(root, state, &endpoint); let digest = |index: usize| format!("sha256:{}", PROVENANCE[index].2); @@ -604,7 +604,8 @@ fn check_lifecycle(case: &Configuration) { assert!( process .wait(Instant::now() + Duration::from_secs(10)) - .unwrap(), + .unwrap() + .is_some(), "{context}: OpenVMM runs on" ); assert_eq!(entries(&state), STOPPED, "{context}"); @@ -694,7 +695,8 @@ fn lifecycle_of_a_sandbox_whose_upper_is_in_ram() { assert!( process .wait(Instant::now() + Duration::from_secs(10)) - .unwrap(), + .unwrap() + .is_some(), "{context}: OpenVMM runs on" ); assert_eq!(entries(&state), STOPPED, "{context}"); @@ -737,7 +739,8 @@ fn stops_keep_an_outcome_report_that_the_caller_reads_whatever_its_umask() { assert!( process .wait(Instant::now() + Duration::from_secs(10)) - .unwrap(), + .unwrap() + .is_some(), "{context}: OpenVMM runs on" ); assert_eq!(entries(&state), STOPPED, "{context}"); @@ -1429,7 +1432,10 @@ fn recycled_process_id(root: &Root) { let path = root.below("state/runtime.json"); let mut record = json(&path); let reference: ProcessRef = serde_json::from_value(record["process"].clone()).unwrap(); - let openvmm = Unrecorded(Process::open(&reference).unwrap().expect("OpenVMM runs")); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("OpenVMM runs"); + }; + let openvmm = Unrecorded(process); let start_time = record["process"]["startTime"].as_u64().unwrap(); record["process"]["startTime"] = json!(start_time + 1); fs::write(&path, serde_json::to_vec_pretty(&record).unwrap()).unwrap(); @@ -1437,9 +1443,10 @@ fn recycled_process_id(root: &Root) { assert_refused(root, &outcome, "sandbox-exited", &["stale", "not running"]); assert!(!path.exists()); assert!( - !openvmm + openvmm .wait(Instant::now() + Duration::from_millis(200)) - .unwrap(), + .unwrap() + .is_none(), "the stale record's process was signaled" ); openvmm.kill().unwrap(); @@ -1447,6 +1454,7 @@ fn recycled_process_id(root: &Root) { openvmm .wait(Instant::now() + Duration::from_secs(10)) .unwrap() + .is_some() ); let outcome = operate(root, "stop", "state", &[]); assert_refused(root, &outcome, "sandbox-not-running", &["not running"]); diff --git a/nvx-rs/tests/fake_openvmm.rs b/nvx-rs/tests/fake_openvmm.rs index c667dbe84..3444797cd 100644 --- a/nvx-rs/tests/fake_openvmm.rs +++ b/nvx-rs/tests/fake_openvmm.rs @@ -20,7 +20,7 @@ use std::time::{Duration, Instant}; use nvx::host::instance::{Found, Instance, Running, Started, Stopped, WorkloadOutcome}; use nvx::host::launch::{LaunchSpec, PROCESS_COMMAND_LINE_LIMIT}; -use nvx::host::process::{Process, ProcessRef}; +use nvx::host::process::{Opened, Process, ProcessRef}; use nvx::host::store::{ArtifactIdentity, CallerFiles, IdentityPart, RuntimeIdentity}; use nvx::host::supervisor::spawn_command; use nvx::protocol::{Workload, WorkloadEnvironment}; @@ -425,9 +425,10 @@ fn a_restarted_orchestrator_reattaches_to_its_vm() { let Found::Mismatched(mismatched) = locked.find(|| Ok(runtime_with('9'))).unwrap() else { panic!("a VM of another runtime identity was not reported"); }; - let process = Process::open(mismatched.vmm().record().process()) - .unwrap() - .expect("the VM runs"); + let Opened::Running(process) = Process::open(mismatched.vmm().record().process()).unwrap() + else { + panic!("the VM runs"); + }; for timeout in [Duration::ZERO, Duration::MAX] { let Found::Mismatched(mismatched) = locked.find(|| Ok(runtime_with('9'))).unwrap() else { panic!("a VM of another runtime identity was not reported"); @@ -467,9 +468,10 @@ fn outcome_reports_of_another_runtime_are_never_read() { let Found::Mismatched(mismatched) = locked.find(|| Ok(runtime_with('9'))).unwrap() else { panic!("a VM of another runtime identity was not reported"); }; - let process = Process::open(mismatched.vmm().record().process()) - .unwrap() - .expect("the VM runs"); + let Opened::Running(process) = Process::open(mismatched.vmm().record().process()).unwrap() + else { + panic!("the VM runs"); + }; fs::write( instance.report_path(), r#"{"schema_version":1,"outcome":{"operation":"managed","category":"success", @@ -478,7 +480,7 @@ fn outcome_reports_of_another_runtime_are_never_read() { .unwrap(); if exits_first { process.kill().unwrap(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } let stopped = locked.terminate(mismatched, TIMEOUT).unwrap(); assert_eq!( @@ -517,7 +519,9 @@ fn records_that_name_another_process_never_end_it() { let mut record: serde_json::Value = serde_json::from_slice(&fs::read(&path).unwrap()).unwrap(); let reference: ProcessRef = serde_json::from_value(record["process"].clone()).unwrap(); - let process = Process::open(&reference).unwrap().expect("the VM runs"); + let Opened::Running(process) = Process::open(&reference).unwrap() else { + panic!("the VM runs"); + }; tamper(&mut record); fs::write(&path, serde_json::to_vec(&record).unwrap()).unwrap(); // Under another runtime identity too, the VM is found stopped, and its record is cleared, @@ -528,11 +532,12 @@ fn records_that_name_another_process_never_end_it() { drop(locked); assert!(records_nothing(&instance)); assert!( - !process + process .wait(Instant::now() + Duration::from_millis(200)) .unwrap() + .is_none() ); process.kill().unwrap(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); } } diff --git a/nvx-rs/tests/store.rs b/nvx-rs/tests/store.rs index 39d55d2e4..ea22e24ed 100644 --- a/nvx-rs/tests/store.rs +++ b/nvx-rs/tests/store.rs @@ -20,7 +20,7 @@ use std::time::{Duration, Instant}; use nvx::ErrorKind; use nvx::host::platform; -use nvx::host::process::{BootId, Process, ProcessRef}; +use nvx::host::process::{BootId, Opened, Process, ProcessRef}; use nvx::host::store::{ ArtifactIdentity, IdentityCheck, IdentityPart, JOURNAL_NAME, Launch, LaunchId, LaunchState, LockedStore, RUNTIME_NAME, Recovery, RuntimeIdentity, SandboxStore, @@ -179,11 +179,13 @@ fn kill(pid: u32, start_time: u64) -> io::Result { start_time, boot: BootId::current()?, }; - let Some(process) = Process::open(&reference)? else { + let Opened::Running(process) = Process::open(&reference)? else { return Ok(true); }; process.kill()?; - process.wait(Instant::now() + RECOVERY_TIMEOUT) + process + .wait(Instant::now() + RECOVERY_TIMEOUT) + .map(|exited| exited.is_some()) } /// A VMM that a crashed orchestrator left behind, which is killed when the test ends if it still @@ -528,12 +530,17 @@ fn a_committed_launch_names_its_vm_for_another_build() { IdentityCheck::Mismatched(vec![IdentityPart::Initramfs]) ); // The record's verified reference holds the VMM, whichever process started it. - let reattached = Process::open(found.process()) - .unwrap() - .expect("the recorded VMM runs"); + let Opened::Running(reattached) = Process::open(found.process()).unwrap() else { + panic!("the recorded VMM runs"); + }; assert!(!reattached.has_exited().unwrap()); drop(vmm); - assert!(reattached.wait(Instant::now() + RECOVERY_TIMEOUT).unwrap()); + assert!( + reattached + .wait(Instant::now() + RECOVERY_TIMEOUT) + .unwrap() + .is_some() + ); other.lock(None).unwrap().clear_runtime().unwrap(); assert!(other.runtime().unwrap().is_none()); } diff --git a/nvx-rs/tests/supervisor.rs b/nvx-rs/tests/supervisor.rs index be2fb01a0..f5a29252a 100644 --- a/nvx-rs/tests/supervisor.rs +++ b/nvx-rs/tests/supervisor.rs @@ -30,7 +30,7 @@ use std::time::{Duration, Instant}; use nvx::ErrorKind; use nvx::host::launch::MAX_TIMEOUT; -use nvx::host::process::{BootId, Process, ProcessRef, WaitMethod}; +use nvx::host::process::{BootId, Opened, Process, ProcessRef, WaitMethod}; #[cfg(windows)] use nvx::host::store::FORMAT; use nvx::host::store::{ @@ -449,8 +449,10 @@ impl Drop for VmmDirectory { fn drop(&mut self) { 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 Ok(Opened::Running(process)) = Process::open(reference) + && !process + .wait(Instant::now() + TIMEOUT) + .is_ok_and(|exited| exited.is_some()) { let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); @@ -570,16 +572,20 @@ impl Drop for Launcher { /// Opens the process that `reference` names, which must still run. fn open_running(reference: &ProcessRef) -> Process { - Process::open(reference) - .unwrap() - .unwrap_or_else(|| panic!("{reference:?} does not run")) + let Opened::Running(process) = Process::open(reference).unwrap() else { + panic!("{reference:?} does not run"); + }; + process } /// 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()); + assert!(matches!( + Process::open(process.reference()).unwrap(), + Opened::Exited(_) + )); } #[test] @@ -613,7 +619,13 @@ fn long_stores_launch_with_a_short_working_directory() { let mut vmm = VmmDirectory::new(); let mut locked = store.lock(None).unwrap(); let running = launch_vmm(&mut locked, &mut vmm); - assert!(!running.process().wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!( + running + .process() + .wait(Instant::now() + SHORT_WAIT) + .unwrap() + .is_none() + ); let stopped = running .stop(&mut locked, Instant::now() + TIMEOUT, None, |_, _| { vmm.end(); @@ -671,15 +683,15 @@ fn a_child_that_exits_wakes_its_waiter() { assert_eq!(process.reference().boot, BootId::current().unwrap()); assert_eq!(process.wait_method(), WaitMethod::Event); assert!(!process.has_exited().unwrap()); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); // The reference finds the same process through a handle of its own. let found = open_running(process.reference()); assert_eq!(found.reference(), process.reference()); let started = Instant::now(); vmm.end(); - assert!(process.wait(started + TIMEOUT).unwrap()); - assert!(found.wait(started + TIMEOUT).unwrap()); + assert!(process.wait(started + TIMEOUT).unwrap().is_some()); + assert!(found.wait(started + TIMEOUT).unwrap().is_some()); assert!(started.elapsed() < TIMEOUT); assert_ended(&process); // Ending a process that exited does nothing. @@ -698,12 +710,12 @@ fn a_process_that_is_not_a_child_wakes_its_waiters() { launcher.finish(); let process = open_running(&reference); assert_eq!(process.wait_method(), WaitMethod::Event); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); let started = Instant::now(); let waiters: Vec = thread::scope(|scope| { let waiters: Vec<_> = (0..2) - .map(|_| scope.spawn(|| process.wait(started + TIMEOUT).unwrap())) + .map(|_| scope.spawn(|| process.wait(started + TIMEOUT).unwrap().is_some())) .collect(); vmm.end(); waiters @@ -740,10 +752,10 @@ fn a_restarted_launcher_finds_its_vmm_again_and_ends_it() { vmm.record(&reference); let process = open_running(&reference); assert_eq!(process.wait_method(), WaitMethod::Event); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); let started = Instant::now(); process.kill().unwrap(); - assert!(process.wait(started + TIMEOUT).unwrap()); + assert!(process.wait(started + TIMEOUT).unwrap().is_some()); assert_ended(&process); } @@ -774,7 +786,7 @@ fn a_reused_process_id_is_never_signaled() { }, ] { assert!( - Process::open(&impostor).unwrap().is_none(), + matches!(Process::open(&impostor).unwrap(), Opened::Exited(_)), "{impostor:?} names a running process" ); } @@ -783,7 +795,7 @@ fn a_reused_process_id_is_never_signaled() { assert!(!process.has_exited().unwrap()); open_running(&reference); vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(launched.kill_and_wait()); } @@ -814,9 +826,12 @@ fn references_that_name_no_process_are_never_opened() { let launched = spawn_vmm(vmm.path(), false); let process = launched.process().unwrap(); vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(launched.kill_and_wait()); - assert!(Process::open(process.reference()).unwrap().is_none()); + assert!(matches!( + Process::open(process.reference()).unwrap(), + Opened::Exited(_) + )); assert!(process.has_exited().unwrap()); } @@ -877,7 +892,7 @@ fn an_unrecorded_vmm_that_serves_no_endpoint_is_never_signaled() { 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()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); match reattach_until_determinate(&mut locked) { Reattached::Exited { record: None, @@ -1264,7 +1279,7 @@ fn check_vmm(reader: PipeReader, writer: &PipeWriter, others: &Mutex> } } vmm.end(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); assert!(launched.kill_and_wait()); } @@ -1493,7 +1508,7 @@ fn a_vmm_in_its_orchestrators_job_ends_with_it() { // 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()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); let mut locked = store.lock(Some(Instant::now() + TIMEOUT)).unwrap(); match supervisor::reattach(&mut locked, &runtime()).unwrap() { Reattached::Exited { @@ -1517,10 +1532,10 @@ fn a_job_that_ends_its_vmm_wakes_the_waiters() { 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); - assert!(!process.wait(Instant::now() + SHORT_WAIT).unwrap()); + assert!(process.wait(Instant::now() + SHORT_WAIT).unwrap().is_none()); let started = Instant::now(); drop(job); - assert!(process.wait(started + TIMEOUT).unwrap()); + assert!(process.wait(started + TIMEOUT).unwrap().is_some()); assert_ended(&process); launcher.wait(); } @@ -1587,7 +1602,7 @@ fn a_process_that_this_user_may_not_terminate_is_waited_for_but_never_signaled() // Its creator ends it, which wakes the waiter, and ending a process that exited does nothing. child.0.kill().unwrap(); - assert!(process.wait(Instant::now() + TIMEOUT).unwrap()); + assert!(process.wait(Instant::now() + TIMEOUT).unwrap().is_some()); process.kill().unwrap(); assert_ended(&process); } diff --git a/nvx-rs/tests/template_capture.rs b/nvx-rs/tests/template_capture.rs index 0036b1c10..d03074966 100644 --- a/nvx-rs/tests/template_capture.rs +++ b/nvx-rs/tests/template_capture.rs @@ -23,7 +23,7 @@ use std::thread; use std::time::{Duration, Instant}; use nvx::host::platform; -use nvx::host::process::{BootId, Process, ProcessRef}; +use nvx::host::process::{BootId, Opened, Process, ProcessRef}; use nvx::host::sandbox::distro::{DISTRO_POLICY_REVISION, DistroBinding, PolicyVerdict}; use nvx::host::sandbox::{LayerRole, SandboxDistro, SandboxLayer, SandboxPlan, SandboxUpper}; #[cfg(target_os = "linux")] @@ -528,7 +528,7 @@ impl Orphan { impl Drop for Orphan { fn drop(&mut self) { - if let Ok(Some(process)) = Process::open(&self.0) { + if let Ok(Opened::Running(process)) = Process::open(&self.0) { let _ = process.kill(); let _ = process.wait(Instant::now() + TIMEOUT); } diff --git a/tests/runtime-adapter/src/bin/runtime-adapter-host.rs b/tests/runtime-adapter/src/bin/runtime-adapter-host.rs index 449ceb1f0..844c9a4f0 100644 --- a/tests/runtime-adapter/src/bin/runtime-adapter-host.rs +++ b/tests/runtime-adapter/src/bin/runtime-adapter-host.rs @@ -2053,6 +2053,7 @@ fn launch_failed( let exited = starting .process() .wait(Instant::now() + STOP_TIMEOUT) + .map(|exited| exited.is_some()) .map_err(|error| format!("cannot wait for the VM to exit: {error}"))?; starting.abandon().map_err(|error| describe(&error))?; println!("stopped={}", if exited { "graceful" } else { "forced" }); @@ -2078,6 +2079,7 @@ fn table_failed( Ok(_) => starting .process() .wait(Instant::now() + STOP_TIMEOUT) + .map(|exited| exited.is_some()) .map_err(|error| format!("cannot wait for the VM to exit: {error}"))?, Err(_) => false, }; @@ -3625,12 +3627,14 @@ fn hold(locked: &mut LockedStore, runtime: &Runtime) -> Result Result<(), String> { - let outcome = report::read(&store.outcome_path()).ok_or_else(|| { - format!( - "the VM left no outcome report {}", - store.outcome_path().display() - ) - })?; + let outcome = report::read(&store.outcome_path()) + .map_err(|error| error.to_string())? + .ok_or_else(|| { + format!( + "the VM left no outcome report {}", + store.outcome_path().display() + ) + })?; print_outcome(&outcome); if ( outcome.category.as_deref(), diff --git a/tests/runtime-adapter/tests/common/mod.rs b/tests/runtime-adapter/tests/common/mod.rs index 3c3540923..fef951c26 100644 --- a/tests/runtime-adapter/tests/common/mod.rs +++ b/tests/runtime-adapter/tests/common/mod.rs @@ -30,7 +30,7 @@ use std::time::{Duration, Instant}; use nvx::Hypervisor; use nvx::host::artifacts::{ArtifactDir, Artifacts, Guest}; -use nvx::host::process::Process; +use nvx::host::process::{Opened, Process}; use nvx::host::store::{LOG_NAME, RuntimeRecord, SandboxStore}; use nvx::host::supervisor::{self, Reattached, StopOutcome}; use nvx::host::template::{CAPTURES_NAME, Store, TRANSACTION_NAME}; @@ -154,11 +154,14 @@ impl Sandbox { pub(crate) fn new(name: &str) -> Self { let pid = std::process::id(); let root = (0..) - .map(|attempt| env::temp_dir().join(format!("runtime-adapter-{name}-{pid}-{attempt}"))) + .map(|attempt| env::temp_dir().join(format!("ra-{pid}-{attempt}"))) .find(|root| match fs::create_dir(root) { Ok(()) => true, Err(error) if error.kind() == io::ErrorKind::AlreadyExists => false, - Err(error) => panic!("cannot create {}: {error}", root.display()), + Err(error) => panic!( + "cannot create the {name} sandbox {}: {error}", + root.display() + ), }) .expect("a directory name is free"); Self { @@ -404,7 +407,10 @@ impl Sandbox { /// Returns the VMM of the sandbox's runtime record, if the record names one that runs. pub(crate) fn vmm(&self) -> Option { - Process::open(self.record()?.process()).unwrap() + match Process::open(self.record()?.process()).unwrap() { + Opened::Running(process) => Some(process), + Opened::Exited(_) => None, + } } /// Ends the VM that the sandbox's store records, whatever its runtime identity: reattach ends diff --git a/tests/runtime-adapter/tests/vm_clone.rs b/tests/runtime-adapter/tests/vm_clone.rs index 654fad486..46f447124 100644 --- a/tests/runtime-adapter/tests/vm_clone.rs +++ b/tests/runtime-adapter/tests/vm_clone.rs @@ -228,7 +228,7 @@ use nvx::exec::{Key, Limits, Timers, Window}; use nvx::host::artifacts::Artifacts; use nvx::host::filesystem::GUEST_EXPORT; use nvx::host::platform::Transport; -use nvx::host::process::{BootId, Process}; +use nvx::host::process::{BootId, Opened, Process}; use nvx::host::sandbox::SandboxNetwork; use nvx::host::store::{CallerFiles, LOG_NAME, SandboxStore}; use nvx::host::template::{ @@ -2321,7 +2321,7 @@ impl CloneControl for Early { .send(&self.early, Some(deadline)) .map_err(io::Error::other)?; let answer = session.receive(Some(deadline)); - self.exited = self.vmm.wait(deadline)?; + self.exited = self.vmm.wait(deadline)?.is_some(); Err(match answer { Ok(message) => io::Error::other(format!("the clone answered {message:?}")), Err(error) => io::Error::other(error), @@ -2379,9 +2379,10 @@ fn refuses_early(template: &Template, early: Message) { let restoring = planned .launch(&mut acquired, &runtime.artifacts.openvmm, &runtime.identity) .unwrap(); - let vmm = Process::open(restoring.starting().process().reference()) - .unwrap() - .expect("the clone's VMM runs"); + let Opened::Running(vmm) = Process::open(restoring.starting().process().reference()).unwrap() + else { + panic!("the clone's VMM runs"); + }; let mut control = Early { early: early.clone(), vmm, @@ -3910,7 +3911,9 @@ fn a_host_program_of_another_runtime_ends_the_clone_through_its_verified_identit assert!(!ended.contains_key("epoch"), "{ended:?}"); assert!(sandbox.stderr().contains("differs in initramfs")); assert!( - vmm.wait(Instant::now() + Duration::from_secs(10)).unwrap(), + vmm.wait(Instant::now() + Duration::from_secs(10)) + .unwrap() + .is_some(), "the clone's VMM still runs" ); assert!(sandbox.record().is_none()); @@ -3948,7 +3951,9 @@ fn a_clone_whose_vm_exits_once_a_stop_found_it_fails_before_its_lease_is_release assert_eq!(sandbox.paused(&mut stopping, &pause), "found"); vmm.kill().unwrap(); assert!( - vmm.wait(Instant::now() + Duration::from_secs(10)).unwrap(), + vmm.wait(Instant::now() + Duration::from_secs(10)) + .unwrap() + .is_some(), "the clone's VMM still runs" ); fs::remove_file(&pause).unwrap(); @@ -5313,7 +5318,9 @@ fn a_run_whose_clone_s_vm_exits_during_its_execution_ends_with_the_clone_s_failu let vmm = sandbox.vmm().expect("the clone's VMM runs"); vmm.kill().unwrap(); assert!( - vmm.wait(Instant::now() + Duration::from_secs(10)).unwrap(), + vmm.wait(Instant::now() + Duration::from_secs(10)) + .unwrap() + .is_some(), "the clone's VMM still runs" ); diff --git a/tests/runtime-adapter/tests/vm_steps.rs b/tests/runtime-adapter/tests/vm_steps.rs index bc4821d7d..9ff0fea3c 100644 --- a/tests/runtime-adapter/tests/vm_steps.rs +++ b/tests/runtime-adapter/tests/vm_steps.rs @@ -20,7 +20,7 @@ use std::path::Path; use std::time::Instant; use nvx::ErrorCode; -use nvx::host::process::Process; +use nvx::host::process::{Opened, Process}; use nvx::host::store::{LaunchState, SandboxStore}; use runtime_adapter::hook::{Action, HOOK_EXIT, STEP_HOOK, Step, StepHook}; use runtime_adapter::staged::Staged; @@ -205,9 +205,9 @@ fn the_hook_pauses_a_launch_at_each_of_its_steps() { .unwrap() .expect("the launch has its journal"); assert_eq!(journal.state(), LaunchState::Spawned, "at {step}"); - let vmm = Process::open(journal.process().unwrap()) - .unwrap() - .expect("the VMM runs"); + let Opened::Running(vmm) = Process::open(journal.process().unwrap()).unwrap() else { + panic!("the VMM runs"); + }; assert!(!vmm.has_exited().unwrap(), "at {step}"); assert_eq!(store.runtime().unwrap(), None, "at {step}"); }