From 35bdc84dd9fc17dbcf861def58f41466915f2bb2 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 10:45:15 -0700 Subject: [PATCH 1/7] guest: prepare a template's guest for a capture before root assembly A platform template's snapshot must hold no tenant root, so that each clone attaches its own image, upper, and export. nvx::guest::clone gives any guest init or agent the preparation that the shell nvx-snapshot lacks: CapturePreparation closes the caller's AdmissionFence, refuses to capture while the mount table shows anything mounted from the distro device or a partition of it, the scratch device, the host filesystem device's tag, or an overlay, bind mounts included, then waits for the time ABI's boot checks, runs its capture checks, and suppresses the stall detectors. The CaptureBarrier that it returns requests the capture through TimeAbi::capture with the platform tier's request, and releases the fence and the detectors when the capture continues, fails, or is canceled, or the barrier is dropped. A restored clone returns with the fence open and the detectors suppressed, for repair to finish. The time module now offers the pre-capture and cancel-capture steps as crate-internal typed calls, which nvx-time's commands run with unchanged diagnostics, and the root module shares its MMIO-window lookup. Part of #840 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da708b6-0bda-4804-b01b-936cdc5f054a --- nvx-rs/README.md | 9 +- nvx-rs/src/guest/clone.rs | 612 ++++++++++++++++++++++ nvx-rs/src/guest/clone/mounts.rs | 347 ++++++++++++ nvx-rs/src/guest/clone/tests.rs | 841 ++++++++++++++++++++++++++++++ nvx-rs/src/guest/mod.rs | 8 +- nvx-rs/src/guest/root.rs | 4 +- nvx-rs/src/guest/root/block.rs | 20 +- nvx-rs/src/guest/root/kernel.rs | 8 +- nvx-rs/src/guest/time/api.rs | 101 ++++ nvx-rs/src/guest/time/commands.rs | 126 ++--- nvx-rs/src/guest/time/mod.rs | 1 + nvx-rs/src/lib.rs | 7 +- 12 files changed, 2008 insertions(+), 76 deletions(-) create mode 100644 nvx-rs/src/guest/clone.rs create mode 100644 nvx-rs/src/guest/clone/mounts.rs create mode 100644 nvx-rs/src/guest/clone/tests.rs diff --git a/nvx-rs/README.md b/nvx-rs/README.md index c0129d510..ab11b447e 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -58,7 +58,8 @@ nvx-rs/ │ │ execution, alone or through the execution engine, collects its children's │ │ statuses, settles its executions' cgroups, mounts host mappings beneath the │ │ descriptors of the roots that it names, assembles and tears down a layered -│ │ root, and reads a GPT disk's partition table through its block device, and +│ │ root, reads a GPT disk's partition table through its block device, and +│ │ prepares a template's guest for a capture before its root's assembly, and │ │ the typed calls through which it runs the time ABI's steps and the reseed │ └── bin/ nvx-rs, nvx-fake-openvmm, and the guest binaries ├── tests/ fake-OpenVMM, CLI, NVXS session, process-supervision, checked-handoff, store, @@ -113,6 +114,7 @@ nvx-rs/ | `nvx::guest::exec` | `Execution`: drives one program that the caller spawned with the program's ends of its `Pipes`, or that the caller started itself and adopts with its own ends of the program's pipes and a `Child` that names it, without blocking, from the caller's own loop or reactor, and reports each step as an `Event`: the launch report, which an adopted program has at once, the output of each stream, kill failures, and the `Outcome`. It reads a stream only as far as the caller's `grant` allows, so a stalled sink stalls its own execution alone, writes the program's standard input without `SIGPIPE`, and ends the program on a cancellation, a timeout, or a disconnection, with `SIGKILL` through the program's pidfd to the program and, if the caller asks, its process group, while no collector has taken its status, and, through the opened directory of a cgroup that the caller names, to that cgroup. It closes the program's input once the program has ended. It waits for its program alone, by a pidfd, or takes the status from the caller's collector with `observe`; with `Options::caller_reaps`, that collector is the program's only one. NVX's managed agent is one caller | | `nvx::guest::exec::binding` | `Binding`: an `nvx::exec::Runner` bound to an `Execution`, the glue that a guest adapter would otherwise write. The runner's read allowances become grants, its discards discards, the input that it accepted writes and then a close, its terminations a cancellation, a disconnection, or a termination, and its settlement a kill of the execution's cgroup and checks until no process occupies it; without a cgroup, nothing proves that the program's descendants are gone, so the execution is fenced. It feeds the execution's events back to the runner, statuses that a caller who reaps hands over with `observe` included, and never blocks: the caller polls its descriptors and its input writer until its deadline, and each call takes a bounded number of steps, so that no execution holds the caller's loop | | `nvx::guest::time` | x86_64: `TimeAbi`, the steps of NVX time ABI v1 as typed calls in the caller's process, for any guest init or agent: the cold-boot time step and, separately, the start of the time daemon in a child process that the caller supervises, the VM generation ID, a `CaptureRequest` that repairs a `Restore`, and a `RestoreFinish`. A call takes the permission for OpenVMM's ports on the calling thread for each port transaction alone, and fails with `TimeError::PortRelease` if the thread may still hold some of it, which a process that it started would inherit, and from which `nvx::guest::spawn` starts none. No call waits for a child that it did not start. `nvx-time` runs its steps through them | +| `nvx::guest::clone` | x86_64: the preparation of a template's guest for a capture before its root's assembly, for a guest init or agent, whether or not it is PID 1. `CapturePreparation`, of the caller's `AdmissionFence` and the `Devices` that the template's clones replace, the `distro` and scratch block devices, which their MMIO windows name, and the host filesystem device's virtio-fs tag: `prepare` closes the fence, so that no execution starts, refuses with a `Mounted` the first mount from those devices, a distro partition included, or from an overlay, bind mounts included, in a bounded read of `/proc/self/mountinfo` of at most `MOUNT_TABLE_LIMIT` bytes, then waits for the time ABI's boot checks, runs its capture checks, and suppresses the stall detectors, as `nvx-time pre-capture` does, and returns an owned `CaptureBarrier`. `CaptureBarrier::capture` requests the capture with the platform tier's request, through `TimeAbi::capture`, and returns `Captured::Continued` once it opened the fence and restored the detectors' settings, or, in a clone, `Captured::Restored`, with the `Restore` that repair takes next, once it opened the fence, with the detectors suppressed until the time daemon restores their settings. `CaptureBarrier::cancel`, a failed capture, and a dropped barrier open the fence and restore the detectors' settings. Each failure is a `CaptureError`, with its `CaptureStage` and `CaptureCause` | | `nvx::guest::reseed` | `Seed::read` and `reseed`: a restore's fresh entropy, which starts with its `GenerationId`, and the reseed of the kernel's random number generator with it, in the caller's process. `nvx-reseed` runs them | | `nvx::guest::root` | The operations of a layered root, for a guest init or agent, whether or not it is PID 1. `bootstrap`: the agent's and the workload's sibling cgroups of a `Layout`, with every process of their parent moved into the agent's, the agent's `memory.low`, and the workload cgroup prepared through `ExecutionCgroup::prepare`, with its `memory.max` and `pids.max`, and user namespaces kept unavailable. `assemble`: the `Assembly`'s block devices, found by their MMIO windows, checked against their roles' access modes and their expected sizes, and flushed, and the `Root` that it mounts, at a runtime directory, a canonical path that no symbolic link leads through, that it holds exclusively until its teardown, where nothing is mounted already, once it has checked everything: the read-only layers of the `Image` that the caller names, which it never probes: an EROFS image, whose expected `nvx::uuid::Uuid` it checks in its superblock, or a GPT disk whose partition table it reads and checks with `nvx::guest::gpt::read_disk`, against the caller's `nvx::gpt::Expected` values, whose first partition it reads, up to the caller's bound, as opaque bytes that it never mounts, and whose later partitions, at least one, hold ext4 layers that it mounts `ro,nosuid,nodev` with `errors=remount-ro`, so that an error that the kernel finds in a layer fails the operation that found it, rather than panic the guest, as the layer's superblock may ask, from the block devices that the kernel created when it found the disk, once their superblocks show that no journal needs a recovery, which their read-only device can't take, partition 2's at the overlay's base and the last partition's at its top, under an overlay mounted `nosuid,nodev` with the one option set of the caller's `OverlayMode`, `metacopy=on,xino=on` or, for lowers that may carry untrusted overlay xattrs, `metacopy=off,redirect_dir=nofollow`, with no fallback, whose upper and work directories are on the `Upper`: an ext4 scratch mounted `nosuid,nodev`, whose superblock it checks for a journal that the caller's `Journal` policy forbids, or a tmpfs mounted `nosuid,nodev` at the kernel's default size. It refuses overlay options longer than `MOUNT_OPTIONS_LIMIT`, the most that a mount passes, which, below `/run/nvx`, fit up to 195 layers of a GPT disk, and an overlay that the kernel's mount table shows read-only or without the mode's options in effect, since the kernel mounts one so, rather than fail, when its upper can't hold its work directory or its extended attributes. The `Root` reports the first partition's bytes, its `Lower` layers, its `UpperKind`, and its `OverlayMode`. `Root::mount_share` and `Root::mount_aggregate`: virtio-fs live shares at targets in the root that it checks, against the targets that `protocol::share` reserves among others, with one aggregate per root, which it attaches through its `mappings::Mapper` beneath the overlay, with NVX's container root's reserved targets: a share's device mounted out of the root, then moved onto its target, and a copy of each child's entry of the aggregate. `Root::mount_export`: the host export, the aggregate device of `protocol::export`, one per root, mounted `nosuid,nodev` with the access of its `ExportMode` at `hostfs/root` in the runtime directory, out of the root, below a directory that only root may enter, whose root it returns as the `SourceRoot` of the caller's mappings. `Root::map`: a caller's host mappings, through the same mapper. A share, a child, the export, or a mapping that fails latches the root. `Root::teardown`: the shares and mappings in reverse order, then the mounts of an aggregate and of the export, the later first, the overlay, the layers from the top down, and the upper, with every failed unmount reported. Each failure is a typed `Error`. The operations collect no child's status, change neither the caller's root nor its credentials, start no async runtime, and never power the VM off | | `nvx::guest::gpt` | `read_disk`: the partition table of a GPT disk, read and checked with `nvx::gpt`'s `Disk::read` through the disk's block device, which the caller opened, at the logical block size and size that `BLKSSZGET` and `BLKGETSIZE64` report, with the caller's `Expected` values, such as the disk GUID that a boot parameter names. It only reads the device, at explicit offsets, so it never moves the file's position. It neither compares the partitions that the kernel created with the table nor asks the kernel to reread the table. Each failure is a typed `DeviceError` | @@ -1059,7 +1061,10 @@ beneath the container's assembled root in sandbox mode, where it unmounts the table before it returns to the init agent, and any other guest init or agent can use them too. Any guest init or agent can also run the steps of `nvx-time` and `nvx-reseed` in its own process, through the -`nvx::guest::time` and `nvx::guest::reseed` calls, on which both programs run. +`nvx::guest::time` and `nvx::guest::reseed` calls, on which both programs run, +and a template's guest prepares for a capture before its root's assembly through +`nvx::guest::clone`, which runs the steps of `nvx-time pre-capture` behind the +caller's fence of workload admission. Nor is the `gpt` library a program: it reads the partition table of a GPT disk's block device. diff --git a/nvx-rs/src/guest/clone.rs b/nvx-rs/src/guest/clone.rs new file mode 100644 index 000000000..8a9f1bd37 --- /dev/null +++ b/nvx-rs/src/guest/clone.rs @@ -0,0 +1,612 @@ +//! Templates and clones in the guest: the capture of a template's guest before its root's +//! assembly, for a guest init or agent, whether or not it is PID 1. +//! +//! A platform template is a VM whose snapshot holds no tenant's root, so that every clone that a +//! restore of the snapshot starts can attach its own image, upper, and export. Its guest captures +//! through a [`CapturePreparation`], which the caller makes of its own fence of workload +//! admission, an [`AdmissionFence`], and the identities of the [`Devices`] from which the +//! snapshot must hold no mount: the `distro` block device and the scratch block device, which +//! their MMIO windows name, and the host filesystem device, which its virtio-fs tag names. +//! [`CapturePreparation::prepare`], in order: +//! +//! 1. closes the caller's fence, so that no execution starts; +//! 2. reads the mount table of the caller's mount namespace, `/proc/self/mountinfo`, at most +//! [`MOUNT_TABLE_LIMIT`] bytes of it, and refuses to capture while anything is mounted from the +//! distro device or one of its partitions, the scratch device, an overlay, or the filesystem +//! tag, bind mounts included, with a [`Mounted`] that names the first such mount; +//! 3. waits until the time ABI's boot checks have completed, for 30 seconds at most, runs its +//! capture checks, and saves the settings of the stall detectors and suppresses them: RCU's +//! stall warnings, the soft-lockup detector, and the hung-task detector. +//! +//! It returns an owned [`CaptureBarrier`], which holds the fence closed and the detectors +//! suppressed. [`CaptureBarrier::capture`] reads the VM generation ID, and requests the capture +//! through [`TimeAbi::capture`] with the platform tier's request, which keeps no scratch. It +//! returns how the capture ended: +//! +//! - [`Captured::Continued`]: the VM continued without a restore, and the barrier opened the +//! fence, then restored the detectors' settings. +//! - [`Captured::Restored`]: a restore of the snapshot started this VM, a clone whose wall clock +//! the time ABI repaired, and the barrier opened the fence. The detectors stay suppressed until +//! the restore's finish, [`TimeAbi::restore_finish`], hands their release to the time daemon. +//! The caller repairs the clone next, from the [`Restored`] restore. OpenVMM ends a platform +//! template's VM once its capture commits, so only clones return with this outcome. +//! +//! [`CaptureBarrier::cancel`] opens the fence, then restores the detectors' settings. So does a +//! capture that fails, after which the caller ends the VM, and so does a barrier that is dropped, +//! which ignores a failure. A preparation that fails releases what it took. A failure is a +//! [`CaptureError`], which names the [`CaptureStage`] that failed and its [`CaptureCause`]. +//! +//! ```no_run +//! use std::io; +//! +//! use nvx::guest::clone::{AdmissionFence, CapturePreparation, Captured, Devices}; +//! use nvx::guest::root::Device; +//! use nvx::protocol::root::{DISTRO_MMIO_BASE, SCRATCH_MMIO_BASE}; +//! +//! /// The agent's admission of executions, which it checks before it starts one. +//! struct Admission { +//! open: bool, +//! } +//! +//! impl AdmissionFence for Admission { +//! fn close(&mut self) -> io::Result<()> { +//! self.open = false; +//! Ok(()) +//! } +//! +//! fn open(&mut self) { +//! self.open = true; +//! } +//! } +//! +//! # fn main() -> Result<(), Box> { +//! let mut admission = Admission { open: true }; +//! let devices = Devices { +//! distro: Device { mmio: DISTRO_MMIO_BASE, sectors: None }, +//! scratch: Device { mmio: SCRATCH_MMIO_BASE, sectors: None }, +//! filesystem_tag: "microvm".to_owned(), +//! }; +//! let barrier = CapturePreparation::new(&mut admission, devices).prepare()?; +//! match barrier.capture()? { +//! Captured::Continued => println!("the VM continued without a restore"), +//! Captured::Restored(restored) => { +//! // A clone continues here, and repairs the restore. +//! println!("restored to {}", restored.restore().generation_id()); +//! } +//! } +//! # Ok(()) +//! # } +//! ``` +//! +//! The operations need the guest's root user and the privileges of the time ABI's calls. They +//! read the mount table of the caller's mount namespace, which must be the one where the guest +//! mounts its devices, such as its initial namespace. They mount nothing, start no process or +//! thread, and collect no child's status. Apart from a violation of the time ABI in enforcing +//! mode, which stops the VM as [`TimeAbi`] tells, they never power the VM off, so a caller that is +//! not PID 1 keeps its own reaping, transport, and shutdown. + +mod mounts; +#[cfg(test)] +mod tests; + +use std::fmt; +use std::fs::{self, File}; +use std::io::{self, Read}; +use std::mem; +use std::path::{Path, PathBuf}; + +use self::mounts::DeviceNumber; +use super::GenerationId; +use super::root::{self, Device, Paths}; +use super::sys; +use super::time::{ + CaptureOutcome, CaptureRequest, PreCaptureError, Restore, ScratchPolicy, TimeAbi, TimeError, +}; + +pub use self::mounts::{Mounted, Origin}; + +/// The most bytes of the mount table that a [`CapturePreparation`] reads: 1 MiB, the lines of +/// thousands of mounts. It refuses a longer table, rather than check a part of it. +pub const MOUNT_TABLE_LIMIT: usize = 1 << 20; + +/// The mount table of the calling process's mount namespace, below procfs. +const MOUNT_TABLE: &str = "self/mountinfo"; +/// The directory of the block devices' sysfs entries, below sysfs, which names each by its +/// device number, `MAJ:MIN`. +const BLOCK_DEVICES: &str = "dev/block"; +/// The file to which a restore writes its fresh entropy, in the time ABI's runtime directory. +const ENTROPY_PATH: &str = "/run/nvx/restore-entropy"; + +/// The caller's fence of workload admission, which its agent checks before it starts each +/// execution: while the fence is closed, no execution starts. +/// +/// A [`CapturePreparation`] closes it before anything else, and its [`CaptureBarrier`] opens it +/// again once the capture continues, restores, fails, or is canceled, or the barrier is dropped. +/// The fence need not end the executions that run already: a template's guest captures before +/// it starts any. A preparation takes the fence, so a caller that keeps its admission's state +/// lends it a handle of that state, or a mutable reference to its fence, which the trait's +/// implementation for `&mut F` takes. +pub trait AdmissionFence { + /// Closes the fence: once it returns, no execution starts until [`open`](Self::open). A + /// failure leaves the fence open. + fn close(&mut self) -> io::Result<()>; + + /// Opens the fence that [`close`](Self::close) closed, so that executions start again. + fn open(&mut self); +} + +impl AdmissionFence for &mut F { + fn close(&mut self) -> io::Result<()> { + (**self).close() + } + + fn open(&mut self) { + (**self).open(); + } +} + +/// The devices of a template's guest from which a capture must find nothing mounted, so that +/// each clone attaches its own. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct Devices { + /// The `distro` block device, which holds a layered root's image, named by its MMIO window, + /// such as [`DISTRO_MMIO_BASE`](crate::protocol::root::DISTRO_MMIO_BASE). Nothing may be + /// mounted from it or from a partition of it. The capture checks no size. + pub distro: Device, + /// The scratch block device, named by its MMIO window, such as + /// [`SCRATCH_MMIO_BASE`](crate::protocol::root::SCRATCH_MMIO_BASE). Nothing may be mounted + /// from it. The capture checks no size. + pub scratch: Device, + /// The virtio-fs tag of the host filesystem device, which the mount table shows as the source + /// of the device's mounts. Nothing may be mounted from it. + pub filesystem_tag: String, +} + +/// The preparation of a template's guest for a capture, which the caller makes of its +/// workload-admission fence and the identities of the devices from which the snapshot must hold +/// no mount; see the [module](self). +pub struct CapturePreparation { + /// The caller's fence of workload admission. + fence: F, + /// The devices from which nothing may be mounted. + devices: Devices, +} + +impl CapturePreparation { + /// Returns the preparation of a capture behind the caller's `fence`, of a guest from whose + /// `devices` nothing may be mounted. + pub fn new(fence: F, devices: Devices) -> Self { + Self { fence, devices } + } + + /// Prepares the guest for the capture, as the [module](self) describes: closes the fence, + /// checks the mount table, waits for the time ABI's boot checks, runs its capture checks, and + /// suppresses the stall detectors, then returns the barrier that holds them. + /// + /// The wait for the boot checks lasts 30 seconds at most. A capture check that fails reports + /// a violation of the time ABI, which stops the VM in enforcing mode; in report-only mode, + /// the preparation goes on, as `nvx-time pre-capture` does. A preparation that fails opens + /// the fence again, and leaves the detectors' settings as they were. + pub fn prepare(self) -> Result, CaptureError> { + let system = LiveSystem { + paths: Paths::default(), + abi: TimeAbi::new(), + }; + self.prepare_with(Box::new(system)) + } + + /// Implements [`prepare`](Self::prepare) on `system`. + fn prepare_with(self, system: Box) -> Result, CaptureError> { + let Self { fence, devices } = self; + let mut barrier = CaptureBarrier { + fence, + fenced: false, + suppressed: false, + system, + }; + barrier + .fence + .close() + .map_err(|error| CaptureError::new(CaptureStage::Fence, CaptureCause::Io(error)))?; + barrier.fenced = true; + check_mounts(barrier.system.as_mut(), &devices) + .map_err(|cause| CaptureError::new(CaptureStage::Mounts, cause))?; + barrier.system.pre_capture().map_err(|error| match error { + PreCaptureError::BootChecks(error) => { + CaptureError::new(CaptureStage::BootChecks, CaptureCause::Time(error)) + } + PreCaptureError::CaptureChecks(error) => { + CaptureError::new(CaptureStage::CaptureChecks, CaptureCause::Time(error)) + } + PreCaptureError::Pending => { + CaptureError::new(CaptureStage::Suppression, CaptureCause::Pending) + } + PreCaptureError::Suppression(error) => { + CaptureError::new(CaptureStage::Suppression, CaptureCause::Io(error.into())) + } + })?; + barrier.suppressed = true; + Ok(barrier) + } +} + +impl fmt::Debug for CapturePreparation { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("CapturePreparation") + .field("devices", &self.devices) + .finish_non_exhaustive() + } +} + +/// Checks, in the mount table that `system` reads, that nothing is mounted from `devices` or an +/// overlay, and fails with the first mount that is, or with why the table could not be checked. +fn check_mounts(system: &mut dyn System, devices: &Devices) -> Result<(), CaptureCause> { + let table = system.mount_table().map_err(CaptureCause::Io)?; + if table.len() > MOUNT_TABLE_LIMIT { + return Err(CaptureCause::Io(io::Error::new( + io::ErrorKind::FileTooLarge, + format!("the mount table is longer than {MOUNT_TABLE_LIMIT} bytes"), + ))); + } + match mounts::first_blocking(system, devices, &table).map_err(CaptureCause::Io)? { + Some(mounted) => Err(CaptureCause::Mounted(mounted)), + None => Ok(()), + } +} + +/// The barrier of a prepared capture, which holds the caller's fence closed and the stall +/// detectors suppressed until the capture ends, or the barrier is canceled or dropped; see the +/// [module](self). +#[must_use = "dropping the barrier opens the fence and restores the stall detectors"] +pub struct CaptureBarrier { + /// The caller's fence of workload admission. + fence: F, + /// Whether the barrier closed the fence, which it opens again. + fenced: bool, + /// Whether the barrier suppressed the stall detectors, whose settings it restores. + suppressed: bool, + /// The kernel and the time ABI. + system: Box, +} + +impl CaptureBarrier { + /// Requests the capture: reads the VM generation ID, then requests a capture through + /// [`TimeAbi::capture`] with the platform tier's request, which keeps no scratch, and returns + /// how it ended. + /// + /// A VM that continues without a restore gets [`Captured::Continued`], once the barrier + /// opened the fence and restored the detectors' settings; a failure to restore them is a + /// [`CaptureStage::Release`] error, after which the caller ends the VM. A clone that a + /// restore of the snapshot started gets [`Captured::Restored`], once the barrier opened the + /// fence, with the detectors still suppressed: see the [module](self). A capture that fails + /// opens the fence and restores the detectors' settings, and the caller ends the VM. + pub fn capture(mut self) -> Result { + let generation_id = self.system.generation_id().map_err(|error| { + CaptureError::new(CaptureStage::GenerationId, CaptureCause::Time(error)) + })?; + let request = CaptureRequest::new( + ScratchPolicy::Fresh, + generation_id, + PathBuf::from(ENTROPY_PATH), + ); + let outcome = self + .system + .capture(request) + .map_err(|error| CaptureError::new(CaptureStage::Capture, CaptureCause::Time(error)))?; + match outcome { + CaptureOutcome::Continued => { + self.release()?; + Ok(Captured::Continued) + } + CaptureOutcome::Restored(restore) => { + // The time daemon restores the detectors' settings once repair finishes the + // restore. + self.suppressed = false; + self.open(); + Ok(Captured::Restored(Restored { restore })) + } + } + } + + /// Cancels the capture: opens the fence, then restores the stall detectors' settings. A + /// failure to restore them is a [`CaptureStage::Release`] error, after which the caller ends + /// the VM. + pub fn cancel(mut self) -> Result<(), CaptureError> { + self.release() + } + + /// Opens the fence, if the barrier holds it closed. + fn open(&mut self) { + if mem::take(&mut self.fenced) { + self.fence.open(); + } + } + + /// Opens the fence, then restores the stall detectors' settings, if the barrier holds them. + fn release(&mut self) -> Result<(), CaptureError> { + self.open(); + if mem::take(&mut self.suppressed) { + self.system.cancel_capture().map_err(|detail| { + let detail = String::from_utf8_lossy(&detail).into_owned(); + CaptureError::new( + CaptureStage::Release, + CaptureCause::Io(io::Error::other(detail)), + ) + })?; + } + Ok(()) + } +} + +impl Drop for CaptureBarrier { + /// Opens the fence and restores the stall detectors' settings, as + /// [`CaptureBarrier::cancel`] does, and ignores a failure. + fn drop(&mut self) { + let _ = self.release(); + } +} + +impl fmt::Debug for CaptureBarrier { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("CaptureBarrier") + .field("fenced", &self.fenced) + .field("suppressed", &self.suppressed) + .finish_non_exhaustive() + } +} + +/// How a capture that [`CaptureBarrier::capture`] requested ended, in the VM in which it +/// returned. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Captured { + /// The VM continued without a restore, as [`CaptureOutcome::Continued`] tells: no snapshot + /// was taken, or the restore packet was of the recorded generation. The barrier opened the + /// fence and restored the stall detectors' settings. + Continued, + /// A restore of the snapshot started the VM, a clone, whose wall clock the time ABI repaired. + /// The barrier opened the fence, and the stall detectors stay suppressed. + Restored(Restored), +} + +/// A clone's restore, which [`CaptureBarrier::capture`] returns for repair to take next. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Restored { + /// The restore that the time ABI repaired, as its restore packet describes it. + restore: Restore, +} + +impl Restored { + /// Returns the restore that the time ABI repaired: its new VM generation ID, its targets, and + /// whether the host gates its input until the guest acknowledges it. + pub fn restore(&self) -> &Restore { + &self.restore + } + + /// Returns the file to which the restore wrote its fresh entropy, 64 bytes that start with the + /// new VM generation ID, and that only root may read: `/run/nvx/restore-entropy`. The caller + /// reseeds from it, then removes it. + pub fn entropy_path(&self) -> &'static Path { + Path::new(ENTROPY_PATH) + } + + /// Returns the restore that the time ABI repaired. + pub fn into_restore(self) -> Restore { + self.restore + } +} + +/// A stage of a capture, which a [`CaptureError`] names. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub enum CaptureStage { + /// Closing the caller's fence of workload admission. + Fence, + /// Checking that nothing is mounted from the template's devices or an overlay. + Mounts, + /// Waiting for the time ABI's boot checks. + BootChecks, + /// Running the time ABI's capture checks. + CaptureChecks, + /// Saving the stall detectors' settings and suppressing the detectors. + Suppression, + /// Reading the VM generation ID. + GenerationId, + /// Requesting the capture, and repairing the time of a restore. + Capture, + /// Restoring the stall detectors' settings, as the barrier releases them. + Release, +} + +impl fmt::Display for CaptureStage { + /// Writes what the stage does, such as `check the mount table`. + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(match self { + Self::Fence => "fence workload admission", + Self::Mounts => "check the mount table", + Self::BootChecks => "wait for the time ABI's boot checks", + Self::CaptureChecks => "run the time ABI's capture checks", + Self::Suppression => "suppress the stall detectors", + Self::GenerationId => "read the VM generation ID", + Self::Capture => "request the capture", + Self::Release => "restore the stall detectors", + }) + } +} + +/// Why a stage of a capture failed, which a [`CaptureError`] holds. +#[derive(Debug)] +#[non_exhaustive] +pub enum CaptureCause { + /// The caller's fence, or a system call, failed with this error: a read of the mount table + /// or of a device's entries in procfs and sysfs, which names the file, a mount table longer + /// than [`MOUNT_TABLE_LIMIT`] or of another form than the kernel's, or a save or restore of + /// the stall detectors' settings. + Io(io::Error), + /// Something is mounted from a device from which the capture must find nothing mounted, or + /// from an overlay. + Mounted(Mounted), + /// The stall detectors' settings are saved already, by a capture that is still pending, which + /// restores them. + Pending, + /// A call of the time ABI failed: a violation that a report-only call reports, or a failure of + /// its ports, its time state, or its entropy file. + Time(TimeError), +} + +impl fmt::Display for CaptureCause { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Io(error) => formatter.write_str(&sys::describe(error)), + Self::Mounted(mounted) => mounted.fmt(formatter), + Self::Pending => formatter.write_str("a capture is already pending"), + Self::Time(error) => error.fmt(formatter), + } + } +} + +/// Why a capture's preparation, request, or release failed: the stage that failed, and its cause. +/// +/// Its `Display` form names both, such as `failed to check the mount table: /run/nvx/rootfs is +/// mounted from an overlay`. +#[derive(Debug)] +pub struct CaptureError { + /// The stage that failed. + stage: CaptureStage, + /// Why it failed. + cause: CaptureCause, +} + +impl CaptureError { + /// Returns the failure of `stage`, because of `cause`. + fn new(stage: CaptureStage, cause: CaptureCause) -> Self { + Self { stage, cause } + } + + /// Returns the stage that failed. + pub fn stage(&self) -> CaptureStage { + self.stage + } + + /// Returns why it failed. + pub fn cause(&self) -> &CaptureCause { + &self.cause + } + + /// Returns why it failed. + pub fn into_cause(self) -> CaptureCause { + self.cause + } +} + +impl fmt::Display for CaptureError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(formatter, "failed to {}: {}", self.stage, self.cause) + } +} + +impl std::error::Error for CaptureError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match &self.cause { + CaptureCause::Io(error) => Some(error), + CaptureCause::Time(error) => Some(error), + CaptureCause::Mounted(_) | CaptureCause::Pending => None, + } + } +} + +/// What a capture reads of the kernel and asks of the time ABI, which the tests' fake replaces. +trait System: Send { + /// Reads the mount table of the calling process's mount namespace, at most one byte more than + /// [`MOUNT_TABLE_LIMIT`] of it. + fn mount_table(&mut self) -> io::Result>; + + /// Returns the canonical sysfs directories of the virtio MMIO windows at `mmio` whose devices + /// the kernel added, below which it creates their block devices and partitions. + fn windows(&mut self, mmio: u64) -> io::Result>; + + /// Returns the canonical sysfs directory of the block device whose number is `device`. + fn block_device(&mut self, device: DeviceNumber) -> io::Result; + + /// Waits for the time ABI's boot checks, runs its capture checks, and suppresses the stall + /// detectors, as [`TimeAbi::pre_capture`] does. + fn pre_capture(&mut self) -> Result<(), PreCaptureError>; + + /// Restores the stall detectors' settings, as [`TimeAbi::cancel_capture`] does. + fn cancel_capture(&mut self) -> Result<(), Vec>; + + /// Reads the VM generation ID, as [`TimeAbi::generation_id`] does. + fn generation_id(&mut self) -> Result; + + /// Requests a capture, as [`TimeAbi::capture`] does. + fn capture(&mut self, request: CaptureRequest) -> Result; +} + +/// The guest's kernel, at the paths of its interfaces, and its time ABI. +struct LiveSystem { + /// The directories of the kernel's interfaces. + paths: Paths, + /// The time ABI, in the mode that the kernel command line selects. + abi: TimeAbi, +} + +impl System for LiveSystem { + fn mount_table(&mut self) -> io::Result> { + let path = self.paths.proc.join(MOUNT_TABLE); + let mut table = Vec::new(); + File::open(&path) + .and_then(|file| { + file.take(MOUNT_TABLE_LIMIT as u64 + 1) + .read_to_end(&mut table) + }) + .map_err(|error| at(&path, &error))?; + Ok(table) + } + + fn windows(&mut self, mmio: u64) -> io::Result> { + let windows = root::window_directories(&self.paths, mmio) + .map_err(|error| at(&self.paths.proc.join("iomem"), &error))?; + let mut directories = Vec::with_capacity(windows.len()); + for window in windows { + match fs::canonicalize(&window) { + Ok(directory) => directories.push(directory), + // The kernel lists the window before it adds the window's device, from which + // nothing is mounted yet. + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(at(&window, &error)), + } + } + Ok(directories) + } + + fn block_device(&mut self, device: DeviceNumber) -> io::Result { + let path = self.paths.sys.join(BLOCK_DEVICES).join(device.to_string()); + fs::canonicalize(&path).map_err(|error| at(&path, &error)) + } + + fn pre_capture(&mut self) -> Result<(), PreCaptureError> { + self.abi.pre_capture() + } + + fn cancel_capture(&mut self) -> Result<(), Vec> { + self.abi.cancel_capture() + } + + fn generation_id(&mut self) -> Result { + self.abi.generation_id() + } + + fn capture(&mut self, request: CaptureRequest) -> Result { + self.abi.capture(request) + } +} + +/// Returns `error`, of an operation on `path`, as an error of the same kind whose text names the +/// path, such as `/proc/self/mountinfo: Permission denied`. +fn at(path: &Path, error: &io::Error) -> io::Error { + io::Error::new( + error.kind(), + format!("{}: {}", path.display(), sys::describe(error)), + ) +} diff --git a/nvx-rs/src/guest/clone/mounts.rs b/nvx-rs/src/guest/clone/mounts.rs new file mode 100644 index 000000000..b78eda997 --- /dev/null +++ b/nvx-rs/src/guest/clone/mounts.rs @@ -0,0 +1,347 @@ +//! The kernel's mount table, and the mounts in it that keep a template's guest from capturing. +//! +//! Each line of `/proc/self/mountinfo` describes one mount with fields separated by spaces: its +//! ID, its parent's ID, the device number of its file system, `MAJ:MIN`, the root of the mount +//! in its file system, the mount point, the mount's options, optional fields, a `-` that ends +//! them, then the file system's type, the mount's source, and the file system's options. The +//! kernel writes a space, a tab, a newline, or a backslash within a field as a backslash and +//! three octal digits. + +use std::ffi::OsString; +use std::fmt; +use std::io; +use std::os::unix::ffi::OsStringExt; +use std::path::{Path, PathBuf}; + +use super::{Devices, System}; +use crate::guest::parse::parse_decimal; + +/// The file system type of an overlay, as the mount table shows it. +const OVERLAY: &[u8] = b"overlay"; +/// The file system type of a virtio-fs device's mounts, whose source is the device's tag. +const VIRTIOFS: &[u8] = b"virtiofs"; +/// The field that ends a line's optional fields. +const SEPARATOR: &[u8] = b"-"; +/// The number of fields before a line's optional fields. +const FIXED_FIELDS: usize = 6; + +/// The number of a device, `MAJ:MIN` in the mount table, as its file system's files report it in +/// their `st_dev`. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub(super) struct DeviceNumber { + /// The major number. A file system without a block device, such as an overlay, a virtio-fs + /// device's, or a tmpfs, has an anonymous device, whose major number is 0. + pub(super) major: u32, + /// The minor number. + pub(super) minor: u32, +} + +impl DeviceNumber { + /// Parses the device number of a mount table's line, two canonical decimal numbers separated + /// by `:`, or returns `None` for anything else. + fn parse(field: &[u8]) -> Option { + let colon = field.iter().position(|&byte| byte == b':')?; + let number = |text| parse_decimal(text).and_then(|number| u32::try_from(number).ok()); + Some(Self { + major: number(&field[..colon])?, + minor: number(&field[colon + 1..])?, + }) + } +} + +impl fmt::Display for DeviceNumber { + /// Writes the number as `MAJ:MIN`, as the mount table and sysfs name a device. + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(formatter, "{}:{}", self.major, self.minor) + } +} + +/// What a line of the mount table tells of a mount: where it is, and what it is from. +#[derive(Clone, Debug, PartialEq, Eq)] +struct Line { + /// The device number of the mount's file system. + device: DeviceNumber, + /// The mount point. + mount_point: PathBuf, + /// The file system's type. + filesystem: Vec, + /// The mount's source, as the mounter named it: a block device's node, a virtio-fs device's + /// tag, or any other text. + source: Vec, +} + +impl Line { + /// Parses one line of the mount table, without its newline, or returns `None` for a line of + /// another form than the kernel's. + fn parse(line: &[u8]) -> Option { + let fields = line.split(|&byte| byte == b' ').collect::>(); + if fields.len() < FIXED_FIELDS { + return None; + } + parse_decimal(fields[0])?; + parse_decimal(fields[1])?; + let device = DeviceNumber::parse(fields[2])?; + let separator = FIXED_FIELDS + + fields[FIXED_FIELDS..] + .iter() + .position(|&field| field == SEPARATOR)?; + let [filesystem, source, ..] = &fields[separator + 1..] else { + return None; + }; + // The kernel writes each field with a byte at least, but the source, which is empty when + // the mounter named an empty one. + if fields[..separator] + .iter() + .chain([filesystem]) + .any(|field| field.is_empty()) + { + return None; + } + Some(Self { + device, + mount_point: PathBuf::from(OsString::from_vec(unescape(fields[4]))), + filesystem: unescape(filesystem), + source: unescape(source), + }) + } +} + +/// Returns `field` of the mount table with the kernel's escapes replaced by the bytes that they +/// stand for: a backslash and three octal digits, of a byte up to 0o377. Any other backslash +/// stays. +fn unescape(field: &[u8]) -> Vec { + let mut bytes = Vec::with_capacity(field.len()); + let mut rest = field; + while let Some((&byte, after)) = rest.split_first() { + if let ( + b'\\', + [ + high @ b'0'..=b'3', + middle @ b'0'..=b'7', + low @ b'0'..=b'7', + tail @ .., + ], + ) = (byte, after) + { + bytes.push(((high - b'0') << 6) | ((middle - b'0') << 3) | (low - b'0')); + rest = tail; + } else { + bytes.push(byte); + rest = after; + } + } + bytes +} + +/// What a mount that keeps a template's guest from capturing is mounted from. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub enum Origin { + /// The distro block device, or one of its partitions. + Distro, + /// The scratch block device. + Scratch, + /// An overlay, such as a layered root's. + Overlay, + /// The host filesystem device, whose virtio-fs tag the mount names as its source. + Filesystem, +} + +impl fmt::Display for Origin { + /// Writes what the mount is from, such as `the distro device`. + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(match self { + Self::Distro => "the distro device", + Self::Scratch => "the scratch device", + Self::Overlay => "an overlay", + Self::Filesystem => "the filesystem tag", + }) + } +} + +/// A mount that keeps a template's guest from capturing, as the mount table shows it. +/// +/// Its `Display` form names its mount point and its origin, such as `/run/nvx/rootfs is mounted +/// from an overlay`. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct Mounted { + /// What the mount is from. + origin: Origin, + /// The mount point. + mount_point: PathBuf, +} + +impl Mounted { + /// Returns what the mount is from. + pub fn origin(&self) -> Origin { + self.origin + } + + /// Returns the mount point, as the mount table shows it. + pub fn mount_point(&self) -> &Path { + &self.mount_point + } +} + +impl fmt::Display for Mounted { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + formatter, + "{} is mounted from {}", + self.mount_point.display(), + self.origin + ) + } +} + +/// Returns the first mount of the mount table `table` that is from `devices` or an overlay, bind +/// mounts included, or `None` if there is none. +/// +/// A mount is from the distro device or the scratch device when its file system's device, which +/// `system` finds in sysfs by its number, is below the sysfs directory of the device's MMIO +/// window, where the kernel creates the device's block device and partitions. A mount is from the +/// filesystem tag when its file system is virtio-fs and its source is the tag, and from an overlay +/// when its file system is one. A line that is not of the kernel's form, or a file system on a +/// block device that sysfs does not show, fails the check, which never passes a mount that it +/// cannot tell apart. +pub(super) fn first_blocking( + system: &mut dyn System, + devices: &Devices, + table: &[u8], +) -> io::Result> { + let distro = system.windows(devices.distro.mmio)?; + let scratch = system.windows(devices.scratch.mmio)?; + let tag = devices.filesystem_tag.as_bytes(); + for (index, text) in table.split(|&byte| byte == b'\n').enumerate() { + if text.is_empty() { + continue; + } + let line = Line::parse(text).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("line {} of the mount table is malformed", index + 1), + ) + })?; + let origin = if line.filesystem == OVERLAY { + Some(Origin::Overlay) + } else if line.filesystem == VIRTIOFS && line.source == tag { + Some(Origin::Filesystem) + } else if line.device.major == 0 { + None + } else { + let device = system.block_device(line.device)?; + let below = + |windows: &[PathBuf]| windows.iter().any(|window| device.starts_with(window)); + if below(&distro) { + Some(Origin::Distro) + } else if below(&scratch) { + Some(Origin::Scratch) + } else { + None + } + }; + if let Some(origin) = origin { + return Ok(Some(Mounted { + origin, + mount_point: line.mount_point, + })); + } + } + Ok(None) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Returns the line that the kernel writes for a mount of `filesystem` from `source` at + /// `mount_point`, on the device `device`, with the optional fields `optional`. + fn line( + device: &str, + mount_point: &str, + optional: &str, + filesystem: &str, + source: &str, + ) -> String { + format!("36 25 {device} / {mount_point} rw,relatime{optional} - {filesystem} {source} rw") + } + + #[test] + fn a_line_tells_the_device_mount_point_type_and_source_of_its_mount() { + let text = line( + "254:2", + "/run/nvx/layers/p2", + " shared:7 master:1", + "ext4", + "/dev/vda2", + ); + assert_eq!( + Line::parse(text.as_bytes()), + Some(Line { + device: DeviceNumber { + major: 254, + minor: 2 + }, + mount_point: PathBuf::from("/run/nvx/layers/p2"), + filesystem: b"ext4".to_vec(), + source: b"/dev/vda2".to_vec(), + }) + ); + // Without optional fields or the file system's options. + let parsed = Line::parse(b"1 1 0:2 / / rw - rootfs rootfs").unwrap(); + assert_eq!(parsed.device, DeviceNumber { major: 0, minor: 2 }); + assert_eq!(parsed.filesystem, b"rootfs"); + assert_eq!(parsed.source, b"rootfs"); + // A source that the mounter left empty, and file system options of any form. + let parsed = Line::parse(b"36 25 0:50 / /mnt rw - tmpfs rw x").unwrap(); + assert_eq!(parsed.filesystem, b"tmpfs"); + assert_eq!(parsed.source, b""); + assert_eq!( + DeviceNumber { + major: 259, + minor: 0 + } + .to_string(), + "259:0" + ); + } + + #[test] + fn escaped_fields_are_unescaped() { + let text = line( + "0:31", + "/mnt/a\\040b\\011c\\012d\\134e", + "", + "virt\\134iofs", + "tag\\040x", + ); + let parsed = Line::parse(text.as_bytes()).unwrap(); + assert_eq!(parsed.mount_point, Path::new("/mnt/a b\tc\nd\\e")); + assert_eq!(parsed.filesystem, b"virt\\iofs"); + assert_eq!(parsed.source, b"tag x"); + // A backslash that starts no escape stays, and so does one before a byte above 0o377. + assert_eq!(unescape(b"a\\b\\04\\400\\377"), b"a\\b\\04\\400\xff"); + } + + #[test] + fn lines_of_another_form_are_refused() { + for text in [ + "", + "36 25 254:2 / /mnt rw - ext4", + "36 25 254:2 / /mnt rw shared:1 ext4 /dev/vda rw", + "36 25 254:2 / /mnt rw", + "x 25 254:2 / /mnt rw - ext4 /dev/vda rw", + "36 -1 254:2 / /mnt rw - ext4 /dev/vda rw", + "36 25 254 / /mnt rw - ext4 /dev/vda rw", + "36 25 254:x / /mnt rw - ext4 /dev/vda rw", + "36 25 4294967296:0 / /mnt rw - ext4 /dev/vda rw", + "36 25 +254:0 / /mnt rw - ext4 /dev/vda rw", + "36 25 254:0 / /mnt rw - ext4 /dev/vda rw", + "36 25 254:0 / /mnt rw - ext4 /dev/vda rw", + "36 25 254:0 / /mnt rw shared:1 - ext4 /dev/vda rw", + "36 25 254:0 / /mnt rw - ext4 /dev/vda rw", + ] { + assert_eq!(Line::parse(text.as_bytes()), None, "{text:?}"); + } + } +} diff --git a/nvx-rs/src/guest/clone/tests.rs b/nvx-rs/src/guest/clone/tests.rs new file mode 100644 index 000000000..bc0458910 --- /dev/null +++ b/nvx-rs/src/guest/clone/tests.rs @@ -0,0 +1,841 @@ +//! Tests of a capture's preparation, request, and release, with a fake fence and a fake system, +//! whose mount table, devices, and time ABI each test chooses, which record their calls in one +//! log, and of the live system's lookups in a temporary directory. + +use std::collections::HashMap; +use std::fs; +use std::io; +use std::os::unix::fs::symlink; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex, PoisonError}; + +use super::mounts::{self, DeviceNumber}; +use super::*; +use crate::guest::sys::Errno; + +/// The base address of the distro device's MMIO window. +const DISTRO: u64 = 0xd000_3000; +/// The base address of the scratch device's MMIO window. +const SCRATCH: u64 = 0xd000_6000; +/// The virtio-fs tag of the host filesystem device. +const TAG: &str = "microvm"; +/// The sysfs directory of the distro device's MMIO window. +const DISTRO_WINDOW: &str = "/sys/devices/virtio-mmio-cmdline/virtio-mmio.0"; +/// The sysfs directory of the scratch device's MMIO window. +const SCRATCH_WINDOW: &str = "/sys/devices/virtio-mmio-cmdline/virtio-mmio.1"; +/// The VM generation ID of the template. +const GENERATION: GenerationId = GenerationId::from_bytes([7; GenerationId::LENGTH]); +/// The VM generation ID of a clone that a restore started. +const RESTORED: GenerationId = GenerationId::from_bytes([8; GenerationId::LENGTH]); + +/// The mount table of a template's guest before its root's assembly, which blocks no capture. +const TEMPLATE_TABLE: &str = "\ +1 1 0:2 / / rw - rootfs rootfs rw +20 1 0:20 / /proc rw,nosuid,nodev,noexec,relatime shared:2 - proc none rw +21 1 0:21 / /sys rw,nosuid,nodev,noexec,relatime shared:3 - sysfs none rw +22 1 0:5 / /dev rw,nosuid,relatime shared:4 - devtmpfs none rw,size=65536k,mode=755 +23 1 0:22 / /run rw,nosuid,nodev,relatime shared:5 - tmpfs tmpfs rw,mode=755 +24 1 0:23 / /tmp rw,relatime shared:6 - tmpfs none rw +25 21 0:24 / /sys/fs/cgroup rw,nosuid,nodev,noexec,relatime shared:7 - cgroup2 none rw +"; + +/// A call of a test's fence or system. +#[derive(Clone, Debug, PartialEq, Eq)] +enum Call { + /// The fence closed. + Close, + /// The fence opened. + Open, + /// The mount table was read. + MountTable, + /// The windows at an MMIO base were looked up. + Windows(u64), + /// A block device was looked up by its number. + BlockDevice(DeviceNumber), + /// The time ABI prepared the capture. + PreCapture, + /// The stall detectors' settings were restored. + CancelCapture, + /// The VM generation ID was read. + GenerationId, + /// A capture was requested. + Capture(CaptureRequest), +} + +/// The log of the calls of a test's fence and system, which they share. +#[derive(Clone, Default)] +struct Calls(Arc>>); + +impl Calls { + /// Records `call`. + fn push(&self, call: Call) { + self.0 + .lock() + .unwrap_or_else(PoisonError::into_inner) + .push(call); + } + + /// Returns the calls recorded since the last take, in order. + fn take(&self) -> Vec { + std::mem::take(&mut *self.0.lock().unwrap_or_else(PoisonError::into_inner)) + } +} + +/// A fence that records when it closes and opens, and that fails to close with an error that a +/// test chooses. +struct FakeFence { + /// The log. + calls: Calls, + /// The error number with which the fence fails to close, if any. + failure: Option, +} + +impl AdmissionFence for FakeFence { + fn close(&mut self) -> io::Result<()> { + if let Some(errno) = self.failure { + return Err(errno.into()); + } + self.calls.push(Call::Close); + Ok(()) + } + + fn open(&mut self) { + self.calls.push(Call::Open); + } +} + +/// How the fake time ABI's preparation of a capture ends. +#[derive(Clone, Copy, Debug)] +enum Preparation { + /// It suppresses the stall detectors. + Succeeds, + /// The boot checks are still pending. + BootChecks, + /// The capture checks cannot run. + CaptureChecks, + /// Another capture is pending. + Pending, + /// The detectors' settings cannot be saved, with this error number. + Suppression(Errno), +} + +/// How the fake capture ends. +#[derive(Clone, Copy, Debug)] +enum Outcome { + /// The VM continues without a restore. + Continued, + /// A restore started the VM, a clone. + Restored, + /// The request fails. + Fails, +} + +/// A system that records its calls, with a mount table, devices, and a time ABI that a test +/// chooses: by default, the template's table, the distro's and the scratch's windows, a distro +/// device with partitions, a scratch device, and two other devices, and a time ABI that +/// prepares, reads the VM generation ID, and continues without a restore. +struct FakeSystem { + /// The log. + calls: Calls, + /// The mount table, or the error number of its read. + table: Result, Errno>, + /// The sysfs directories of the windows at each MMIO base. + windows: HashMap>, + /// The sysfs directory of each block device. + devices: HashMap, + /// How the time ABI's preparation ends. + preparation: Preparation, + /// Whether the detectors' settings cannot be restored. + restoration_fails: bool, + /// Whether the VM generation ID cannot be read. + generation_fails: bool, + /// How the capture ends. + outcome: Outcome, +} + +impl FakeSystem { + /// Returns the default system, which records its calls in `calls`. + fn new(calls: &Calls) -> Self { + let block = |window: &str, path: &str| PathBuf::from(format!("{window}/{path}")); + Self { + calls: calls.clone(), + table: Ok(TEMPLATE_TABLE.as_bytes().to_vec()), + windows: HashMap::from([ + (DISTRO, vec![PathBuf::from(DISTRO_WINDOW)]), + (SCRATCH, vec![PathBuf::from(SCRATCH_WINDOW)]), + ]), + devices: HashMap::from([ + (number(254, 0), block(DISTRO_WINDOW, "virtio0/block/vda")), + ( + number(254, 2), + block(DISTRO_WINDOW, "virtio0/block/vda/vda2"), + ), + // The kernel numbers partitions after the 15th from the extended major. + ( + number(259, 0), + block(DISTRO_WINDOW, "virtio0/block/vda/vda17"), + ), + (number(254, 16), block(SCRATCH_WINDOW, "virtio1/block/vdb")), + ( + number(254, 32), + PathBuf::from( + "/sys/devices/virtio-mmio-cmdline/virtio-mmio.2/virtio2/block/vdc", + ), + ), + // The window's name starts with the scratch window's, but is another. + ( + number(254, 48), + PathBuf::from( + "/sys/devices/virtio-mmio-cmdline/virtio-mmio.10/virtio10/block/vdd", + ), + ), + ]), + preparation: Preparation::Succeeds, + restoration_fails: false, + generation_fails: false, + outcome: Outcome::Continued, + } + } + + /// Returns the system with the template's mount table and `lines` after it. + fn with_lines(mut self, lines: &str) -> Self { + self.table = Ok(format!("{TEMPLATE_TABLE}{lines}").into_bytes()); + self + } +} + +impl System for FakeSystem { + fn mount_table(&mut self) -> io::Result> { + self.calls.push(Call::MountTable); + self.table.clone().map_err(io::Error::from) + } + + fn windows(&mut self, mmio: u64) -> io::Result> { + self.calls.push(Call::Windows(mmio)); + Ok(self.windows.get(&mmio).cloned().unwrap_or_default()) + } + + fn block_device(&mut self, device: DeviceNumber) -> io::Result { + self.calls.push(Call::BlockDevice(device)); + self.devices + .get(&device) + .cloned() + .ok_or_else(|| Errno::ENOENT.into()) + } + + fn pre_capture(&mut self) -> Result<(), PreCaptureError> { + self.calls.push(Call::PreCapture); + match self.preparation { + Preparation::Succeeds => Ok(()), + Preparation::BootChecks => Err(PreCaptureError::BootChecks(TimeError::PortAccess( + Errno::EPERM.into(), + ))), + Preparation::CaptureChecks => { + Err(PreCaptureError::CaptureChecks(TimeError::StateUnavailable)) + } + Preparation::Pending => Err(PreCaptureError::Pending), + Preparation::Suppression(errno) => Err(PreCaptureError::Suppression(errno)), + } + } + + fn cancel_capture(&mut self) -> Result<(), Vec> { + self.calls.push(Call::CancelCapture); + if self.restoration_fails { + Err(b"read /run/nvx/time-suppression: No such file or directory".to_vec()) + } else { + Ok(()) + } + } + + fn generation_id(&mut self) -> Result { + self.calls.push(Call::GenerationId); + if self.generation_fails { + Err(TimeError::GenerationIdUnavailable) + } else { + Ok(GENERATION) + } + } + + fn capture(&mut self, request: CaptureRequest) -> Result { + self.calls.push(Call::Capture(request)); + match self.outcome { + Outcome::Continued => Ok(CaptureOutcome::Continued), + Outcome::Restored => Ok(CaptureOutcome::Restored(Restore::to(RESTORED))), + Outcome::Fails => Err(TimeError::StateUnavailable), + } + } +} + +/// Returns the device number `major:minor`. +fn number(major: u32, minor: u32) -> DeviceNumber { + DeviceNumber { major, minor } +} + +/// Returns the template's devices: the distro and the scratch devices at their windows, and the +/// host filesystem device's tag. +fn devices() -> Devices { + Devices { + distro: Device { + mmio: DISTRO, + sectors: None, + }, + scratch: Device { + mmio: SCRATCH, + sectors: Some(2048), + }, + filesystem_tag: TAG.to_owned(), + } +} + +/// Returns a fence that records its calls in `calls`, and closes. +fn fence(calls: &Calls) -> FakeFence { + FakeFence { + calls: calls.clone(), + failure: None, + } +} + +/// Prepares a capture of the template's devices behind `fence`, on `system`. +fn prepare( + fence: FakeFence, + system: FakeSystem, +) -> Result, CaptureError> { + CapturePreparation::new(fence, devices()).prepare_with(Box::new(system)) +} + +/// Returns a barrier that `system` prepared behind a fence, which record their calls in `calls`, +/// with the preparation's calls taken. +fn prepared(calls: &Calls, system: FakeSystem) -> CaptureBarrier { + let barrier = prepare(fence(calls), system).unwrap(); + assert_eq!(calls.take(), preparation_calls()); + barrier +} + +/// Returns the calls of a preparation of the template's table that succeeds. +fn preparation_calls() -> Vec { + vec![ + Call::Close, + Call::MountTable, + Call::Windows(DISTRO), + Call::Windows(SCRATCH), + Call::PreCapture, + ] +} + +/// Returns the platform tier's request of a capture of the template. +fn platform_request() -> CaptureRequest { + CaptureRequest::new( + ScratchPolicy::Fresh, + GENERATION, + PathBuf::from("/run/nvx/restore-entropy"), + ) +} + +#[test] +fn the_fence_closes_before_the_checks_and_the_detectors_are_suppressed_before_the_capture() { + let calls = Calls::default(); + let barrier = prepare(fence(&calls), FakeSystem::new(&calls)).unwrap(); + // The fence first, then the mount table, then the time ABI, which suppresses the detectors. + assert_eq!(calls.take(), preparation_calls()); + assert_eq!(barrier.capture().unwrap(), Captured::Continued); + // The capture's request follows the suppression. A VM that continues opens the fence, then + // restores the detectors' settings. + assert_eq!( + calls.take(), + [ + Call::GenerationId, + Call::Capture(platform_request()), + Call::Open, + Call::CancelCapture, + ] + ); +} + +#[test] +fn each_mount_from_the_template_s_devices_blocks_a_capture() { + let cases = [ + ( + "30 23 254:0 / /run/nvx/layers/distro ro,relatime shared:8 - erofs /dev/vda ro", + Origin::Distro, + "/run/nvx/layers/distro", + ), + // A GPT disk's layers, in its partitions. + ( + "30 23 254:2 / /run/nvx/layers/p2 ro,nosuid,nodev,relatime - ext4 /dev/vda2 ro", + Origin::Distro, + "/run/nvx/layers/p2", + ), + ( + "30 23 259:0 / /run/nvx/layers/p17 ro,nosuid,nodev,relatime - ext4 /dev/vda17 ro", + Origin::Distro, + "/run/nvx/layers/p17", + ), + // A bind mount of a directory of the distro's file system. + ( + "30 23 254:0 /etc /mnt/etc ro,relatime - erofs /dev/vda ro", + Origin::Distro, + "/mnt/etc", + ), + ( + "30 23 254:16 / /run/nvx/scratch rw,nosuid,nodev,relatime - ext4 /dev/vdb rw", + Origin::Scratch, + "/run/nvx/scratch", + ), + ( + "30 23 0:30 / /run/nvx/rootfs rw,nosuid,nodev,relatime - overlay overlay \ + rw,lowerdir=/run/nvx/layers/distro,upperdir=/run/nvx/tmpfs/upper,\ + workdir=/run/nvx/tmpfs/work", + Origin::Overlay, + "/run/nvx/rootfs", + ), + ( + "30 23 0:31 / /run/nvx/hostfs/root ro,nosuid,nodev,relatime - virtiofs microvm ro", + Origin::Filesystem, + "/run/nvx/hostfs/root", + ), + // A bind mount of a child of the export, at a mount point that the kernel escapes. + ( + "30 23 0:31 /src /work\\040tree rw,nosuid,nodev,relatime - virtiofs microvm rw", + Origin::Filesystem, + "/work tree", + ), + ]; + for (line, origin, mount_point) in cases { + let calls = Calls::default(); + let system = FakeSystem::new(&calls).with_lines(&format!("{line}\n")); + let error = prepare(fence(&calls), system).unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Mounts, "{line}"); + let CaptureCause::Mounted(mounted) = error.cause() else { + panic!("{error:?}"); + }; + assert_eq!(mounted.origin(), origin, "{line}"); + assert_eq!(mounted.mount_point(), Path::new(mount_point)); + assert_eq!( + error.to_string(), + format!("failed to check the mount table: {mount_point} is mounted from {origin}") + ); + // The time ABI prepares nothing, and the fence opens again. + let calls = calls.take(); + assert_eq!(calls.first(), Some(&Call::Close)); + assert_eq!(calls.last(), Some(&Call::Open)); + assert!(!calls.contains(&Call::PreCapture), "{calls:?}"); + assert!(!calls.contains(&Call::CancelCapture), "{calls:?}"); + } +} + +#[test] +fn the_first_blocking_mount_of_the_table_is_reported() { + let calls = Calls::default(); + let system = FakeSystem::new(&calls).with_lines( + "30 23 0:30 / /run/nvx/rootfs rw - overlay overlay rw\n\ + 31 23 254:0 / /run/nvx/layers/distro ro - erofs /dev/vda ro\n", + ); + let error = prepare(fence(&calls), system).unwrap_err(); + assert_eq!( + error.to_string(), + "failed to check the mount table: /run/nvx/rootfs is mounted from an overlay" + ); + assert_eq!( + calls.take(), + [ + Call::Close, + Call::MountTable, + Call::Windows(DISTRO), + Call::Windows(SCRATCH), + Call::Open, + ] + ); +} + +#[test] +fn mounts_from_other_devices_and_tags_block_no_capture() { + let calls = Calls::default(); + // Another tag's export, a tmpfs whose source names the tag or an overlay, a file system on + // another block device, and one whose source names the distro's node but whose device is + // another's, below a window whose name only starts with the scratch's. + let system = FakeSystem::new(&calls).with_lines( + "30 23 0:31 / /mnt/other ro - virtiofs other ro\n\ + 31 23 0:32 / /mnt/tag rw - tmpfs microvm rw\n\ + 32 23 0:33 / /mnt/overlay rw - tmpfs overlay rw\n\ + 33 23 254:32 / /mnt/vdc rw - ext4 /dev/vdc rw\n\ + 34 23 254:48 / /mnt/vdd rw - ext4 /dev/vda rw\n", + ); + let barrier = prepare(fence(&calls), system).unwrap(); + assert_eq!( + calls.take(), + [ + Call::Close, + Call::MountTable, + Call::Windows(DISTRO), + Call::Windows(SCRATCH), + Call::BlockDevice(number(254, 32)), + Call::BlockDevice(number(254, 48)), + Call::PreCapture, + ] + ); + barrier.cancel().unwrap(); + assert_eq!(calls.take(), [Call::Open, Call::CancelCapture]); +} + +#[test] +fn a_device_without_a_window_has_nothing_mounted_from_it() { + let calls = Calls::default(); + let mut system = + FakeSystem::new(&calls).with_lines("30 23 254:32 / /mnt/vdc rw - ext4 /dev/vdc rw\n"); + // The kernel lists no window at the scratch's base, as in a VM without a scratch device. + system.windows.remove(&SCRATCH); + system.devices.remove(&number(254, 16)); + let barrier = prepare(fence(&calls), system).unwrap(); + assert!(calls.take().contains(&Call::PreCapture)); + drop(barrier); + assert_eq!(calls.take(), [Call::Open, Call::CancelCapture]); +} + +#[test] +fn a_mount_table_that_cannot_be_checked_blocks_a_capture() { + let lines = |lines: &str| Ok(format!("{TEMPLATE_TABLE}{lines}").into_bytes()); + let cases = [ + ( + Err(Errno::EACCES), + io::ErrorKind::PermissionDenied, + Errno::EACCES.desc().to_owned(), + ), + ( + Ok(vec![b'\n'; MOUNT_TABLE_LIMIT + 1]), + io::ErrorKind::FileTooLarge, + format!("the mount table is longer than {MOUNT_TABLE_LIMIT} bytes"), + ), + ( + lines("30 23\n"), + io::ErrorKind::InvalidData, + "line 8 of the mount table is malformed".to_owned(), + ), + // A file system on a block device that sysfs does not show. + ( + lines("30 23 8:0 / /mnt rw - ext4 /dev/sda rw\n"), + io::ErrorKind::NotFound, + Errno::ENOENT.desc().to_owned(), + ), + ]; + for (table, kind, text) in cases { + let calls = Calls::default(); + let mut system = FakeSystem::new(&calls); + system.table = table; + let error = prepare(fence(&calls), system).unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Mounts); + let CaptureCause::Io(cause) = error.cause() else { + panic!("{error:?}"); + }; + assert_eq!(cause.kind(), kind, "{error}"); + assert_eq!( + error.to_string(), + format!("failed to check the mount table: {text}") + ); + let calls = calls.take(); + assert_eq!(calls.last(), Some(&Call::Open)); + assert!(!calls.contains(&Call::PreCapture), "{calls:?}"); + } + // A table of the limit's length is read whole. + let calls = Calls::default(); + let mut system = FakeSystem::new(&calls); + system.table = Ok(vec![b'\n'; MOUNT_TABLE_LIMIT]); + prepare(fence(&calls), system).unwrap().cancel().unwrap(); +} + +#[test] +fn a_fence_that_fails_to_close_stops_the_preparation() { + let calls = Calls::default(); + let fence = FakeFence { + calls: calls.clone(), + failure: Some(Errno::EBUSY), + }; + let error = prepare(fence, FakeSystem::new(&calls)).unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Fence); + let CaptureCause::Io(cause) = error.cause() else { + panic!("{error:?}"); + }; + assert_eq!(cause.raw_os_error(), Some(Errno::EBUSY as i32)); + assert_eq!( + error.to_string(), + format!( + "failed to fence workload admission: {}", + Errno::EBUSY.desc() + ) + ); + // Nothing else runs, and the fence that did not close does not open. + assert_eq!(calls.take(), Vec::::new()); +} + +#[test] +fn a_failed_time_preparation_names_its_stage_and_opens_the_fence() { + let cases = [ + (Preparation::BootChecks, CaptureStage::BootChecks), + (Preparation::CaptureChecks, CaptureStage::CaptureChecks), + (Preparation::Pending, CaptureStage::Suppression), + ( + Preparation::Suppression(Errno::EROFS), + CaptureStage::Suppression, + ), + ]; + for (failure, stage) in cases { + let calls = Calls::default(); + let mut system = FakeSystem::new(&calls); + system.preparation = failure; + let error = prepare(fence(&calls), system).unwrap_err(); + assert_eq!(error.stage(), stage, "{failure:?}"); + match (failure, error.cause()) { + (Preparation::BootChecks | Preparation::CaptureChecks, CaptureCause::Time(_)) + | (Preparation::Pending, CaptureCause::Pending) => {} + (Preparation::Suppression(errno), CaptureCause::Io(cause)) + if cause.raw_os_error() == Some(errno as i32) => {} + _ => panic!("{failure:?}: {error:?}"), + } + // The time ABI left the detectors as they were, so the barrier restores nothing. + let mut expected = preparation_calls(); + expected.push(Call::Open); + assert_eq!(calls.take(), expected, "{failure:?}"); + } +} + +#[test] +fn cancel_and_drop_open_the_fence_then_restore_the_detectors() { + let calls = Calls::default(); + prepared(&calls, FakeSystem::new(&calls)).cancel().unwrap(); + assert_eq!(calls.take(), [Call::Open, Call::CancelCapture]); + + drop(prepared(&calls, FakeSystem::new(&calls))); + assert_eq!(calls.take(), [Call::Open, Call::CancelCapture]); +} + +#[test] +fn a_failed_restoration_of_the_detectors_is_reported_after_the_fence_opens() { + let failing = |calls: &Calls| { + let mut system = FakeSystem::new(calls); + system.restoration_fails = true; + system + }; + let text = "failed to restore the stall detectors: read /run/nvx/time-suppression: No such file \ + or directory"; + let calls = Calls::default(); + let error = prepared(&calls, failing(&calls)).cancel().unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Release); + assert_eq!(error.to_string(), text); + // The barrier releases once: dropping it after the failure releases nothing more. + assert_eq!(calls.take(), [Call::Open, Call::CancelCapture]); + + let error = prepared(&calls, failing(&calls)).capture().unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Release); + assert_eq!(error.to_string(), text); + assert_eq!( + calls.take(), + [ + Call::GenerationId, + Call::Capture(platform_request()), + Call::Open, + Call::CancelCapture, + ] + ); + + drop(prepared(&calls, failing(&calls))); + assert_eq!(calls.take(), [Call::Open, Call::CancelCapture]); +} + +#[test] +fn a_restored_clone_keeps_the_detectors_suppressed() { + let calls = Calls::default(); + let mut system = FakeSystem::new(&calls); + system.outcome = Outcome::Restored; + let Captured::Restored(restored) = prepared(&calls, system).capture().unwrap() else { + panic!("the clone continued"); + }; + // The fence opens, and the time daemon restores the detectors' settings once repair + // finishes the restore. + assert_eq!( + calls.take(), + [ + Call::GenerationId, + Call::Capture(platform_request()), + Call::Open, + ] + ); + assert_eq!(restored.restore().generation_id(), RESTORED); + assert_eq!( + restored.entropy_path(), + Path::new("/run/nvx/restore-entropy") + ); + assert_eq!(restored.into_restore(), Restore::to(RESTORED)); + assert_eq!(calls.take(), Vec::::new()); +} + +#[test] +fn a_failed_capture_releases_the_barrier() { + let calls = Calls::default(); + let mut system = FakeSystem::new(&calls); + system.generation_fails = true; + let error = prepared(&calls, system).capture().unwrap_err(); + assert_eq!(error.stage(), CaptureStage::GenerationId); + assert!( + matches!( + error.cause(), + CaptureCause::Time(TimeError::GenerationIdUnavailable) + ), + "{error:?}" + ); + assert_eq!( + error.to_string(), + "failed to read the VM generation ID: the VM generation ID is unavailable" + ); + assert_eq!( + calls.take(), + [Call::GenerationId, Call::Open, Call::CancelCapture] + ); + + let mut system = FakeSystem::new(&calls); + system.outcome = Outcome::Fails; + let error = prepared(&calls, system).capture().unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Capture); + assert_eq!( + error.to_string(), + "failed to request the capture: the time state is unavailable" + ); + assert!(std::error::Error::source(&error).is_some()); + assert_eq!( + calls.take(), + [ + Call::GenerationId, + Call::Capture(platform_request()), + Call::Open, + Call::CancelCapture, + ] + ); +} + +#[test] +fn errors_name_their_stage_and_their_cause() { + let error = CaptureError::new(CaptureStage::Suppression, CaptureCause::Pending); + assert_eq!( + error.to_string(), + "failed to suppress the stall detectors: a capture is already pending" + ); + assert!(std::error::Error::source(&error).is_none()); + assert!(matches!(error.into_cause(), CaptureCause::Pending)); + let stages = [ + (CaptureStage::Fence, "fence workload admission"), + (CaptureStage::Mounts, "check the mount table"), + ( + CaptureStage::BootChecks, + "wait for the time ABI's boot checks", + ), + ( + CaptureStage::CaptureChecks, + "run the time ABI's capture checks", + ), + (CaptureStage::Suppression, "suppress the stall detectors"), + (CaptureStage::GenerationId, "read the VM generation ID"), + (CaptureStage::Capture, "request the capture"), + (CaptureStage::Release, "restore the stall detectors"), + ]; + for (stage, text) in stages { + assert_eq!(stage.to_string(), text); + } + let origins = [ + (Origin::Distro, "the distro device"), + (Origin::Scratch, "the scratch device"), + (Origin::Overlay, "an overlay"), + (Origin::Filesystem, "the filesystem tag"), + ]; + for (origin, text) in origins { + assert_eq!(origin.to_string(), text); + } +} + +/// Writes `contents` to `path`, after creating its parent directories. +fn write(path: &Path, contents: impl AsRef<[u8]>) { + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, contents).unwrap(); +} + +#[test] +fn the_live_system_finds_windows_and_block_devices_in_procfs_and_sysfs() { + let directory = tempfile::tempdir().unwrap(); + let base = directory.path().canonicalize().unwrap(); + let paths = Paths { + proc: base.join("proc"), + sys: base.join("sys"), + dev: base.join("dev"), + }; + write( + &paths.proc.join("iomem"), + format!( + "00000000-00000fff : Reserved\n {DISTRO:08x}-{:08x} : virtio-mmio.0\n\ + {SCRATCH:08x}-{:08x} : virtio-mmio.1\n", + DISTRO + 0xfff, + SCRATCH + 0xfff, + ), + ); + // The distro device has a partition. The kernel lists the scratch's window, but has not added + // its device. + let window = paths.sys.join("devices/virtio-mmio-cmdline/virtio-mmio.0"); + let disk = window.join("virtio0/block/vda"); + fs::create_dir_all(disk.join("vda2")).unwrap(); + let blocks = paths.sys.join("dev/block"); + fs::create_dir_all(&blocks).unwrap(); + let target = "../../devices/virtio-mmio-cmdline/virtio-mmio.0/virtio0/block/vda"; + symlink(target, blocks.join("254:0")).unwrap(); + symlink(format!("{target}/vda2"), blocks.join("254:2")).unwrap(); + let mut system = LiveSystem { + paths: paths.clone(), + abi: TimeAbi::report_only(), + }; + assert_eq!(system.windows(DISTRO).unwrap(), [window]); + assert_eq!(system.windows(SCRATCH).unwrap(), Vec::::new()); + assert_eq!(system.windows(0x1000).unwrap(), Vec::::new()); + assert_eq!(system.block_device(number(254, 0)).unwrap(), disk); + assert_eq!( + system.block_device(number(254, 2)).unwrap(), + disk.join("vda2") + ); + let error = system.block_device(number(8, 0)).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::NotFound); + assert_eq!( + error.to_string(), + format!("{}: {}", blocks.join("8:0").display(), Errno::ENOENT.desc()) + ); + + // The check finds a layer in the partition, from the live system's answers. + let table = + format!("{TEMPLATE_TABLE}30 23 254:2 / /run/nvx/layers/p2 ro - ext4 /dev/vda2 ro\n"); + let mounted = mounts::first_blocking(&mut system, &devices(), table.as_bytes()) + .unwrap() + .unwrap(); + assert_eq!(mounted.origin(), Origin::Distro); + assert_eq!(mounted.mount_point(), Path::new("/run/nvx/layers/p2")); + + // The live system reads one byte more than the limit of a longer table, which the check + // refuses. + write( + &paths.proc.join("self/mountinfo"), + vec![b'\n'; MOUNT_TABLE_LIMIT + 10], + ); + assert_eq!(system.mount_table().unwrap().len(), MOUNT_TABLE_LIMIT + 1); + write(&paths.proc.join("self/mountinfo"), TEMPLATE_TABLE); + assert_eq!(system.mount_table().unwrap(), TEMPLATE_TABLE.as_bytes()); + + // A failed read names its file. + fs::remove_file(paths.proc.join("iomem")).unwrap(); + let error = system.windows(DISTRO).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::NotFound); + assert_eq!( + error.to_string(), + format!( + "{}: {}", + paths.proc.join("iomem").display(), + Errno::ENOENT.desc() + ) + ); + fs::remove_file(paths.proc.join("self/mountinfo")).unwrap(); + assert_eq!( + system.mount_table().unwrap_err().kind(), + io::ErrorKind::NotFound + ); +} diff --git a/nvx-rs/src/guest/mod.rs b/nvx-rs/src/guest/mod.rs index f567e6eb0..329c42cca 100644 --- a/nvx-rs/src/guest/mod.rs +++ b/nvx-rs/src/guest/mod.rs @@ -9,9 +9,11 @@ //! program as one execution, with its launch, its bounded output and input, its ending, and its //! outcome, alone or, through its [`binding`](exec::binding), under the //! [execution engine](crate::exec); [`root`], through which it bootstraps the guest for a layered -//! root, assembles the root, mounts live shares and mappings in it, and tears it down; and +//! root, assembles the root, mounts live shares and mappings in it, and tears it down; //! [`gpt`], through which it reads the partition table of a GPT disk that the kernel attached as -//! a block device. +//! a block device; and, on x86-64, `clone`, through which a template's guest prepares for and +//! requests a snapshot's capture before its root's assembly, behind the caller's fence of +//! workload admission, with nothing mounted from the devices that its clones replace. //! //! Each other module holds one program, and the crate's binary of the same name runs its `main`. //! Two of them also offer their steps as typed calls, which any guest init or agent runs in its @@ -82,6 +84,8 @@ mod kernel_text; pub use generation::GenerationId; pub mod cgroup; +#[cfg(target_arch = "x86_64")] +pub mod clone; pub mod console_pending; #[cfg(target_arch = "x86_64")] pub mod device_io; diff --git a/nvx-rs/src/guest/root.rs b/nvx-rs/src/guest/root.rs index ae049ea49..5b506c4b7 100644 --- a/nvx-rs/src/guest/root.rs +++ b/nvx-rs/src/guest/root.rs @@ -172,14 +172,16 @@ use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; use std::time::Duration; -use self::kernel::{Kernel, Paths, SystemKernel}; +use self::kernel::{Kernel, SystemKernel}; use super::gpt::DeviceError; use super::mappings::{Mapper, MappingError, below_its_root}; use super::sys::{self, Errno, MountFlags}; use crate::gpt::{Expected, GptError, Partition}; use crate::uuid::Uuid; +pub(crate) use self::block::window_directories; pub use self::bootstrap::{Bootstrap, BootstrapStep, Resources, bootstrap}; +pub(crate) use self::kernel::Paths; #[cfg(test)] pub(crate) use self::kernel::{Call, FakeKernel, Operation, attach_calls}; pub use self::share::{ShareChild, ShareKind, ShareRefusal}; diff --git a/nvx-rs/src/guest/root/block.rs b/nvx-rs/src/guest/root/block.rs index 07ca596b9..f79a37900 100644 --- a/nvx-rs/src/guest/root/block.rs +++ b/nvx-rs/src/guest/root/block.rs @@ -188,7 +188,21 @@ fn partition_node(disk: &Path, number: u8) -> PathBuf { /// Returns the name of the block device below the virtio MMIO window at `mmio`, or `None` while /// the kernel lists no such window, or has not probed it yet. fn find(paths: &Paths, mmio: u64) -> io::Result> { + for window in window_directories(paths, mmio)? { + if let Some(block) = block_device(&window)? { + return Ok(Some(block)); + } + } + Ok(None) +} + +/// Returns the sysfs directories of the virtio MMIO windows that the kernel lists at `mmio` in +/// `/proc/iomem`, in its order, below which it creates their virtio devices, and the block devices +/// and partitions of those. A directory may not exist yet: the kernel lists a window before it adds +/// the window's device. +pub(crate) fn window_directories(paths: &Paths, mmio: u64) -> io::Result> { let windows = fs::read(paths.proc.join("iomem"))?; + let mut directories = Vec::new(); for line in windows.split(|&byte| byte == b'\n') { // `START-END : NAME`, indented below the window that holds it. let mut fields = line @@ -205,11 +219,9 @@ fn find(paths: &Paths, mmio: u64) -> io::Result> { let Ok(name) = std::str::from_utf8(name) else { continue; }; - if let Some(block) = block_device(&paths.sys.join(WINDOW_DEVICES).join(name))? { - return Ok(Some(block)); - } + directories.push(paths.sys.join(WINDOW_DEVICES).join(name)); } - Ok(None) + Ok(directories) } /// Returns the name of the first block device, in byte order, of the first virtio device of the diff --git a/nvx-rs/src/guest/root/kernel.rs b/nvx-rs/src/guest/root/kernel.rs index 1d70c2e65..cfc3b488c 100644 --- a/nvx-rs/src/guest/root/kernel.rs +++ b/nvx-rs/src/guest/root/kernel.rs @@ -162,13 +162,13 @@ pub(super) fn super_options(table: &[u8], mount: u64) -> Option<&[u8]> { /// The directories of the kernel's interfaces that the root operations read. #[derive(Clone, Debug, PartialEq, Eq)] -pub(super) struct Paths { +pub(crate) struct Paths { /// The mount point of procfs. - pub(super) proc: PathBuf, + pub(crate) proc: PathBuf, /// The mount point of sysfs. - pub(super) sys: PathBuf, + pub(crate) sys: PathBuf, /// The directory of the device nodes. - pub(super) dev: PathBuf, + pub(crate) dev: PathBuf, } impl Default for Paths { diff --git a/nvx-rs/src/guest/time/api.rs b/nvx-rs/src/guest/time/api.rs index 3813fd157..db4f43a01 100644 --- a/nvx-rs/src/guest/time/api.rs +++ b/nvx-rs/src/guest/time/api.rs @@ -160,6 +160,27 @@ impl TimeAbi { pub fn restore_finish(&self, request: RestoreFinish) -> Result<(), TimeError> { released(restore_finish(&self.runtime(), request)) } + + /// Prepares the guest for a capture, as `nvx-time pre-capture` does: waits until the boot + /// checks have completed, for 30 seconds at most, since they start the time daemon that the + /// capture checks require, runs the capture checks, saves the stall detectors' settings and + /// suppresses the detectors, and records the capture checks. + /// + /// A capture check that fails reports a violation, which stops the VM in enforcing mode; a + /// report-only call goes on, as `nvx-time` does. A call that fails leaves the detectors as + /// they were: it restores the settings that it saved, and never touches those of a capture + /// that is pending already, [`PreCaptureError::Pending`]. + /// [`cancel_capture`](Self::cancel_capture) restores the settings that it saved. + pub(crate) fn pre_capture(&self) -> Result<(), PreCaptureError> { + pre_capture(&self.runtime()) + } + + /// Restores the stall detectors' settings that [`pre_capture`](Self::pre_capture) saved, and + /// removes their record, as `nvx-time cancel-capture` does, or fails with its diagnostic, + /// such as `read /run/nvx/time-suppression: No such file or directory`. + pub(crate) fn cancel_capture(&self) -> Result<(), Vec> { + system::suppression_restore(false) + } } impl Default for TimeAbi { @@ -259,6 +280,71 @@ fn read_generation_id(runtime: &Runtime<'_>) -> Result Ok(GenerationId::from_bytes(id)) } +/// Why [`TimeAbi::pre_capture`] failed. +#[derive(Debug)] +pub(crate) enum PreCaptureError { + /// The boot checks were still pending when the wait ended: a violation of check C10, which + /// a report-only call reports with its event. + BootChecks(TimeError), + /// The capture checks could not read the CPU sets: a violation of check C6, which a + /// report-only call reports with its event. + CaptureChecks(TimeError), + /// The stall detectors' settings are saved already, by a capture that is still pending. + Pending, + /// The stall detectors' settings could not be saved, or the detectors not suppressed, with + /// this error. The call restored the settings that it had saved. + Suppression(Errno), +} + +/// The preparation of [`TimeAbi::pre_capture`]. +fn pre_capture(runtime: &Runtime<'_>) -> Result<(), PreCaptureError> { + let (state, completed) = checks::wait_for_checks(true); + if !completed { + return Err(PreCaptureError::BootChecks(check_failed( + runtime, + Phase::Capture, + CheckId::C10, + "the boot checks are still pending", + ))); + } + let start = clock_ns(sys::Clock::Monotonic); + let cpu_start = cpu_time_ns(); + runtime.set_generation(state.generation as u32); + let mut checks = Checks::load(Phase::Capture).map_err(|_| { + PreCaptureError::CaptureChecks(check_failed( + runtime, + Phase::Capture, + CheckId::C6, + "cannot read the CPU sets", + )) + })?; + checks::run_capture_checks(runtime, &mut checks); + let elapsed = clock_ns(sys::Clock::Monotonic).wrapping_sub(start); + let cpu = cpu_time_ns().wrapping_sub(cpu_start); + if let Err(error) = system::suppression_save() { + if error == Errno::EEXIST { + return Err(PreCaptureError::Pending); + } + let _ = system::suppression_restore(false); + return Err(PreCaptureError::Suppression(error)); + } + if let Err(error) = checks::record_check( + runtime, + &checks, + elapsed / NSEC_PER_USEC, + cpu / NSEC_PER_USEC, + 0, + 0, + ) { + checks.fail( + runtime, + CheckId::C10, + format!("cannot record the capture check: {}", error.desc()), + ); + } + Ok(()) +} + /// How a snapshot treats the guest's scratch disk, which the capture request selects. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum ScratchPolicy { @@ -394,6 +480,21 @@ impl Restore { } } +#[cfg(test)] +impl Restore { + /// Returns an unfinished restore to the new VM generation ID `generation_id`, with nothing to + /// activate, as a test's fake capture returns one. + pub(crate) fn to(generation_id: GenerationId) -> Self { + Self { + flags: 0, + online_vp_count: 0, + generation_id, + memory_ranges: Vec::new(), + finished: false, + } + } +} + /// A range of guest physical memory that a restore adds. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub struct MemoryRange { diff --git a/nvx-rs/src/guest/time/commands.rs b/nvx-rs/src/guest/time/commands.rs index 1ef0a552d..84514d914 100644 --- a/nvx-rs/src/guest/time/commands.rs +++ b/nvx-rs/src/guest/time/commands.rs @@ -7,20 +7,19 @@ use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; use super::api::{ - self, CaptureOutcome, CaptureRequest, CpuSet, RestoreFinish, ScratchPolicy, TimeAbi, TimeError, + self, CaptureOutcome, CaptureRequest, CpuSet, PreCaptureError, RestoreFinish, ScratchPolicy, + TimeAbi, TimeError, }; use super::checks::{self, Checks}; use super::cpu_mask::CpuMask; use super::decoders::{self, GENERATION_ID_SIZE}; use super::events::CheckId; -use super::system::{ - self, PORTB_TIME_WINDOW, Runtime, clock_ns, cpu_time_ns, suppression_restore, suppression_save, -}; +use super::system::{self, PORTB_TIME_WINDOW, Runtime, clock_ns, cpu_time_ns}; use super::{NSEC_PER_USEC, Phase, USAGE}; use crate::guest::GenerationId; use crate::guest::parse::parse_decimal; use crate::guest::status::CommandError; -use crate::guest::sys::{self, Errno, Port, PortError}; +use crate::guest::sys::{self, Port, PortError}; /// The capture requests that `capture` accepts, and writes to the snapshot port: 0 or 1. const CAPTURE_REQUESTS: std::ops::RangeInclusive = 0..=1; @@ -145,64 +144,35 @@ fn cmd_check( /// Preparation steps 1 to 4: waits for the boot checks, which start the daemon that C10 /// requires, runs the capture checks, and suppresses the stall detectors before the snapshot. -fn cmd_pre_capture(runtime: &Runtime<'_>, err: &mut dyn Write) -> Result<(), CommandError> { - let (state, completed) = checks::wait_for_checks(true); - if !completed { - let mut checks = Checks::empty(Phase::Capture); - checks.fail(runtime, CheckId::C10, "the boot checks are still pending"); - return Err(CommandError::Operation); - } - let start = clock_ns(sys::Clock::Monotonic); - let cpu_start = cpu_time_ns(); - runtime.set_generation(state.generation as u32); - let mut checks = match Checks::load(Phase::Capture) { - Ok(checks) => checks, - Err(_) => { - let mut checks = Checks::empty(Phase::Capture); - checks.fail(runtime, CheckId::C6, "cannot read the CPU sets"); - return Err(CommandError::Operation); +fn cmd_pre_capture(abi: TimeAbi, err: &mut dyn Write) -> Result<(), CommandError> { + abi.pre_capture() + .map_err(|error| pre_capture_failure(err, error)) +} + +/// Reports why the preparation of a capture failed, such as `nvx-time: cannot suppress the stall +/// detectors: a capture is already pending`, and fails. A violation's event is its only report. +fn pre_capture_failure(err: &mut dyn Write, error: PreCaptureError) -> CommandError { + let reason = match error { + PreCaptureError::BootChecks(_) | PreCaptureError::CaptureChecks(_) => { + return CommandError::Operation; } + PreCaptureError::Pending => "a capture is already pending", + PreCaptureError::Suppression(error) => error.desc(), }; - checks::run_capture_checks(runtime, &mut checks); - let elapsed = clock_ns(sys::Clock::Monotonic).wrapping_sub(start); - let cpu = cpu_time_ns().wrapping_sub(cpu_start); - if let Err(error) = suppression_save() { - let reason = if error == Errno::EEXIST { - "a capture is already pending" - } else { - let _ = suppression_restore(false); - error.desc() - }; - report( - err, - &[ - b"nvx-time: cannot suppress the stall detectors: ", - reason.as_bytes(), - b"\n", - ], - ); - return Err(CommandError::Operation); - } - if let Err(error) = checks::record_check( - runtime, - &checks, - elapsed / NSEC_PER_USEC, - cpu / NSEC_PER_USEC, - 0, - 0, - ) { - checks.fail( - runtime, - CheckId::C10, - format!("cannot record the capture check: {}", error.desc()), - ); - } - Ok(()) + report( + err, + &[ + b"nvx-time: cannot suppress the stall detectors: ", + reason.as_bytes(), + b"\n", + ], + ); + CommandError::Operation } /// Cancels capture by restoring saved stall-detector settings. -fn cmd_cancel_capture(err: &mut dyn Write) -> Result<(), CommandError> { - suppression_restore(false).map_err(|detail| { +fn cmd_cancel_capture(abi: TimeAbi, err: &mut dyn Write) -> Result<(), CommandError> { + abi.cancel_capture().map_err(|detail| { report(err, &[b"nvx-time: ", &detail, b"\n"]); CommandError::Operation }) @@ -387,9 +357,9 @@ pub(super) fn run( let result = match (command, rest) { (b"boot", []) => cmd_boot(abi, &runtime, err), (b"check", arguments) => cmd_check(&runtime, arguments, err), - (b"pre-capture", []) => cmd_pre_capture(&runtime, err), + (b"pre-capture", []) => cmd_pre_capture(abi, err), (b"capture", arguments) => cmd_capture(abi, arguments, out, err), - (b"cancel-capture", []) => cmd_cancel_capture(err), + (b"cancel-capture", []) => cmd_cancel_capture(abi, err), (b"restore-finish", arguments) => cmd_restore_finish(abi, arguments, err), (b"status", []) => cmd_status(&runtime, out), (b"sample", []) => cmd_sample(&runtime, out, err), @@ -406,6 +376,42 @@ pub(super) fn run( #[cfg(test)] mod tests { use super::*; + use crate::guest::sys::Errno; + + #[test] + fn a_failed_preparation_reports_why_the_detectors_stay_unsuppressed() { + let cases = [ + ( + PreCaptureError::Pending, + "nvx-time: cannot suppress the stall detectors: a capture is already pending\n" + .to_owned(), + ), + ( + PreCaptureError::Suppression(Errno::EROFS), + format!( + "nvx-time: cannot suppress the stall detectors: {}\n", + Errno::EROFS.desc() + ), + ), + // A violation's event is its only report. + ( + PreCaptureError::BootChecks(TimeError::StateUnavailable), + String::new(), + ), + ( + PreCaptureError::CaptureChecks(TimeError::StateUnavailable), + String::new(), + ), + ]; + for (error, text) in cases { + let mut err = Vec::new(); + assert_eq!( + pre_capture_failure(&mut err, error), + CommandError::Operation + ); + assert_eq!(String::from_utf8(err).unwrap(), text); + } + } #[test] fn capture_requests_are_plain_decimal_zero_or_one() { diff --git a/nvx-rs/src/guest/time/mod.rs b/nvx-rs/src/guest/time/mod.rs index e26dc5f3c..d263bb158 100644 --- a/nvx-rs/src/guest/time/mod.rs +++ b/nvx-rs/src/guest/time/mod.rs @@ -63,6 +63,7 @@ use std::ffi::CString; use std::io::{self, Write}; use std::process::ExitCode; +pub(crate) use api::PreCaptureError; pub use api::{ CaptureOutcome, CaptureRequest, ColdBoot, CpuSet, MemoryRange, Restore, RestoreFinish, ScratchPolicy, TimeAbi, TimeError, Violation, diff --git a/nvx-rs/src/lib.rs b/nvx-rs/src/lib.rs index 00c51adf6..c03b3e88b 100644 --- a/nvx-rs/src/lib.rs +++ b/nvx-rs/src/lib.rs @@ -49,9 +49,10 @@ //! execution engine (`guest::exec::binding`), bootstraps the guest for a layered root, //! assembles the root, mounts live shares and mappings in it, and tears it down //! (`guest::root`), and reads the partition table of a GPT disk's block device (`guest::gpt`), -//! and the typed calls through which it runs the time ABI's steps (`guest::time`) and reseeds -//! after a restore (`guest::reseed`) in its own process. The crate's `nvx-*` guest binaries run -//! the programs. +//! the typed calls through which it runs the time ABI's steps (`guest::time`) and reseeds after +//! a restore (`guest::reseed`) in its own process, and the preparation of a template's guest +//! for a capture before its root's assembly (`guest::clone`). The crate's `nvx-*` guest +//! binaries run the programs. //! //! # Example //! From 1b0ba5d3e8b67e6be97f62011359b9114d30c3ee Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 11:20:46 -0700 Subject: [PATCH 2/7] guest: keep a clone's admission closed until repair, and own the suppression Review of the capture preparation found three gaps, which this fixes. A restored clone runs nothing before repair acknowledges the restore, so the barrier no longer opens the fence on Captured::Restored: Restored holds the caller's fence, still closed, and Restored::into_parts hands it over with the restore. A capture request that fails may fail in a clone whose restore it could not repair, so it restores the stall detectors' settings but leaves the fence closed too. TimeAbi::pre_capture now fails, as the other calls do, when the thread may still hold some of the permission for the time ABI's ports, once it restored the detectors' settings that it saved. nvx-time pre-capture still reports that once, but no longer leaves the detectors suppressed after it. The detectors' suppression now saves under the lock that its restore takes, so that no restore runs between its reads and its writes, and a save that fails undoes only what it changed: it never restores another capture's record. Part of #840 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da708b6-0bda-4804-b01b-936cdc5f054a --- nvx-rs/README.md | 2 +- nvx-rs/src/guest/clone.rs | 161 +++++++++------ nvx-rs/src/guest/clone/tests.rs | 69 +++++-- nvx-rs/src/guest/time/api.rs | 78 +++++++- nvx-rs/src/guest/time/commands.rs | 12 +- nvx-rs/src/guest/time/system.rs | 321 +++++++++++++++++++++++++----- 6 files changed, 500 insertions(+), 143 deletions(-) diff --git a/nvx-rs/README.md b/nvx-rs/README.md index ab11b447e..f78b3f451 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -114,7 +114,7 @@ nvx-rs/ | `nvx::guest::exec` | `Execution`: drives one program that the caller spawned with the program's ends of its `Pipes`, or that the caller started itself and adopts with its own ends of the program's pipes and a `Child` that names it, without blocking, from the caller's own loop or reactor, and reports each step as an `Event`: the launch report, which an adopted program has at once, the output of each stream, kill failures, and the `Outcome`. It reads a stream only as far as the caller's `grant` allows, so a stalled sink stalls its own execution alone, writes the program's standard input without `SIGPIPE`, and ends the program on a cancellation, a timeout, or a disconnection, with `SIGKILL` through the program's pidfd to the program and, if the caller asks, its process group, while no collector has taken its status, and, through the opened directory of a cgroup that the caller names, to that cgroup. It closes the program's input once the program has ended. It waits for its program alone, by a pidfd, or takes the status from the caller's collector with `observe`; with `Options::caller_reaps`, that collector is the program's only one. NVX's managed agent is one caller | | `nvx::guest::exec::binding` | `Binding`: an `nvx::exec::Runner` bound to an `Execution`, the glue that a guest adapter would otherwise write. The runner's read allowances become grants, its discards discards, the input that it accepted writes and then a close, its terminations a cancellation, a disconnection, or a termination, and its settlement a kill of the execution's cgroup and checks until no process occupies it; without a cgroup, nothing proves that the program's descendants are gone, so the execution is fenced. It feeds the execution's events back to the runner, statuses that a caller who reaps hands over with `observe` included, and never blocks: the caller polls its descriptors and its input writer until its deadline, and each call takes a bounded number of steps, so that no execution holds the caller's loop | | `nvx::guest::time` | x86_64: `TimeAbi`, the steps of NVX time ABI v1 as typed calls in the caller's process, for any guest init or agent: the cold-boot time step and, separately, the start of the time daemon in a child process that the caller supervises, the VM generation ID, a `CaptureRequest` that repairs a `Restore`, and a `RestoreFinish`. A call takes the permission for OpenVMM's ports on the calling thread for each port transaction alone, and fails with `TimeError::PortRelease` if the thread may still hold some of it, which a process that it started would inherit, and from which `nvx::guest::spawn` starts none. No call waits for a child that it did not start. `nvx-time` runs its steps through them | -| `nvx::guest::clone` | x86_64: the preparation of a template's guest for a capture before its root's assembly, for a guest init or agent, whether or not it is PID 1. `CapturePreparation`, of the caller's `AdmissionFence` and the `Devices` that the template's clones replace, the `distro` and scratch block devices, which their MMIO windows name, and the host filesystem device's virtio-fs tag: `prepare` closes the fence, so that no execution starts, refuses with a `Mounted` the first mount from those devices, a distro partition included, or from an overlay, bind mounts included, in a bounded read of `/proc/self/mountinfo` of at most `MOUNT_TABLE_LIMIT` bytes, then waits for the time ABI's boot checks, runs its capture checks, and suppresses the stall detectors, as `nvx-time pre-capture` does, and returns an owned `CaptureBarrier`. `CaptureBarrier::capture` requests the capture with the platform tier's request, through `TimeAbi::capture`, and returns `Captured::Continued` once it opened the fence and restored the detectors' settings, or, in a clone, `Captured::Restored`, with the `Restore` that repair takes next, once it opened the fence, with the detectors suppressed until the time daemon restores their settings. `CaptureBarrier::cancel`, a failed capture, and a dropped barrier open the fence and restore the detectors' settings. Each failure is a `CaptureError`, with its `CaptureStage` and `CaptureCause` | +| `nvx::guest::clone` | x86_64: the preparation of a template's guest for a capture before its root's assembly, for a guest init or agent, whether or not it is PID 1. `CapturePreparation`, of the caller's `AdmissionFence` and the `Devices` that the template's clones replace, the `distro` and scratch block devices, which their MMIO windows name, and the host filesystem device's virtio-fs tag: `prepare` closes the fence, so that no execution starts, refuses with a `Mounted` the first mount from those devices, a distro partition included, or from an overlay, bind mounts included, in a bounded read of `/proc/self/mountinfo` of at most `MOUNT_TABLE_LIMIT` bytes, then waits for the time ABI's boot checks, runs its capture checks, and suppresses the stall detectors, as `nvx-time pre-capture` does, and returns an owned `CaptureBarrier`. `CaptureBarrier::capture` requests the capture with the platform tier's request, through `TimeAbi::capture`, and returns `Captured::Continued` once it opened the fence and restored the detectors' settings, or, in a clone, `Captured::Restored`, with the fence still closed, since nothing else may run before repair acknowledges the restore, and the detectors suppressed until the time daemon restores their settings: `Restored::into_parts` hands the `Restore` that repair takes next and the closed fence to the caller, which opens it once repair acknowledged the restore. `CaptureBarrier::cancel`, a dropped barrier, and a failure to read the VM generation ID open the fence and restore the detectors' settings; a capture request that fails restores the settings but leaves the fence closed, since it may fail in a clone whose restore it could not repair. Each failure is a `CaptureError`, with its `CaptureStage` and `CaptureCause` | | `nvx::guest::reseed` | `Seed::read` and `reseed`: a restore's fresh entropy, which starts with its `GenerationId`, and the reseed of the kernel's random number generator with it, in the caller's process. `nvx-reseed` runs them | | `nvx::guest::root` | The operations of a layered root, for a guest init or agent, whether or not it is PID 1. `bootstrap`: the agent's and the workload's sibling cgroups of a `Layout`, with every process of their parent moved into the agent's, the agent's `memory.low`, and the workload cgroup prepared through `ExecutionCgroup::prepare`, with its `memory.max` and `pids.max`, and user namespaces kept unavailable. `assemble`: the `Assembly`'s block devices, found by their MMIO windows, checked against their roles' access modes and their expected sizes, and flushed, and the `Root` that it mounts, at a runtime directory, a canonical path that no symbolic link leads through, that it holds exclusively until its teardown, where nothing is mounted already, once it has checked everything: the read-only layers of the `Image` that the caller names, which it never probes: an EROFS image, whose expected `nvx::uuid::Uuid` it checks in its superblock, or a GPT disk whose partition table it reads and checks with `nvx::guest::gpt::read_disk`, against the caller's `nvx::gpt::Expected` values, whose first partition it reads, up to the caller's bound, as opaque bytes that it never mounts, and whose later partitions, at least one, hold ext4 layers that it mounts `ro,nosuid,nodev` with `errors=remount-ro`, so that an error that the kernel finds in a layer fails the operation that found it, rather than panic the guest, as the layer's superblock may ask, from the block devices that the kernel created when it found the disk, once their superblocks show that no journal needs a recovery, which their read-only device can't take, partition 2's at the overlay's base and the last partition's at its top, under an overlay mounted `nosuid,nodev` with the one option set of the caller's `OverlayMode`, `metacopy=on,xino=on` or, for lowers that may carry untrusted overlay xattrs, `metacopy=off,redirect_dir=nofollow`, with no fallback, whose upper and work directories are on the `Upper`: an ext4 scratch mounted `nosuid,nodev`, whose superblock it checks for a journal that the caller's `Journal` policy forbids, or a tmpfs mounted `nosuid,nodev` at the kernel's default size. It refuses overlay options longer than `MOUNT_OPTIONS_LIMIT`, the most that a mount passes, which, below `/run/nvx`, fit up to 195 layers of a GPT disk, and an overlay that the kernel's mount table shows read-only or without the mode's options in effect, since the kernel mounts one so, rather than fail, when its upper can't hold its work directory or its extended attributes. The `Root` reports the first partition's bytes, its `Lower` layers, its `UpperKind`, and its `OverlayMode`. `Root::mount_share` and `Root::mount_aggregate`: virtio-fs live shares at targets in the root that it checks, against the targets that `protocol::share` reserves among others, with one aggregate per root, which it attaches through its `mappings::Mapper` beneath the overlay, with NVX's container root's reserved targets: a share's device mounted out of the root, then moved onto its target, and a copy of each child's entry of the aggregate. `Root::mount_export`: the host export, the aggregate device of `protocol::export`, one per root, mounted `nosuid,nodev` with the access of its `ExportMode` at `hostfs/root` in the runtime directory, out of the root, below a directory that only root may enter, whose root it returns as the `SourceRoot` of the caller's mappings. `Root::map`: a caller's host mappings, through the same mapper. A share, a child, the export, or a mapping that fails latches the root. `Root::teardown`: the shares and mappings in reverse order, then the mounts of an aggregate and of the export, the later first, the overlay, the layers from the top down, and the upper, with every failed unmount reported. Each failure is a typed `Error`. The operations collect no child's status, change neither the caller's root nor its credentials, start no async runtime, and never power the VM off | | `nvx::guest::gpt` | `read_disk`: the partition table of a GPT disk, read and checked with `nvx::gpt`'s `Disk::read` through the disk's block device, which the caller opened, at the logical block size and size that `BLKSSZGET` and `BLKGETSIZE64` report, with the caller's `Expected` values, such as the disk GUID that a boot parameter names. It only reads the device, at explicit offsets, so it never moves the file's position. It neither compares the partitions that the kernel created with the table nor asks the kernel to reread the table. Each failure is a typed `DeviceError` | diff --git a/nvx-rs/src/guest/clone.rs b/nvx-rs/src/guest/clone.rs index 8a9f1bd37..c85b93fa8 100644 --- a/nvx-rs/src/guest/clone.rs +++ b/nvx-rs/src/guest/clone.rs @@ -26,15 +26,19 @@ //! - [`Captured::Continued`]: the VM continued without a restore, and the barrier opened the //! fence, then restored the detectors' settings. //! - [`Captured::Restored`]: a restore of the snapshot started this VM, a clone whose wall clock -//! the time ABI repaired, and the barrier opened the fence. The detectors stay suppressed until -//! the restore's finish, [`TimeAbi::restore_finish`], hands their release to the time daemon. -//! The caller repairs the clone next, from the [`Restored`] restore. OpenVMM ends a platform +//! the time ABI repaired. Nothing else may run before repair acknowledges the restore, so the +//! fence stays closed: [`Restored::into_parts`] hands it to the caller, which opens it once +//! repair acknowledged the restore, or later. The detectors stay suppressed until the +//! restore's finish, [`TimeAbi::restore_finish`], hands their release to the time daemon. The +//! caller repairs the clone next, from the [`Restored`] restore. OpenVMM ends a platform //! template's VM once its capture commits, so only clones return with this outcome. //! -//! [`CaptureBarrier::cancel`] opens the fence, then restores the detectors' settings. So does a -//! capture that fails, after which the caller ends the VM, and so does a barrier that is dropped, -//! which ignores a failure. A preparation that fails releases what it took. A failure is a -//! [`CaptureError`], which names the [`CaptureStage`] that failed and its [`CaptureCause`]. +//! [`CaptureBarrier::cancel`] opens the fence, then restores the detectors' settings, and so does +//! a barrier that is dropped, which ignores a failure. A capture that fails restores the +//! detectors' settings too, after which the caller ends the VM, but a request that fails leaves +//! the fence closed: it may fail in a clone whose restore it could not repair. A preparation that +//! fails releases what it took. A failure is a [`CaptureError`], which names the +//! [`CaptureStage`] that failed and its [`CaptureCause`]. //! //! ```no_run //! use std::io; @@ -70,8 +74,11 @@ //! match barrier.capture()? { //! Captured::Continued => println!("the VM continued without a restore"), //! Captured::Restored(restored) => { -//! // A clone continues here, and repairs the restore. -//! println!("restored to {}", restored.restore().generation_id()); +//! // A clone continues here: it repairs the restore, and opens the fence once repair +//! // acknowledged it. +//! let (restore, fence) = restored.into_parts(); +//! println!("restored to {}", restore.generation_id()); +//! fence.open(); //! } //! } //! # Ok(()) @@ -121,11 +128,12 @@ const ENTROPY_PATH: &str = "/run/nvx/restore-entropy"; /// execution: while the fence is closed, no execution starts. /// /// A [`CapturePreparation`] closes it before anything else, and its [`CaptureBarrier`] opens it -/// again once the capture continues, restores, fails, or is canceled, or the barrier is dropped. -/// The fence need not end the executions that run already: a template's guest captures before -/// it starts any. A preparation takes the fence, so a caller that keeps its admission's state -/// lends it a handle of that state, or a mutable reference to its fence, which the trait's -/// implementation for `&mut F` takes. +/// again once the capture continues or is canceled, or the barrier is dropped. A clone that a +/// restore started gets the fence back closed, in its [`Restored`], and a capture request that +/// fails leaves it closed. The fence need not end the executions that run already: a template's +/// guest captures before it starts any. A preparation takes the fence, so a caller that keeps +/// its admission's state lends it a handle of that state, or a mutable reference to its fence, +/// which the trait's implementation for `&mut F` takes. pub trait AdmissionFence { /// Closes the fence: once it returns, no execution starts until [`open`](Self::open). A /// failure leaves the fence open. @@ -186,7 +194,10 @@ impl CapturePreparation { /// The wait for the boot checks lasts 30 seconds at most. A capture check that fails reports /// a violation of the time ABI, which stops the VM in enforcing mode; in report-only mode, /// the preparation goes on, as `nvx-time pre-capture` does. A preparation that fails opens - /// the fence again, and leaves the detectors' settings as they were. + /// the fence again, and leaves the detectors' settings as they were. One after which the + /// calling thread may still hold some of the permission for the time ABI's ports fails with + /// [`TimeError::PortRelease`], after which the thread must start neither a thread nor a + /// process, which would inherit it. pub fn prepare(self) -> Result, CaptureError> { let system = LiveSystem { paths: Paths::default(), @@ -197,25 +208,23 @@ impl CapturePreparation { /// Implements [`prepare`](Self::prepare) on `system`. fn prepare_with(self, system: Box) -> Result, CaptureError> { - let Self { fence, devices } = self; + let Self { mut fence, devices } = self; + // A fence that failed to close is dropped closed or not, but never opened. + fence + .close() + .map_err(|error| CaptureError::new(CaptureStage::Fence, CaptureCause::Io(error)))?; let mut barrier = CaptureBarrier { - fence, - fenced: false, + fence: Some(fence), suppressed: false, system, }; - barrier - .fence - .close() - .map_err(|error| CaptureError::new(CaptureStage::Fence, CaptureCause::Io(error)))?; - barrier.fenced = true; check_mounts(barrier.system.as_mut(), &devices) .map_err(|cause| CaptureError::new(CaptureStage::Mounts, cause))?; barrier.system.pre_capture().map_err(|error| match error { PreCaptureError::BootChecks(error) => { CaptureError::new(CaptureStage::BootChecks, CaptureCause::Time(error)) } - PreCaptureError::CaptureChecks(error) => { + PreCaptureError::CaptureChecks(error) | PreCaptureError::PortRelease(error) => { CaptureError::new(CaptureStage::CaptureChecks, CaptureCause::Time(error)) } PreCaptureError::Pending => { @@ -260,10 +269,9 @@ fn check_mounts(system: &mut dyn System, devices: &Devices) -> Result<(), Captur /// [module](self). #[must_use = "dropping the barrier opens the fence and restores the stall detectors"] pub struct CaptureBarrier { - /// The caller's fence of workload admission. - fence: F, - /// Whether the barrier closed the fence, which it opens again. - fenced: bool, + /// The caller's fence of workload admission, which the barrier holds closed, until it opens + /// it, or hands it over closed. + fence: Option, /// Whether the barrier suppressed the stall detectors, whose settings it restores. suppressed: bool, /// The kernel and the time ABI. @@ -278,10 +286,15 @@ impl CaptureBarrier { /// A VM that continues without a restore gets [`Captured::Continued`], once the barrier /// opened the fence and restored the detectors' settings; a failure to restore them is a /// [`CaptureStage::Release`] error, after which the caller ends the VM. A clone that a - /// restore of the snapshot started gets [`Captured::Restored`], once the barrier opened the - /// fence, with the detectors still suppressed: see the [module](self). A capture that fails - /// opens the fence and restores the detectors' settings, and the caller ends the VM. - pub fn capture(mut self) -> Result { + /// restore of the snapshot started gets [`Captured::Restored`], with the fence still closed + /// and the detectors still suppressed: see the [module](self). + /// + /// A failure to read the VM generation ID, before the request, opens the fence and restores + /// the detectors' settings, as [`cancel`](Self::cancel) does. A request that fails restores + /// the detectors' settings, but leaves the fence closed, since it may fail in a clone whose + /// restore it could not repair, where no execution may start. Either way, the caller ends + /// the VM. + pub fn capture(mut self) -> Result, CaptureError> { let generation_id = self.system.generation_id().map_err(|error| { CaptureError::new(CaptureStage::GenerationId, CaptureCause::Time(error)) })?; @@ -290,10 +303,17 @@ impl CaptureBarrier { generation_id, PathBuf::from(ENTROPY_PATH), ); - let outcome = self - .system - .capture(request) - .map_err(|error| CaptureError::new(CaptureStage::Capture, CaptureCause::Time(error)))?; + let outcome = match self.system.capture(request) { + Ok(outcome) => outcome, + Err(error) => { + // The fence is dropped closed. + self.fence = None; + return Err(CaptureError::new( + CaptureStage::Capture, + CaptureCause::Time(error), + )); + } + }; match outcome { CaptureOutcome::Continued => { self.release()?; @@ -301,10 +321,13 @@ impl CaptureBarrier { } CaptureOutcome::Restored(restore) => { // The time daemon restores the detectors' settings once repair finishes the - // restore. + // restore, and the caller opens the fence once repair acknowledged it. self.suppressed = false; - self.open(); - Ok(Captured::Restored(Restored { restore })) + let fence = self + .fence + .take() + .expect("a barrier holds the fence until it releases it"); + Ok(Captured::Restored(Restored { restore, fence })) } } } @@ -316,16 +339,11 @@ impl CaptureBarrier { self.release() } - /// Opens the fence, if the barrier holds it closed. - fn open(&mut self) { - if mem::take(&mut self.fenced) { - self.fence.open(); - } - } - /// Opens the fence, then restores the stall detectors' settings, if the barrier holds them. fn release(&mut self) -> Result<(), CaptureError> { - self.open(); + if let Some(mut fence) = self.fence.take() { + fence.open(); + } if mem::take(&mut self.suppressed) { self.system.cancel_capture().map_err(|detail| { let detail = String::from_utf8_lossy(&detail).into_owned(); @@ -351,7 +369,7 @@ impl fmt::Debug for CaptureBarrier { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter .debug_struct("CaptureBarrier") - .field("fenced", &self.fenced) + .field("fenced", &self.fence.is_some()) .field("suppressed", &self.suppressed) .finish_non_exhaustive() } @@ -359,25 +377,38 @@ impl fmt::Debug for CaptureBarrier { /// How a capture that [`CaptureBarrier::capture`] requested ended, in the VM in which it /// returned. -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum Captured { +pub enum Captured { /// The VM continued without a restore, as [`CaptureOutcome::Continued`] tells: no snapshot /// was taken, or the restore packet was of the recorded generation. The barrier opened the /// fence and restored the stall detectors' settings. Continued, /// A restore of the snapshot started the VM, a clone, whose wall clock the time ABI repaired. - /// The barrier opened the fence, and the stall detectors stay suppressed. - Restored(Restored), + /// The fence stays closed, and the stall detectors stay suppressed. + Restored(Restored), } -/// A clone's restore, which [`CaptureBarrier::capture`] returns for repair to take next. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct Restored { +impl fmt::Debug for Captured { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Continued => formatter.write_str("Continued"), + Self::Restored(restored) => formatter.debug_tuple("Restored").field(restored).finish(), + } + } +} + +/// A clone's restore, which [`CaptureBarrier::capture`] returns for repair to take next, with +/// the caller's fence, which stays closed until the caller opens it. +/// +/// No execution may start before repair acknowledged the restore, so the caller opens the fence +/// only then, or later. Dropping it leaves the fence closed. +pub struct Restored { /// The restore that the time ABI repaired, as its restore packet describes it. restore: Restore, + /// The caller's fence of workload admission, closed. + fence: F, } -impl Restored { +impl Restored { /// Returns the restore that the time ABI repaired: its new VM generation ID, its targets, and /// whether the host gates its input until the guest acknowledges it. pub fn restore(&self) -> &Restore { @@ -391,9 +422,18 @@ impl Restored { Path::new(ENTROPY_PATH) } - /// Returns the restore that the time ABI repaired. - pub fn into_restore(self) -> Restore { - self.restore + /// Returns the restore that the time ABI repaired, and the caller's fence, still closed. + pub fn into_parts(self) -> (Restore, F) { + (self.restore, self.fence) + } +} + +impl fmt::Debug for Restored { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("Restored") + .field("restore", &self.restore) + .finish_non_exhaustive() } } @@ -407,7 +447,8 @@ pub enum CaptureStage { Mounts, /// Waiting for the time ABI's boot checks. BootChecks, - /// Running the time ABI's capture checks. + /// Running the time ABI's capture checks, after which the calling thread must hold none of + /// the permission for the time ABI's ports. CaptureChecks, /// Saving the stall detectors' settings and suppressing the detectors. Suppression, diff --git a/nvx-rs/src/guest/clone/tests.rs b/nvx-rs/src/guest/clone/tests.rs index bc0458910..f16c302a6 100644 --- a/nvx-rs/src/guest/clone/tests.rs +++ b/nvx-rs/src/guest/clone/tests.rs @@ -11,7 +11,7 @@ use std::sync::{Arc, Mutex, PoisonError}; use super::mounts::{self, DeviceNumber}; use super::*; -use crate::guest::sys::Errno; +use crate::guest::sys::{Errno, PortError}; /// The base address of the distro device's MMIO window. const DISTRO: u64 = 0xd000_3000; @@ -117,6 +117,8 @@ enum Preparation { Pending, /// The detectors' settings cannot be saved, with this error number. Suppression(Errno), + /// The thread may still hold some of the permission for the ports, with this error number. + PortRelease(Errno), } /// How the fake capture ends. @@ -235,6 +237,9 @@ impl System for FakeSystem { } Preparation::Pending => Err(PreCaptureError::Pending), Preparation::Suppression(errno) => Err(PreCaptureError::Suppression(errno)), + Preparation::PortRelease(errno) => Err(PreCaptureError::PortRelease( + PortError::Revoke(errno).into(), + )), } } @@ -337,7 +342,7 @@ fn the_fence_closes_before_the_checks_and_the_detectors_are_suppressed_before_th let barrier = prepare(fence(&calls), FakeSystem::new(&calls)).unwrap(); // The fence first, then the mount table, then the time ABI, which suppresses the detectors. assert_eq!(calls.take(), preparation_calls()); - assert_eq!(barrier.capture().unwrap(), Captured::Continued); + assert!(matches!(barrier.capture().unwrap(), Captured::Continued)); // The capture's request follows the suppression. A VM that continues opens the fence, then // restores the detectors' settings. assert_eq!( @@ -576,6 +581,10 @@ fn a_failed_time_preparation_names_its_stage_and_opens_the_fence() { Preparation::Suppression(Errno::EROFS), CaptureStage::Suppression, ), + ( + Preparation::PortRelease(Errno::ENOMEM), + CaptureStage::CaptureChecks, + ), ]; for (failure, stage) in cases { let calls = Calls::default(); @@ -588,6 +597,15 @@ fn a_failed_time_preparation_names_its_stage_and_opens_the_fence() { | (Preparation::Pending, CaptureCause::Pending) => {} (Preparation::Suppression(errno), CaptureCause::Io(cause)) if cause.raw_os_error() == Some(errno as i32) => {} + (Preparation::PortRelease(_), CaptureCause::Time(TimeError::PortRelease(_))) => { + assert_eq!( + error.to_string(), + format!( + "failed to run the time ABI's capture checks: ioperm revoke: {}", + Errno::ENOMEM.desc() + ) + ); + } _ => panic!("{failure:?}: {error:?}"), } // The time ABI left the detectors as they were, so the barrier restores nothing. @@ -641,34 +659,42 @@ fn a_failed_restoration_of_the_detectors_is_reported_after_the_fence_opens() { } #[test] -fn a_restored_clone_keeps_the_detectors_suppressed() { +fn a_restored_clone_gets_the_fence_closed_with_the_detectors_suppressed() { let calls = Calls::default(); - let mut system = FakeSystem::new(&calls); - system.outcome = Outcome::Restored; - let Captured::Restored(restored) = prepared(&calls, system).capture().unwrap() else { - panic!("the clone continued"); + let restored = |calls: &Calls| { + let mut system = FakeSystem::new(calls); + system.outcome = Outcome::Restored; + let Captured::Restored(restored) = prepared(calls, system).capture().unwrap() else { + panic!("the clone continued"); + }; + restored }; - // The fence opens, and the time daemon restores the detectors' settings once repair - // finishes the restore. + let clone = restored(&calls); + // Nothing opens the fence before repair, and the time daemon restores the detectors' + // settings once repair finishes the restore. assert_eq!( calls.take(), - [ - Call::GenerationId, - Call::Capture(platform_request()), - Call::Open, - ] + [Call::GenerationId, Call::Capture(platform_request())] ); - assert_eq!(restored.restore().generation_id(), RESTORED); + assert_eq!(clone.restore().generation_id(), RESTORED); + assert_eq!(clone.entropy_path(), Path::new("/run/nvx/restore-entropy")); + let (restore, mut fence) = clone.into_parts(); + assert_eq!(restore, Restore::to(RESTORED)); + assert_eq!(calls.take(), Vec::::new()); + // The caller opens the fence once repair acknowledged the restore. + fence.open(); + assert_eq!(calls.take(), [Call::Open]); + + // A clone that is dropped leaves the fence closed. + drop(restored(&calls)); assert_eq!( - restored.entropy_path(), - Path::new("/run/nvx/restore-entropy") + calls.take(), + [Call::GenerationId, Call::Capture(platform_request())] ); - assert_eq!(restored.into_restore(), Restore::to(RESTORED)); - assert_eq!(calls.take(), Vec::::new()); } #[test] -fn a_failed_capture_releases_the_barrier() { +fn a_failed_capture_restores_the_detectors_and_opens_the_fence_only_before_its_request() { let calls = Calls::default(); let mut system = FakeSystem::new(&calls); system.generation_fails = true; @@ -690,6 +716,8 @@ fn a_failed_capture_releases_the_barrier() { [Call::GenerationId, Call::Open, Call::CancelCapture] ); + // A request that fails may fail in a clone that its restore left unrepaired, so the fence + // stays closed. let mut system = FakeSystem::new(&calls); system.outcome = Outcome::Fails; let error = prepared(&calls, system).capture().unwrap_err(); @@ -704,7 +732,6 @@ fn a_failed_capture_releases_the_barrier() { [ Call::GenerationId, Call::Capture(platform_request()), - Call::Open, Call::CancelCapture, ] ); diff --git a/nvx-rs/src/guest/time/api.rs b/nvx-rs/src/guest/time/api.rs index db4f43a01..52b7aeece 100644 --- a/nvx-rs/src/guest/time/api.rs +++ b/nvx-rs/src/guest/time/api.rs @@ -168,16 +168,21 @@ impl TimeAbi { /// /// A capture check that fails reports a violation, which stops the VM in enforcing mode; a /// report-only call goes on, as `nvx-time` does. A call that fails leaves the detectors as - /// they were: it restores the settings that it saved, and never touches those of a capture - /// that is pending already, [`PreCaptureError::Pending`]. + /// they were: it restores the settings that it changed, and never touches those of a capture + /// that is pending already, [`PreCaptureError::Pending`]. Like the other calls, it fails with + /// [`TimeError::PortRelease`], as [`PreCaptureError::PortRelease`], if the thread may still + /// hold some of the permission for the ports, once it restored the detectors' settings. /// [`cancel_capture`](Self::cancel_capture) restores the settings that it saved. pub(crate) fn pre_capture(&self) -> Result<(), PreCaptureError> { - pre_capture(&self.runtime()) + released_preparation(pre_capture(&self.runtime()), sys::check_released(), || { + let _ = system::suppression_restore(false); + }) } /// Restores the stall detectors' settings that [`pre_capture`](Self::pre_capture) saved, and /// removes their record, as `nvx-time cancel-capture` does, or fails with its diagnostic, - /// such as `read /run/nvx/time-suppression: No such file or directory`. + /// such as `read /run/nvx/time-suppression: No such file or directory`. It takes no + /// permission for the ports. pub(crate) fn cancel_capture(&self) -> Result<(), Vec> { system::suppression_restore(false) } @@ -206,6 +211,23 @@ fn released(result: Result) -> Result { result } +/// Ends a preparation of a capture with `result`, unless the calling thread may still hold port +/// permission, as `released` tells, which the preparation reports instead, once `restore` +/// restored the stall detectors' settings that a preparation that succeeded saved. +fn released_preparation( + result: Result<(), PreCaptureError>, + released: Result<(), PortError>, + restore: impl FnOnce(), +) -> Result<(), PreCaptureError> { + let Err(error) = released else { + return result; + }; + if result.is_ok() { + restore(); + } + Err(PreCaptureError::PortRelease(error.into())) +} + /// The outcome of the cold-boot time step, from which [`TimeAbi::start_daemon`] starts the time /// daemon. #[derive(Debug, PartialEq, Eq)] @@ -292,8 +314,12 @@ pub(crate) enum PreCaptureError { /// The stall detectors' settings are saved already, by a capture that is still pending. Pending, /// The stall detectors' settings could not be saved, or the detectors not suppressed, with - /// this error. The call restored the settings that it had saved. + /// this error. The call restored the settings that it had changed. Suppression(Errno), + /// The thread may still hold some of the permission for the ports, a + /// [`TimeError::PortRelease`], after which it must start neither a thread nor a process. The + /// call restored the stall detectors' settings. + PortRelease(TimeError), } /// The preparation of [`TimeAbi::pre_capture`]. @@ -322,11 +348,12 @@ fn pre_capture(runtime: &Runtime<'_>) -> Result<(), PreCaptureError> { let elapsed = clock_ns(sys::Clock::Monotonic).wrapping_sub(start); let cpu = cpu_time_ns().wrapping_sub(cpu_start); if let Err(error) = system::suppression_save() { - if error == Errno::EEXIST { - return Err(PreCaptureError::Pending); - } - let _ = system::suppression_restore(false); - return Err(PreCaptureError::Suppression(error)); + // The save left the settings as they were, whether another capture is pending or not. + return Err(if error == Errno::EEXIST { + PreCaptureError::Pending + } else { + PreCaptureError::Suppression(error) + }); } if let Err(error) = checks::record_check( runtime, @@ -1239,6 +1266,37 @@ mod tests { assert_eq!(sys::check_released(), Ok(())); } + #[test] + fn a_preparation_that_may_leave_port_permission_fails_with_the_detectors_restored() { + let revoke = Err(PortError::Revoke(Errno::ENOMEM)); + let mut restores = 0; + // A preparation that suppressed the detectors restores their settings. + let error = released_preparation(Ok(()), revoke, || restores += 1).unwrap_err(); + assert!( + matches!( + &error, + PreCaptureError::PortRelease(TimeError::PortRelease(source)) + if has_errno(source, Errno::ENOMEM) + ), + "{error:?}" + ); + assert_eq!(restores, 1); + // One that failed changed no setting. + let failed = Err(PreCaptureError::Pending); + let error = released_preparation(failed, revoke, || restores += 1).unwrap_err(); + assert!( + matches!(error, PreCaptureError::PortRelease(_)), + "{error:?}" + ); + assert_eq!(restores, 1); + // Without port permission, the preparation's result stands. + released_preparation(Ok(()), Ok(()), || restores += 1).unwrap(); + let failed = Err(PreCaptureError::Pending); + let error = released_preparation(failed, Ok(()), || restores += 1).unwrap_err(); + assert!(matches!(error, PreCaptureError::Pending), "{error:?}"); + assert_eq!(restores, 1); + } + /// Returns the outcome of a cold boot that accepted no sample. fn cold_boot() -> ColdBoot { ColdBoot { diff --git a/nvx-rs/src/guest/time/commands.rs b/nvx-rs/src/guest/time/commands.rs index 84514d914..b4817aa7f 100644 --- a/nvx-rs/src/guest/time/commands.rs +++ b/nvx-rs/src/guest/time/commands.rs @@ -150,10 +150,13 @@ fn cmd_pre_capture(abi: TimeAbi, err: &mut dyn Write) -> Result<(), CommandError } /// Reports why the preparation of a capture failed, such as `nvx-time: cannot suppress the stall -/// detectors: a capture is already pending`, and fails. A violation's event is its only report. +/// detectors: a capture is already pending`, and fails. A violation's event is its only report, +/// and `run` reports a permission that the thread may still hold once, when the command ends. fn pre_capture_failure(err: &mut dyn Write, error: PreCaptureError) -> CommandError { let reason = match error { - PreCaptureError::BootChecks(_) | PreCaptureError::CaptureChecks(_) => { + PreCaptureError::BootChecks(_) + | PreCaptureError::CaptureChecks(_) + | PreCaptureError::PortRelease(_) => { return CommandError::Operation; } PreCaptureError::Pending => "a capture is already pending", @@ -402,6 +405,11 @@ mod tests { PreCaptureError::CaptureChecks(TimeError::StateUnavailable), String::new(), ), + // `run` reports the permission once, when the command ends. + ( + PreCaptureError::PortRelease(PortError::Revoke(Errno::ENOMEM).into()), + String::new(), + ), ]; for (error, text) in cases { let mut err = Vec::new(); diff --git a/nvx-rs/src/guest/time/system.rs b/nvx-rs/src/guest/time/system.rs index 7ac502f9d..32ee1b5e6 100644 --- a/nvx-rs/src/guest/time/system.rs +++ b/nvx-rs/src/guest/time/system.rs @@ -471,30 +471,11 @@ pub(crate) fn rates_load() -> Result<(u64, u64), Errno> { } /// Saves the stall detectors' settings in the suppression record, and suppresses them for a -/// snapshot capture. A record that exists already, of a pending capture, is `EEXIST`. +/// snapshot capture; see [`Suppression::save`]. A record that exists already, of a pending +/// capture, is `EEXIST`. pub(crate) fn suppression_save() -> Result<(), Errno> { - let mut saved = Vec::new(); - for (index, (path, _)) in SUPPRESSED_DETECTORS.iter().enumerate() { - if let Err(error) = sys::check_exists(path) { - if index == 0 { - return Err(error); - } - continue; - } - let value = sys::read_number::(path)?; - saved.extend_from_slice(path.to_bytes()); - saved.extend_from_slice(format!("={value}\n").as_bytes()); - } - let record = CreatedFile::create(SUPPRESSION_PATH, true, PRIVATE_FILE_MODE)?; - record.write_all(&saved)?; - // As in the C program, a record that fails to close stays, and the suppression goes on. - let _ = record.keep(); - for (path, value) in SUPPRESSED_DETECTORS { - if sys::path_exists(path) { - sys::write_number(path, value)?; - } - } - Ok(()) + make_runtime_directories(); + SUPPRESSION.save() } /// Formats a diagnostic: `prefix`, then the description of `error`. @@ -502,36 +483,102 @@ fn describe(prefix: &[u8], error: Errno) -> Vec { [prefix, error.desc().as_bytes()].concat() } -/// Restores the stall detectors' settings from the suppression record, and removes the record. -/// A missing record succeeds when `missing_ok`. +/// Restores the stall detectors' settings from the suppression record, and removes the record; +/// see [`Suppression::restore`]. A missing record succeeds when `missing_ok`. pub(crate) fn suppression_restore(missing_ok: bool) -> Result<(), Vec> { - let _lock = lock_file(SUPPRESSION_LOCK_PATH) - .map_err(|error| describe(b"lock /run/nvx/time-suppression.lock: ", error))?; - let text = match sys::read_file(SUPPRESSION_PATH) { - Ok(text) => text, - Err(Errno::ENOENT) if missing_ok => return Ok(()), - Err(error) => return Err(describe(b"read /run/nvx/time-suppression: ", error)), - }; - for line in text - .split(|byte| *byte == b'\n') - .filter(|line| !line.is_empty()) - { - let Some(equal) = line.iter().position(|byte| *byte == b'=') else { - break; - }; - let Ok(path) = CString::new(&line[..equal]) else { - break; + make_runtime_directories(); + SUPPRESSION.restore(missing_ok) +} + +/// The suppression of the stall detectors for a capture: the detectors, the record of their +/// saved settings, which exists while a capture is pending, and the lock that serializes the +/// record's saves and restores, so that each restores only the settings that a save recorded. +struct Suppression<'a> { + /// The detectors, of which only the first must exist, and the value that suppresses each. + detectors: &'a [(&'a CStr, i64)], + /// The record of the detectors' saved settings. + record: &'a CStr, + /// The lock that serializes the saves and restores of the record. + lock: &'a CStr, +} + +/// The guest's suppression of its stall detectors. +const SUPPRESSION: Suppression<'static> = Suppression { + detectors: &SUPPRESSED_DETECTORS, + record: SUPPRESSION_PATH, + lock: SUPPRESSION_LOCK_PATH, +}; + +impl Suppression<'_> { + /// Saves the detectors' settings in the record, and suppresses the detectors, under the + /// lock, so that no restore runs between its reads and its writes. A record that exists + /// already, of a pending capture, is `EEXIST`. A save that fails before it created the + /// record changes nothing; one that fails after restores the settings that it changed, and + /// removes the record. + fn save(&self) -> Result<(), Errno> { + let _lock = sys::lock_file(self.lock, PRIVATE_FILE_MODE)?; + let mut saved = Vec::new(); + for (index, &(path, suppressed)) in self.detectors.iter().enumerate() { + if let Err(error) = sys::check_exists(path) { + if index == 0 { + return Err(error); + } + continue; + } + saved.push((path, sys::read_number::(path)?, suppressed)); + } + let mut text = Vec::new(); + for (path, value, _) in &saved { + text.extend_from_slice(path.to_bytes()); + text.extend_from_slice(format!("={value}\n").as_bytes()); + } + let record = CreatedFile::create(self.record, true, PRIVATE_FILE_MODE)?; + record.write_all(&text)?; + // As in the C program, a record that fails to close stays, and the suppression goes on. + let _ = record.keep(); + for (written, &(path, _, suppressed)) in saved.iter().enumerate() { + if let Err(error) = sys::write_number(path, suppressed) { + for &(path, value, _) in &saved[..written] { + let _ = sys::write_number(path, value); + } + let _ = sys::unlink(self.record); + return Err(error); + } + } + Ok(()) + } + + /// Restores the detectors' settings from the record, and removes it, under the lock. A + /// missing record succeeds when `missing_ok`. + fn restore(&self, missing_ok: bool) -> Result<(), Vec> { + let _lock = sys::lock_file(self.lock, PRIVATE_FILE_MODE) + .map_err(|error| describe(b"lock /run/nvx/time-suppression.lock: ", error))?; + let text = match sys::read_file(self.record) { + Ok(text) => text, + Err(Errno::ENOENT) if missing_ok => return Ok(()), + Err(error) => return Err(describe(b"read /run/nvx/time-suppression: ", error)), }; - if let Err(error) = sys::write_file(&path, &line[equal + 1..]) { - let quoted = &line[..equal.min(QUOTED_PATH_LIMIT)]; - return Err(describe( - &[b"restore ".as_slice(), quoted, b": "].concat(), - error, - )); + for line in text + .split(|byte| *byte == b'\n') + .filter(|line| !line.is_empty()) + { + let Some(equal) = line.iter().position(|byte| *byte == b'=') else { + break; + }; + let Ok(path) = CString::new(&line[..equal]) else { + break; + }; + if let Err(error) = sys::write_file(&path, &line[equal + 1..]) { + let quoted = &line[..equal.min(QUOTED_PATH_LIMIT)]; + return Err(describe( + &[b"restore ".as_slice(), quoted, b": "].concat(), + error, + )); + } } + sys::unlink(self.record) + .map_err(|error| describe(b"remove /run/nvx/time-suppression: ", error)) } - sys::unlink(SUPPRESSION_PATH) - .map_err(|error| describe(b"remove /run/nvx/time-suppression: ", error)) } /// Reads a byte buffer using one scalar four-byte port access per chunk. @@ -752,7 +799,13 @@ pub(crate) mod tests { //! A fake of the privileged operations, and checks of the runtime over it. use std::collections::VecDeque; + use std::ffi::OsStr; + use std::os::unix::ffi::{OsStrExt, OsStringExt}; + use std::path::Path; use std::sync::atomic::AtomicUsize; + use std::sync::mpsc::{self, RecvTimeoutError}; + use std::thread; + use std::time::Duration; use super::*; use crate::guest::sys::PortBackend; @@ -1116,4 +1169,174 @@ pub(crate) mod tests { assert!(path.to_bytes().starts_with(b"/run/nvx/"), "{path:?}"); } } + + /// Stall detectors in a temporary directory, as files that hold their settings, and the + /// record and the lock of their suppression beside them. + struct TestDetectors { + /// The temporary directory, which holds the files until the test ends. + _directory: tempfile::TempDir, + /// The detectors' files: RCU's stall warnings, the soft-lockup detector, and the + /// hung-task detector. + paths: [CString; 3], + /// The suppression record. + record: CString, + /// The suppression lock. + lock: CString, + } + + impl TestDetectors { + /// Returns detectors whose files hold `settings`, and are missing for `None`. + fn new(settings: [Option<&str>; 3]) -> Self { + let directory = tempfile::tempdir().unwrap(); + let path = |name: &str| { + CString::new(directory.path().join(name).into_os_string().into_vec()).unwrap() + }; + let paths = ["rcu", "soft-lockup", "hung-task"].map(&path); + for (path, setting) in paths.iter().zip(settings) { + if let Some(setting) = setting { + std::fs::write(std_path(path), format!("{setting}\n")).unwrap(); + } + } + let record = path("suppression"); + let lock = path("suppression.lock"); + Self { + _directory: directory, + paths, + record, + lock, + } + } + + /// Returns the detectors, with the value that suppresses each, as the guest's. + fn detectors(&self) -> [(&CStr, i64); 3] { + let [rcu, soft_lockup, hung_task] = &self.paths; + [ + (rcu.as_c_str(), 1), + (soft_lockup.as_c_str(), 0), + (hung_task.as_c_str(), 0), + ] + } + + /// Returns the suppression of `detectors`, with the record and the lock. + fn suppression<'a>(&'a self, detectors: &'a [(&'a CStr, i64)]) -> Suppression<'a> { + Suppression { + detectors, + record: &self.record, + lock: &self.lock, + } + } + + /// Returns the first line of each detector's file, or `None` for one that can't be read. + fn settings(&self) -> [Option; 3] { + self.paths.each_ref().map(|path| { + let text = sys::read_file(path).ok()?; + let text = String::from_utf8(text).unwrap(); + Some(text.lines().next().unwrap_or_default().to_owned()) + }) + } + } + + /// Returns `path` as a standard path. + fn std_path(path: &CStr) -> &Path { + Path::new(OsStr::from_bytes(path.to_bytes())) + } + + /// Returns `settings` as the detectors' settings that [`TestDetectors::settings`] returns. + fn settings(settings: [Option<&str>; 3]) -> [Option; 3] { + settings.map(|setting| setting.map(str::to_owned)) + } + + #[test] + fn a_suppression_saves_the_settings_and_suppresses_the_detectors_until_its_restore() { + let test = TestDetectors::new([Some("0"), Some("60"), Some("120")]); + let detectors = test.detectors(); + let suppression = test.suppression(&detectors); + suppression.save().unwrap(); + let suppressed = settings([Some("1"), Some("0"), Some("0")]); + assert_eq!(test.settings(), suppressed); + let [rcu, soft_lockup, hung_task] = + test.paths.each_ref().map(|path| path.to_str().unwrap()); + let record = format!("{rcu}=0\n{soft_lockup}=60\n{hung_task}=120\n"); + assert_eq!(sys::read_file(&test.record).unwrap(), record.as_bytes()); + + // Another save finds the pending capture's record, and changes nothing. + assert_eq!(suppression.save(), Err(Errno::EEXIST)); + assert_eq!(test.settings(), suppressed); + assert_eq!(sys::read_file(&test.record).unwrap(), record.as_bytes()); + + suppression.restore(false).unwrap(); + assert_eq!( + test.settings(), + settings([Some("0"), Some("60"), Some("120")]) + ); + assert_eq!(sys::check_exists(&test.record), Err(Errno::ENOENT)); + // Without a record, only a restore that allows a missing one succeeds. + assert_eq!( + suppression.restore(false).unwrap_err(), + format!("read /run/nvx/time-suppression: {}", Errno::ENOENT.desc()).into_bytes() + ); + suppression.restore(true).unwrap(); + } + + #[test] + fn a_save_that_fails_before_its_record_changes_nothing() { + // The first detector, which must exist, is missing. + let test = TestDetectors::new([None, Some("60"), Some("120")]); + let detectors = test.detectors(); + let suppression = test.suppression(&detectors); + let unchanged = settings([None, Some("60"), Some("120")]); + assert_eq!(suppression.save(), Err(Errno::ENOENT)); + assert_eq!(sys::check_exists(&test.record), Err(Errno::ENOENT)); + assert_eq!(test.settings(), unchanged); + + // A detector that can't be read, while another capture is pending, leaves that capture's + // record and the settings as they were. + std::fs::create_dir(std_path(&test.paths[0])).unwrap(); + let pending = b"/proc/sys/kernel/hung_task_timeout_secs=120\n"; + std::fs::write(std_path(&test.record), pending).unwrap(); + assert!(suppression.save().is_err()); + assert_eq!(sys::read_file(&test.record).unwrap(), pending); + assert_eq!(test.settings(), unchanged); + } + + #[test] + fn a_save_that_fails_to_suppress_a_detector_restores_the_others_and_removes_its_record() { + let test = TestDetectors::new([Some("0"), Some("60"), None]); + // The hung-task detector reads as a number, but nobody may write it, root included. + std::os::unix::fs::symlink("/proc/sys/kernel/ngroups_max", std_path(&test.paths[2])) + .unwrap(); + let ngroups_max = test.settings()[2].clone(); + assert!(ngroups_max.is_some()); + let detectors = test.detectors(); + assert!(test.suppression(&detectors).save().is_err()); + let [rcu, soft_lockup, hung_task] = test.settings(); + assert_eq!(rcu.as_deref(), Some("0")); + assert_eq!(soft_lockup.as_deref(), Some("60")); + assert_eq!(hung_task, ngroups_max); + assert_eq!(sys::check_exists(&test.record), Err(Errno::ENOENT)); + } + + #[test] + fn a_save_waits_for_the_suppression_lock() { + let test = TestDetectors::new([Some("0"), Some("60"), Some("120")]); + let detectors = test.detectors(); + let suppression = test.suppression(&detectors); + let lock = sys::lock_file(&test.lock, PRIVATE_FILE_MODE).unwrap(); + let (sender, receiver) = mpsc::channel(); + thread::scope(|scope| { + scope.spawn(|| sender.send(suppression.save()).unwrap()); + // While a restore holds the lock, the save reads and changes nothing. + assert_eq!( + receiver.recv_timeout(Duration::from_millis(200)), + Err(RecvTimeoutError::Timeout) + ); + assert_eq!( + test.settings(), + settings([Some("0"), Some("60"), Some("120")]) + ); + drop(lock); + assert_eq!(receiver.recv().unwrap(), Ok(())); + }); + assert_eq!(test.settings(), settings([Some("1"), Some("0"), Some("0")])); + } } From 28137dc46455e72db0c9db272cc1734ce6e4fa6c Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 11:34:58 -0700 Subject: [PATCH 3/7] guest: keep the suppression record when a failed save cannot undo itself A save of the stall detectors' settings that failed to suppress one, then to undo an earlier suppression, removed its record anyway: the detector stayed suppressed, and its original setting was lost, so a later capture could record the suppressed value as the original one. The save now keeps its record unless it undid every change, and reports which of the two happened. A preparation that kept the record, or that suppressed the detectors before the thread failed to release the time ABI's ports, now tells its caller that it holds their suppression, which the caller releases as after a preparation that succeeded: nvx-time pre-capture restores the settings before it fails, as it did before, and the capture barrier restores them, or fails with a Release error. The tests inject the failing writes instead of relying on a read-only sysctl. Part of #840 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da708b6-0bda-4804-b01b-936cdc5f054a --- nvx-rs/src/guest/clone.rs | 44 ++++++---- nvx-rs/src/guest/clone/tests.rs | 75 +++++++++++++--- nvx-rs/src/guest/time/api.rs | 139 ++++++++++++++++++++---------- nvx-rs/src/guest/time/commands.rs | 26 ++++-- nvx-rs/src/guest/time/system.rs | 125 +++++++++++++++++++++------ 5 files changed, 298 insertions(+), 111 deletions(-) diff --git a/nvx-rs/src/guest/clone.rs b/nvx-rs/src/guest/clone.rs index c85b93fa8..1387b74f0 100644 --- a/nvx-rs/src/guest/clone.rs +++ b/nvx-rs/src/guest/clone.rs @@ -194,10 +194,11 @@ impl CapturePreparation { /// The wait for the boot checks lasts 30 seconds at most. A capture check that fails reports /// a violation of the time ABI, which stops the VM in enforcing mode; in report-only mode, /// the preparation goes on, as `nvx-time pre-capture` does. A preparation that fails opens - /// the fence again, and leaves the detectors' settings as they were. One after which the - /// calling thread may still hold some of the permission for the time ABI's ports fails with - /// [`TimeError::PortRelease`], after which the thread must start neither a thread nor a - /// process, which would inherit it. + /// the fence again, and leaves the detectors' settings as they were: when it suppressed some + /// already, it restores their settings, and a failure to restore them is a + /// [`CaptureStage::Release`] error. One after which the calling thread may still hold some of + /// the permission for the time ABI's ports fails with [`TimeError::PortRelease`], after which + /// the thread must start neither a thread nor a process, which would inherit it. pub fn prepare(self) -> Result, CaptureError> { let system = LiveSystem { paths: Paths::default(), @@ -220,20 +221,29 @@ impl CapturePreparation { }; check_mounts(barrier.system.as_mut(), &devices) .map_err(|cause| CaptureError::new(CaptureStage::Mounts, cause))?; - barrier.system.pre_capture().map_err(|error| match error { - PreCaptureError::BootChecks(error) => { - CaptureError::new(CaptureStage::BootChecks, CaptureCause::Time(error)) + if let Err(error) = barrier.system.pre_capture() { + // The barrier restores a suppression that the time ABI holds after a failure, as + // after a preparation that succeeded, and reports a failure to restore it. + if error.holds_suppression() { + barrier.suppressed = true; + barrier.release()?; } - PreCaptureError::CaptureChecks(error) | PreCaptureError::PortRelease(error) => { - CaptureError::new(CaptureStage::CaptureChecks, CaptureCause::Time(error)) - } - PreCaptureError::Pending => { - CaptureError::new(CaptureStage::Suppression, CaptureCause::Pending) - } - PreCaptureError::Suppression(error) => { - CaptureError::new(CaptureStage::Suppression, CaptureCause::Io(error.into())) - } - })?; + return Err(match error { + PreCaptureError::BootChecks(error) => { + CaptureError::new(CaptureStage::BootChecks, CaptureCause::Time(error)) + } + PreCaptureError::CaptureChecks(error) + | PreCaptureError::PortRelease { error, .. } => { + CaptureError::new(CaptureStage::CaptureChecks, CaptureCause::Time(error)) + } + PreCaptureError::Pending => { + CaptureError::new(CaptureStage::Suppression, CaptureCause::Pending) + } + PreCaptureError::Suppression(error) | PreCaptureError::Unrestored(error) => { + CaptureError::new(CaptureStage::Suppression, CaptureCause::Io(error.into())) + } + }); + } barrier.suppressed = true; Ok(barrier) } diff --git a/nvx-rs/src/guest/clone/tests.rs b/nvx-rs/src/guest/clone/tests.rs index f16c302a6..d504fe7bc 100644 --- a/nvx-rs/src/guest/clone/tests.rs +++ b/nvx-rs/src/guest/clone/tests.rs @@ -117,8 +117,12 @@ enum Preparation { Pending, /// The detectors' settings cannot be saved, with this error number. Suppression(Errno), - /// The thread may still hold some of the permission for the ports, with this error number. - PortRelease(Errno), + /// A detector cannot be suppressed, nor the others restored, with this error number, so + /// the time ABI holds their suppression. + Unrestored(Errno), + /// The thread may still hold some of the permission for the ports, with this error number, + /// after a preparation that held the suppression or not. + PortRelease(Errno, bool), } /// How the fake capture ends. @@ -237,9 +241,11 @@ impl System for FakeSystem { } Preparation::Pending => Err(PreCaptureError::Pending), Preparation::Suppression(errno) => Err(PreCaptureError::Suppression(errno)), - Preparation::PortRelease(errno) => Err(PreCaptureError::PortRelease( - PortError::Revoke(errno).into(), - )), + Preparation::Unrestored(errno) => Err(PreCaptureError::Unrestored(errno)), + Preparation::PortRelease(errno, suppressed) => Err(PreCaptureError::PortRelease { + error: PortError::Revoke(errno).into(), + suppressed, + }), } } @@ -573,20 +579,37 @@ fn a_fence_that_fails_to_close_stops_the_preparation() { #[test] fn a_failed_time_preparation_names_its_stage_and_opens_the_fence() { + // Each failure, its stage, and whether the time ABI holds the detectors' suppression. let cases = [ - (Preparation::BootChecks, CaptureStage::BootChecks), - (Preparation::CaptureChecks, CaptureStage::CaptureChecks), - (Preparation::Pending, CaptureStage::Suppression), + (Preparation::BootChecks, CaptureStage::BootChecks, false), + ( + Preparation::CaptureChecks, + CaptureStage::CaptureChecks, + false, + ), + (Preparation::Pending, CaptureStage::Suppression, false), ( Preparation::Suppression(Errno::EROFS), CaptureStage::Suppression, + false, + ), + ( + Preparation::Unrestored(Errno::EIO), + CaptureStage::Suppression, + true, ), ( - Preparation::PortRelease(Errno::ENOMEM), + Preparation::PortRelease(Errno::ENOMEM, false), CaptureStage::CaptureChecks, + false, + ), + ( + Preparation::PortRelease(Errno::ENOMEM, true), + CaptureStage::CaptureChecks, + true, ), ]; - for (failure, stage) in cases { + for (failure, stage, held) in cases { let calls = Calls::default(); let mut system = FakeSystem::new(&calls); system.preparation = failure; @@ -595,9 +618,11 @@ fn a_failed_time_preparation_names_its_stage_and_opens_the_fence() { match (failure, error.cause()) { (Preparation::BootChecks | Preparation::CaptureChecks, CaptureCause::Time(_)) | (Preparation::Pending, CaptureCause::Pending) => {} - (Preparation::Suppression(errno), CaptureCause::Io(cause)) - if cause.raw_os_error() == Some(errno as i32) => {} - (Preparation::PortRelease(_), CaptureCause::Time(TimeError::PortRelease(_))) => { + ( + Preparation::Suppression(errno) | Preparation::Unrestored(errno), + CaptureCause::Io(cause), + ) if cause.raw_os_error() == Some(errno as i32) => {} + (Preparation::PortRelease(..), CaptureCause::Time(TimeError::PortRelease(_))) => { assert_eq!( error.to_string(), format!( @@ -608,13 +633,35 @@ fn a_failed_time_preparation_names_its_stage_and_opens_the_fence() { } _ => panic!("{failure:?}: {error:?}"), } - // The time ABI left the detectors as they were, so the barrier restores nothing. + // The fence opens, and the barrier restores the detectors' settings that the time ABI + // holds, or nothing. let mut expected = preparation_calls(); expected.push(Call::Open); + if held { + expected.push(Call::CancelCapture); + } assert_eq!(calls.take(), expected, "{failure:?}"); } } +#[test] +fn a_held_suppression_that_a_failed_preparation_cannot_restore_is_a_release_error() { + let calls = Calls::default(); + let mut system = FakeSystem::new(&calls); + system.preparation = Preparation::Unrestored(Errno::EIO); + system.restoration_fails = true; + let error = prepare(fence(&calls), system).unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Release); + assert_eq!( + error.to_string(), + "failed to restore the stall detectors: read /run/nvx/time-suppression: No such file or \ + directory" + ); + let mut expected = preparation_calls(); + expected.extend([Call::Open, Call::CancelCapture]); + assert_eq!(calls.take(), expected); +} + #[test] fn cancel_and_drop_open_the_fence_then_restore_the_detectors() { let calls = Calls::default(); diff --git a/nvx-rs/src/guest/time/api.rs b/nvx-rs/src/guest/time/api.rs index 52b7aeece..c621ac763 100644 --- a/nvx-rs/src/guest/time/api.rs +++ b/nvx-rs/src/guest/time/api.rs @@ -19,8 +19,8 @@ use super::state::{ use super::system::{ self, PORTB_GENERATION_ID_AVAILABLE, PORTB_PACKET_AVAILABLE, PRIVATE_FILE_MODE, RESTORE_PATH, RESTORE_TEMP_PATH, RealOps, Runtime, SELECT_GENERATION_ID, SELECT_PACKET, SampleError, - SpawnError, clock_ns, cpu_time_ns, current_frequency, fork_process, initial_sample, portb_read, - set_frequency, state_apply, state_load, state_store, step_clock, + SaveError, SpawnError, clock_ns, cpu_time_ns, current_frequency, fork_process, initial_sample, + portb_read, set_frequency, state_apply, state_load, state_store, step_clock, }; use super::{NSEC_PER_USEC, Phase}; use crate::guest::GenerationId; @@ -168,15 +168,15 @@ impl TimeAbi { /// /// A capture check that fails reports a violation, which stops the VM in enforcing mode; a /// report-only call goes on, as `nvx-time` does. A call that fails leaves the detectors as - /// they were: it restores the settings that it changed, and never touches those of a capture - /// that is pending already, [`PreCaptureError::Pending`]. Like the other calls, it fails with - /// [`TimeError::PortRelease`], as [`PreCaptureError::PortRelease`], if the thread may still - /// hold some of the permission for the ports, once it restored the detectors' settings. - /// [`cancel_capture`](Self::cancel_capture) restores the settings that it saved. + /// they were, and never touches those of a capture that is pending already, + /// [`PreCaptureError::Pending`], unless it + /// [holds their suppression](PreCaptureError::holds_suppression), which the caller then + /// releases with [`cancel_capture`](Self::cancel_capture), as after a call that succeeded. + /// Like the other calls, it fails with [`TimeError::PortRelease`], as + /// [`PreCaptureError::PortRelease`], if the thread may still hold some of the permission for + /// the ports. pub(crate) fn pre_capture(&self) -> Result<(), PreCaptureError> { - released_preparation(pre_capture(&self.runtime()), sys::check_released(), || { - let _ = system::suppression_restore(false); - }) + released_preparation(pre_capture(&self.runtime()), sys::check_released()) } /// Restores the stall detectors' settings that [`pre_capture`](Self::pre_capture) saved, and @@ -212,20 +212,22 @@ fn released(result: Result) -> Result { } /// Ends a preparation of a capture with `result`, unless the calling thread may still hold port -/// permission, as `released` tells, which the preparation reports instead, once `restore` -/// restored the stall detectors' settings that a preparation that succeeded saved. +/// permission, as `released` tells, which the preparation reports instead, with whether it holds +/// the stall detectors' suppression. fn released_preparation( result: Result<(), PreCaptureError>, released: Result<(), PortError>, - restore: impl FnOnce(), ) -> Result<(), PreCaptureError> { let Err(error) = released else { return result; }; - if result.is_ok() { - restore(); - } - Err(PreCaptureError::PortRelease(error.into())) + let suppressed = result + .as_ref() + .map_or_else(PreCaptureError::holds_suppression, |()| true); + Err(PreCaptureError::PortRelease { + error: error.into(), + suppressed, + }) } /// The outcome of the cold-boot time step, from which [`TimeAbi::start_daemon`] starts the time @@ -314,12 +316,37 @@ pub(crate) enum PreCaptureError { /// The stall detectors' settings are saved already, by a capture that is still pending. Pending, /// The stall detectors' settings could not be saved, or the detectors not suppressed, with - /// this error. The call restored the settings that it had changed. + /// this error. The call left the settings as they were. Suppression(Errno), + /// The stall detectors could not be suppressed, with this error, nor the ones that the call + /// suppressed restored, so it kept the record of their settings: the call + /// [holds their suppression](PreCaptureError::holds_suppression). + Unrestored(Errno), /// The thread may still hold some of the permission for the ports, a - /// [`TimeError::PortRelease`], after which it must start neither a thread nor a process. The - /// call restored the stall detectors' settings. - PortRelease(TimeError), + /// [`TimeError::PortRelease`], after which it must start neither a thread nor a process. + PortRelease { + /// The error. + error: TimeError, + /// Whether the call [holds the suppression](PreCaptureError::holds_suppression) of the + /// stall detectors, as one that succeeded does. + suppressed: bool, + }, +} + +impl PreCaptureError { + /// Returns whether the call that failed holds the suppression of the stall detectors, as a + /// call that succeeded does: their record stays, and the caller restores their settings with + /// [`TimeAbi::cancel_capture`]. + pub(crate) fn holds_suppression(&self) -> bool { + matches!( + self, + Self::Unrestored(_) + | Self::PortRelease { + suppressed: true, + .. + } + ) + } } /// The preparation of [`TimeAbi::pre_capture`]. @@ -347,13 +374,11 @@ fn pre_capture(runtime: &Runtime<'_>) -> Result<(), PreCaptureError> { checks::run_capture_checks(runtime, &mut checks); let elapsed = clock_ns(sys::Clock::Monotonic).wrapping_sub(start); let cpu = cpu_time_ns().wrapping_sub(cpu_start); - if let Err(error) = system::suppression_save() { - // The save left the settings as they were, whether another capture is pending or not. - return Err(if error == Errno::EEXIST { - PreCaptureError::Pending - } else { - PreCaptureError::Suppression(error) - }); + match system::suppression_save() { + Ok(()) => {} + Err(SaveError::Undone(Errno::EEXIST)) => return Err(PreCaptureError::Pending), + Err(SaveError::Undone(error)) => return Err(PreCaptureError::Suppression(error)), + Err(SaveError::Kept(error)) => return Err(PreCaptureError::Unrestored(error)), } if let Err(error) = checks::record_check( runtime, @@ -1267,34 +1292,54 @@ mod tests { } #[test] - fn a_preparation_that_may_leave_port_permission_fails_with_the_detectors_restored() { + fn a_preparation_that_may_leave_port_permission_fails_with_its_suppression() { let revoke = Err(PortError::Revoke(Errno::ENOMEM)); - let mut restores = 0; - // A preparation that suppressed the detectors restores their settings. - let error = released_preparation(Ok(()), revoke, || restores += 1).unwrap_err(); + // A preparation that suppressed the detectors holds their suppression. + let error = released_preparation(Ok(()), revoke).unwrap_err(); assert!( matches!( &error, - PreCaptureError::PortRelease(TimeError::PortRelease(source)) - if has_errno(source, Errno::ENOMEM) + PreCaptureError::PortRelease { + error: TimeError::PortRelease(source), + suppressed: true, + } if has_errno(source, Errno::ENOMEM) ), "{error:?}" ); - assert_eq!(restores, 1); - // One that failed changed no setting. - let failed = Err(PreCaptureError::Pending); - let error = released_preparation(failed, revoke, || restores += 1).unwrap_err(); - assert!( - matches!(error, PreCaptureError::PortRelease(_)), - "{error:?}" - ); - assert_eq!(restores, 1); + assert!(error.holds_suppression()); + // One that kept the record of the settings that it could not restore holds it too. + let kept = Err(PreCaptureError::Unrestored(Errno::EIO)); + let error = released_preparation(kept, revoke).unwrap_err(); + assert!(error.holds_suppression(), "{error:?}"); + // One that left the settings as they were holds nothing. + for failed in [ + PreCaptureError::Pending, + PreCaptureError::Suppression(Errno::EIO), + ] { + let error = released_preparation(Err(failed), revoke).unwrap_err(); + assert!( + matches!( + error, + PreCaptureError::PortRelease { + suppressed: false, + .. + } + ), + "{error:?}" + ); + assert!(!error.holds_suppression()); + } // Without port permission, the preparation's result stands. - released_preparation(Ok(()), Ok(()), || restores += 1).unwrap(); - let failed = Err(PreCaptureError::Pending); - let error = released_preparation(failed, Ok(()), || restores += 1).unwrap_err(); + released_preparation(Ok(()), Ok(())).unwrap(); + let error = released_preparation(Err(PreCaptureError::Pending), Ok(())).unwrap_err(); assert!(matches!(error, PreCaptureError::Pending), "{error:?}"); - assert_eq!(restores, 1); + assert!(!error.holds_suppression()); + let kept = Err(PreCaptureError::Unrestored(Errno::EIO)); + assert!( + released_preparation(kept, Ok(())) + .unwrap_err() + .holds_suppression() + ); } /// Returns the outcome of a cold boot that accepted no sample. diff --git a/nvx-rs/src/guest/time/commands.rs b/nvx-rs/src/guest/time/commands.rs index b4817aa7f..b7fba92de 100644 --- a/nvx-rs/src/guest/time/commands.rs +++ b/nvx-rs/src/guest/time/commands.rs @@ -144,9 +144,15 @@ fn cmd_check( /// Preparation steps 1 to 4: waits for the boot checks, which start the daemon that C10 /// requires, runs the capture checks, and suppresses the stall detectors before the snapshot. +/// A preparation that fails leaves the detectors as they were, as far as it can: the snapshot +/// script cancels only a capture whose preparation succeeded. fn cmd_pre_capture(abi: TimeAbi, err: &mut dyn Write) -> Result<(), CommandError> { - abi.pre_capture() - .map_err(|error| pre_capture_failure(err, error)) + abi.pre_capture().map_err(|error| { + if error.holds_suppression() { + let _ = abi.cancel_capture(); + } + pre_capture_failure(err, error) + }) } /// Reports why the preparation of a capture failed, such as `nvx-time: cannot suppress the stall @@ -156,11 +162,11 @@ fn pre_capture_failure(err: &mut dyn Write, error: PreCaptureError) -> CommandEr let reason = match error { PreCaptureError::BootChecks(_) | PreCaptureError::CaptureChecks(_) - | PreCaptureError::PortRelease(_) => { + | PreCaptureError::PortRelease { .. } => { return CommandError::Operation; } PreCaptureError::Pending => "a capture is already pending", - PreCaptureError::Suppression(error) => error.desc(), + PreCaptureError::Suppression(error) | PreCaptureError::Unrestored(error) => error.desc(), }; report( err, @@ -396,6 +402,13 @@ mod tests { Errno::EROFS.desc() ), ), + ( + PreCaptureError::Unrestored(Errno::EIO), + format!( + "nvx-time: cannot suppress the stall detectors: {}\n", + Errno::EIO.desc() + ), + ), // A violation's event is its only report. ( PreCaptureError::BootChecks(TimeError::StateUnavailable), @@ -407,7 +420,10 @@ mod tests { ), // `run` reports the permission once, when the command ends. ( - PreCaptureError::PortRelease(PortError::Revoke(Errno::ENOMEM).into()), + PreCaptureError::PortRelease { + error: PortError::Revoke(Errno::ENOMEM).into(), + suppressed: true, + }, String::new(), ), ]; diff --git a/nvx-rs/src/guest/time/system.rs b/nvx-rs/src/guest/time/system.rs index 32ee1b5e6..ba1a6994d 100644 --- a/nvx-rs/src/guest/time/system.rs +++ b/nvx-rs/src/guest/time/system.rs @@ -473,11 +473,23 @@ pub(crate) fn rates_load() -> Result<(u64, u64), Errno> { /// Saves the stall detectors' settings in the suppression record, and suppresses them for a /// snapshot capture; see [`Suppression::save`]. A record that exists already, of a pending /// capture, is `EEXIST`. -pub(crate) fn suppression_save() -> Result<(), Errno> { +pub(crate) fn suppression_save() -> Result<(), SaveError> { make_runtime_directories(); SUPPRESSION.save() } +/// Why a save of the stall detectors' settings failed. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum SaveError { + /// The save failed with this error, and left the settings as they were: it changed none, or + /// undid every change and removed its record. A record that exists already, of a pending + /// capture, is `EEXIST`, and stays that capture's. + Undone(Errno), + /// The save failed to suppress a detector with this error, then to undo every change, so it + /// kept its record, from which [`suppression_restore`] restores the settings. + Kept(Errno), +} + /// Formats a diagnostic: `prefix`, then the description of `error`. fn describe(prefix: &[u8], error: Errno) -> Vec { [prefix, error.desc().as_bytes()].concat() @@ -513,36 +525,51 @@ impl Suppression<'_> { /// Saves the detectors' settings in the record, and suppresses the detectors, under the /// lock, so that no restore runs between its reads and its writes. A record that exists /// already, of a pending capture, is `EEXIST`. A save that fails before it created the - /// record changes nothing; one that fails after restores the settings that it changed, and - /// removes the record. - fn save(&self) -> Result<(), Errno> { - let _lock = sys::lock_file(self.lock, PRIVATE_FILE_MODE)?; + /// record changes nothing; one that fails after undoes its changes and removes the record, + /// or keeps the record if it can't. + fn save(&self) -> Result<(), SaveError> { + self.save_with(sys::write_number) + } + + /// Implements [`save`](Self::save), with `write` writing a detector's setting. + fn save_with( + &self, + mut write: impl FnMut(&CStr, i64) -> Result<(), Errno>, + ) -> Result<(), SaveError> { + let _lock = sys::lock_file(self.lock, PRIVATE_FILE_MODE).map_err(SaveError::Undone)?; let mut saved = Vec::new(); for (index, &(path, suppressed)) in self.detectors.iter().enumerate() { if let Err(error) = sys::check_exists(path) { if index == 0 { - return Err(error); + return Err(SaveError::Undone(error)); } continue; } - saved.push((path, sys::read_number::(path)?, suppressed)); + let value = sys::read_number::(path).map_err(SaveError::Undone)?; + saved.push((path, value, suppressed)); } let mut text = Vec::new(); for (path, value, _) in &saved { text.extend_from_slice(path.to_bytes()); text.extend_from_slice(format!("={value}\n").as_bytes()); } - let record = CreatedFile::create(self.record, true, PRIVATE_FILE_MODE)?; - record.write_all(&text)?; + let record = + CreatedFile::create(self.record, true, PRIVATE_FILE_MODE).map_err(SaveError::Undone)?; + record.write_all(&text).map_err(SaveError::Undone)?; // As in the C program, a record that fails to close stays, and the suppression goes on. let _ = record.keep(); for (written, &(path, _, suppressed)) in saved.iter().enumerate() { - if let Err(error) = sys::write_number(path, suppressed) { + if let Err(error) = write(path, suppressed) { + // The record stays unless every earlier write is undone, so that a restore can + // still restore the settings. + let mut undone = true; for &(path, value, _) in &saved[..written] { - let _ = sys::write_number(path, value); + undone &= write(path, value).is_ok(); + } + if undone && sys::unlink(self.record).is_ok() { + return Err(SaveError::Undone(error)); } - let _ = sys::unlink(self.record); - return Err(error); + return Err(SaveError::Kept(error)); } } Ok(()) @@ -1260,7 +1287,7 @@ pub(crate) mod tests { assert_eq!(sys::read_file(&test.record).unwrap(), record.as_bytes()); // Another save finds the pending capture's record, and changes nothing. - assert_eq!(suppression.save(), Err(Errno::EEXIST)); + assert_eq!(suppression.save(), Err(SaveError::Undone(Errno::EEXIST))); assert_eq!(test.settings(), suppressed); assert_eq!(sys::read_file(&test.record).unwrap(), record.as_bytes()); @@ -1285,7 +1312,7 @@ pub(crate) mod tests { let detectors = test.detectors(); let suppression = test.suppression(&detectors); let unchanged = settings([None, Some("60"), Some("120")]); - assert_eq!(suppression.save(), Err(Errno::ENOENT)); + assert_eq!(suppression.save(), Err(SaveError::Undone(Errno::ENOENT))); assert_eq!(sys::check_exists(&test.record), Err(Errno::ENOENT)); assert_eq!(test.settings(), unchanged); @@ -1294,25 +1321,67 @@ pub(crate) mod tests { std::fs::create_dir(std_path(&test.paths[0])).unwrap(); let pending = b"/proc/sys/kernel/hung_task_timeout_secs=120\n"; std::fs::write(std_path(&test.record), pending).unwrap(); - assert!(suppression.save().is_err()); + let error = suppression.save().unwrap_err(); + assert!(matches!(error, SaveError::Undone(_)), "{error:?}"); assert_eq!(sys::read_file(&test.record).unwrap(), pending); assert_eq!(test.settings(), unchanged); } + /// Returns a save's write of a detector's setting, which fails with `EIO` for each detector + /// and value of `failing`, and writes the others. + fn failing_writes<'a>( + failing: &'a [(&'a CStr, i64)], + ) -> impl FnMut(&CStr, i64) -> Result<(), Errno> + 'a { + move |path, value| { + let fails = failing + .iter() + .any(|&(failing, failing_value)| failing == path && failing_value == value); + if fails { + Err(Errno::EIO) + } else { + sys::write_number(path, value) + } + } + } + #[test] - fn a_save_that_fails_to_suppress_a_detector_restores_the_others_and_removes_its_record() { - let test = TestDetectors::new([Some("0"), Some("60"), None]); - // The hung-task detector reads as a number, but nobody may write it, root included. - std::os::unix::fs::symlink("/proc/sys/kernel/ngroups_max", std_path(&test.paths[2])) - .unwrap(); - let ngroups_max = test.settings()[2].clone(); - assert!(ngroups_max.is_some()); + fn a_save_that_fails_to_suppress_a_detector_undoes_its_writes_and_removes_its_record() { + let test = TestDetectors::new([Some("0"), Some("60"), Some("120")]); let detectors = test.detectors(); - assert!(test.suppression(&detectors).save().is_err()); - let [rcu, soft_lockup, hung_task] = test.settings(); - assert_eq!(rcu.as_deref(), Some("0")); - assert_eq!(soft_lockup.as_deref(), Some("60")); - assert_eq!(hung_task, ngroups_max); + let suppression = test.suppression(&detectors); + let failing = [(test.paths[1].as_c_str(), 0)]; + assert_eq!( + suppression.save_with(failing_writes(&failing)), + Err(SaveError::Undone(Errno::EIO)) + ); + assert_eq!( + test.settings(), + settings([Some("0"), Some("60"), Some("120")]) + ); + assert_eq!(sys::check_exists(&test.record), Err(Errno::ENOENT)); + } + + #[test] + fn a_save_that_fails_to_undo_a_write_keeps_its_record_for_a_restore() { + let test = TestDetectors::new([Some("0"), Some("60"), Some("120")]); + let detectors = test.detectors(); + let suppression = test.suppression(&detectors); + // The soft-lockup detector can't be suppressed, nor RCU's stall warnings restored. + let failing = [(test.paths[1].as_c_str(), 0), (test.paths[0].as_c_str(), 0)]; + assert_eq!( + suppression.save_with(failing_writes(&failing)), + Err(SaveError::Kept(Errno::EIO)) + ); + assert_eq!( + test.settings(), + settings([Some("1"), Some("60"), Some("120")]) + ); + // The record keeps the settings, which a restore restores. + suppression.restore(false).unwrap(); + assert_eq!( + test.settings(), + settings([Some("0"), Some("60"), Some("120")]) + ); assert_eq!(sys::check_exists(&test.record), Err(Errno::ENOENT)); } From 40308dbe4f7574e5a289c5c4dc3d8343c44b9cb3 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 11:41:04 -0700 Subject: [PATCH 4/7] guest: let a retained port permission outrank a failed restore A preparation that failed because the thread may still hold some of the permission for the time ABI's ports, after it suppressed the stall detectors, reported a failure of the barrier to restore their settings instead, which hid that the thread must start neither a thread nor a process. The port release's failure now takes precedence, and the barrier's restore stays best effort. Part of #840 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da708b6-0bda-4804-b01b-936cdc5f054a --- nvx-rs/src/guest/clone.rs | 12 ++++++++---- nvx-rs/src/guest/clone/tests.rs | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/nvx-rs/src/guest/clone.rs b/nvx-rs/src/guest/clone.rs index 1387b74f0..6d297bbc4 100644 --- a/nvx-rs/src/guest/clone.rs +++ b/nvx-rs/src/guest/clone.rs @@ -197,8 +197,8 @@ impl CapturePreparation { /// the fence again, and leaves the detectors' settings as they were: when it suppressed some /// already, it restores their settings, and a failure to restore them is a /// [`CaptureStage::Release`] error. One after which the calling thread may still hold some of - /// the permission for the time ABI's ports fails with [`TimeError::PortRelease`], after which - /// the thread must start neither a thread nor a process, which would inherit it. + /// the permission for the time ABI's ports fails with [`TimeError::PortRelease`] instead, + /// after which the thread must start neither a thread nor a process, which would inherit it. pub fn prepare(self) -> Result, CaptureError> { let system = LiveSystem { paths: Paths::default(), @@ -223,10 +223,14 @@ impl CapturePreparation { .map_err(|cause| CaptureError::new(CaptureStage::Mounts, cause))?; if let Err(error) = barrier.system.pre_capture() { // The barrier restores a suppression that the time ABI holds after a failure, as - // after a preparation that succeeded, and reports a failure to restore it. + // after a preparation that succeeded, and reports a failure to restore it, unless the + // thread may still hold some of the permission for the ports, which takes precedence. if error.holds_suppression() { barrier.suppressed = true; - barrier.release()?; + let released = barrier.release(); + if !matches!(error, PreCaptureError::PortRelease { .. }) { + released?; + } } return Err(match error { PreCaptureError::BootChecks(error) => { diff --git a/nvx-rs/src/guest/clone/tests.rs b/nvx-rs/src/guest/clone/tests.rs index d504fe7bc..e14732dd9 100644 --- a/nvx-rs/src/guest/clone/tests.rs +++ b/nvx-rs/src/guest/clone/tests.rs @@ -662,6 +662,24 @@ fn a_held_suppression_that_a_failed_preparation_cannot_restore_is_a_release_erro assert_eq!(calls.take(), expected); } +#[test] +fn a_port_permission_that_the_thread_may_hold_outranks_a_failed_restore() { + let calls = Calls::default(); + let mut system = FakeSystem::new(&calls); + system.preparation = Preparation::PortRelease(Errno::ENOMEM, true); + system.restoration_fails = true; + let error = prepare(fence(&calls), system).unwrap_err(); + assert_eq!(error.stage(), CaptureStage::CaptureChecks); + assert!( + matches!(error.cause(), CaptureCause::Time(TimeError::PortRelease(_))), + "{error:?}" + ); + // The barrier still tried to restore the detectors' settings. + let mut expected = preparation_calls(); + expected.extend([Call::Open, Call::CancelCapture]); + assert_eq!(calls.take(), expected); +} + #[test] fn cancel_and_drop_open_the_fence_then_restore_the_detectors() { let calls = Calls::default(); From 25f55051dee8cada83e2e238ad1ca99e93e87585 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 11:53:36 -0700 Subject: [PATCH 5/7] guest: report a capture's failure to restore the detectors A failure to read the VM generation ID, or a capture request that failed, left the barrier's release to its drop, which discarded a failure to restore the stall detectors' settings. Both paths now release explicitly, as a failed preparation does, and report that failure as a Release error, unless the thread may still hold some of the permission for the time ABI's ports, which takes precedence. A failed request still leaves the fence closed. Part of #840 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da708b6-0bda-4804-b01b-936cdc5f054a --- nvx-rs/src/guest/clone.rs | 52 +++++++++++++++++++++------------ nvx-rs/src/guest/clone/tests.rs | 47 +++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 19 deletions(-) diff --git a/nvx-rs/src/guest/clone.rs b/nvx-rs/src/guest/clone.rs index 6d297bbc4..68eada608 100644 --- a/nvx-rs/src/guest/clone.rs +++ b/nvx-rs/src/guest/clone.rs @@ -223,16 +223,9 @@ impl CapturePreparation { .map_err(|cause| CaptureError::new(CaptureStage::Mounts, cause))?; if let Err(error) = barrier.system.pre_capture() { // The barrier restores a suppression that the time ABI holds after a failure, as - // after a preparation that succeeded, and reports a failure to restore it, unless the - // thread may still hold some of the permission for the ports, which takes precedence. - if error.holds_suppression() { - barrier.suppressed = true; - let released = barrier.release(); - if !matches!(error, PreCaptureError::PortRelease { .. }) { - released?; - } - } - return Err(match error { + // after a preparation that succeeded. + barrier.suppressed = error.holds_suppression(); + let error = match error { PreCaptureError::BootChecks(error) => { CaptureError::new(CaptureStage::BootChecks, CaptureCause::Time(error)) } @@ -246,7 +239,8 @@ impl CapturePreparation { PreCaptureError::Suppression(error) | PreCaptureError::Unrestored(error) => { CaptureError::new(CaptureStage::Suppression, CaptureCause::Io(error.into())) } - }); + }; + return Err(barrier.fail(error)); } barrier.suppressed = true; Ok(barrier) @@ -307,11 +301,18 @@ impl CaptureBarrier { /// the detectors' settings, as [`cancel`](Self::cancel) does. A request that fails restores /// the detectors' settings, but leaves the fence closed, since it may fail in a clone whose /// restore it could not repair, where no execution may start. Either way, the caller ends - /// the VM. + /// the VM, and a failure to restore the settings is a [`CaptureStage::Release`] error, unless + /// the calling thread may still hold some of the permission for the time ABI's ports, which + /// [`TimeError::PortRelease`] reports instead. pub fn capture(mut self) -> Result, CaptureError> { - let generation_id = self.system.generation_id().map_err(|error| { - CaptureError::new(CaptureStage::GenerationId, CaptureCause::Time(error)) - })?; + let generation_id = match self.system.generation_id() { + Ok(generation_id) => generation_id, + Err(error) => { + let error = + CaptureError::new(CaptureStage::GenerationId, CaptureCause::Time(error)); + return Err(self.fail(error)); + } + }; let request = CaptureRequest::new( ScratchPolicy::Fresh, generation_id, @@ -322,10 +323,8 @@ impl CaptureBarrier { Err(error) => { // The fence is dropped closed. self.fence = None; - return Err(CaptureError::new( - CaptureStage::Capture, - CaptureCause::Time(error), - )); + let error = CaptureError::new(CaptureStage::Capture, CaptureCause::Time(error)); + return Err(self.fail(error)); } }; match outcome { @@ -369,6 +368,21 @@ impl CaptureBarrier { } Ok(()) } + + /// Releases what the barrier holds after `error`, as [`release`](Self::release) does, and + /// returns `error`, or the failure to restore the stall detectors' settings, unless `error` + /// tells that the calling thread may still hold some of the permission for the time ABI's + /// ports, which takes precedence. + fn fail(&mut self, error: CaptureError) -> CaptureError { + match self.release() { + Err(release) + if !matches!(error.cause, CaptureCause::Time(TimeError::PortRelease(_))) => + { + release + } + _ => error, + } + } } impl Drop for CaptureBarrier { diff --git a/nvx-rs/src/guest/clone/tests.rs b/nvx-rs/src/guest/clone/tests.rs index e14732dd9..324e1d64c 100644 --- a/nvx-rs/src/guest/clone/tests.rs +++ b/nvx-rs/src/guest/clone/tests.rs @@ -134,6 +134,8 @@ enum Outcome { Restored, /// The request fails. Fails, + /// The request fails, and the thread may still hold some of the permission for the ports. + PortRelease, } /// A system that records its calls, with a mount table, devices, and a time ABI that a test @@ -273,6 +275,7 @@ impl System for FakeSystem { Outcome::Continued => Ok(CaptureOutcome::Continued), Outcome::Restored => Ok(CaptureOutcome::Restored(Restore::to(RESTORED))), Outcome::Fails => Err(TimeError::StateUnavailable), + Outcome::PortRelease => Err(PortError::Revoke(Errno::ENOMEM).into()), } } } @@ -802,6 +805,50 @@ fn a_failed_capture_restores_the_detectors_and_opens_the_fence_only_before_its_r ); } +#[test] +fn a_failed_capture_whose_detectors_cannot_be_restored_is_a_release_error() { + let release = "failed to restore the stall detectors: read /run/nvx/time-suppression: No such \ + file or directory"; + let calls = Calls::default(); + let failing = |calls: &Calls| { + let mut system = FakeSystem::new(calls); + system.restoration_fails = true; + system + }; + let mut system = failing(&calls); + system.generation_fails = true; + let error = prepared(&calls, system).capture().unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Release); + assert_eq!(error.to_string(), release); + assert_eq!( + calls.take(), + [Call::GenerationId, Call::Open, Call::CancelCapture] + ); + + let mut system = failing(&calls); + system.outcome = Outcome::Fails; + let error = prepared(&calls, system).capture().unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Release); + assert_eq!(error.to_string(), release); + let request_calls = [ + Call::GenerationId, + Call::Capture(platform_request()), + Call::CancelCapture, + ]; + assert_eq!(calls.take(), request_calls); + + // A permission for the ports that the thread may still hold takes precedence. + let mut system = failing(&calls); + system.outcome = Outcome::PortRelease; + let error = prepared(&calls, system).capture().unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Capture); + assert!( + matches!(error.cause(), CaptureCause::Time(TimeError::PortRelease(_))), + "{error:?}" + ); + assert_eq!(calls.take(), request_calls); +} + #[test] fn errors_name_their_stage_and_their_cause() { let error = CaptureError::new(CaptureStage::Suppression, CaptureCause::Pending); From 991e4e0ed227f51e5a1563ca2beb7054851f1745 Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 12:25:36 -0700 Subject: [PATCH 6/7] guest: show a clone's restore finish before its fence opens The module's example opened a restored clone's admission fence as soon as it took the fence from the restore, before the clone reseeded and finished, and if needed acknowledged, its restore. Show those steps first, as the time module's example does, so the example keeps nothing running before the restore's acknowledgement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da708b6-0bda-4804-b01b-936cdc5f054a --- nvx-rs/src/guest/clone.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/nvx-rs/src/guest/clone.rs b/nvx-rs/src/guest/clone.rs index 68eada608..53f7721d5 100644 --- a/nvx-rs/src/guest/clone.rs +++ b/nvx-rs/src/guest/clone.rs @@ -44,7 +44,9 @@ //! use std::io; //! //! use nvx::guest::clone::{AdmissionFence, CapturePreparation, Captured, Devices}; +//! use nvx::guest::reseed::{self, Seed}; //! use nvx::guest::root::Device; +//! use nvx::guest::time::{RestoreFinish, TimeAbi}; //! use nvx::protocol::root::{DISTRO_MMIO_BASE, SCRATCH_MMIO_BASE}; //! //! /// The agent's admission of executions, which it checks before it starts one. @@ -74,10 +76,15 @@ //! match barrier.capture()? { //! Captured::Continued => println!("the VM continued without a restore"), //! Captured::Restored(restored) => { -//! // A clone continues here: it repairs the restore, and opens the fence once repair -//! // acknowledged it. +//! // A clone continues here, with its wall clock repaired. It finishes the restore, and +//! // acknowledges it if the restore needs that, before its fence opens. +//! reseed::reseed(&Seed::read(restored.entropy_path())?)?; //! let (restore, fence) = restored.into_parts(); -//! println!("restored to {}", restore.generation_id()); +//! let mut finish = RestoreFinish::new(); +//! if restore.acknowledgement_required() { +//! finish = finish.acknowledge(); +//! } +//! TimeAbi::new().restore_finish(finish)?; //! fence.open(); //! } //! } From 060eefc2d3ccaca66aa5ab8429898cf96eb9750a Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Sat, 24 Oct 2026 12:36:31 -0700 Subject: [PATCH 7/7] guest: make closing an admission fence acquire it A caller may lend preparations several handles of one admission state. With a close that succeeds on a closed fence, a second preparation, which would fail with the first one's suppression pending, reopened the fence while the first barrier still held it, and executions could start after the first preparation's mount check. AdmissionFence::close now fails while the fence is closed, whoever closed it, and a failure leaves the fence as it was. Since a preparation never opens a fence that failed to close, it opens only one that it closed. The example's fence refuses a second close, and the tests' fence shares its state between handles, refuses a close while closed, and checks that nothing opens an open fence, with tests of two preparations behind one gate and of a preparation behind a restored clone's closed fence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da708b6-0bda-4804-b01b-936cdc5f054a --- nvx-rs/src/guest/clone.rs | 27 ++++++++---- nvx-rs/src/guest/clone/tests.rs | 77 +++++++++++++++++++++++++++++---- 2 files changed, 86 insertions(+), 18 deletions(-) diff --git a/nvx-rs/src/guest/clone.rs b/nvx-rs/src/guest/clone.rs index 53f7721d5..22d1580d3 100644 --- a/nvx-rs/src/guest/clone.rs +++ b/nvx-rs/src/guest/clone.rs @@ -9,7 +9,8 @@ //! their MMIO windows name, and the host filesystem device, which its virtio-fs tag names. //! [`CapturePreparation::prepare`], in order: //! -//! 1. closes the caller's fence, so that no execution starts; +//! 1. closes the caller's fence, so that no execution starts; a fence that is closed already +//! fails to close, which stops the preparation; //! 2. reads the mount table of the caller's mount namespace, `/proc/self/mountinfo`, at most //! [`MOUNT_TABLE_LIMIT`] bytes of it, and refuses to capture while anything is mounted from the //! distro device or one of its partitions, the scratch device, an overlay, or the filesystem @@ -56,6 +57,10 @@ //! //! impl AdmissionFence for Admission { //! fn close(&mut self) -> io::Result<()> { +//! // Another preparation, or a clone before its repair, holds the fence closed. +//! if !self.open { +//! return Err(io::ErrorKind::ResourceBusy.into()); +//! } //! self.open = false; //! Ok(()) //! } @@ -140,10 +145,14 @@ const ENTROPY_PATH: &str = "/run/nvx/restore-entropy"; /// fails leaves it closed. The fence need not end the executions that run already: a template's /// guest captures before it starts any. A preparation takes the fence, so a caller that keeps /// its admission's state lends it a handle of that state, or a mutable reference to its fence, -/// which the trait's implementation for `&mut F` takes. +/// which the trait's implementation for `&mut F` takes. Several handles may share one state, so +/// closing the fence acquires it: [`close`](Self::close) fails while the fence is closed, such as +/// by another preparation's barrier, and a preparation opens only a fence that it closed, so it +/// never opens one that another holds closed. pub trait AdmissionFence { - /// Closes the fence: once it returns, no execution starts until [`open`](Self::open). A - /// failure leaves the fence open. + /// Closes the fence: once it returns, no execution starts until [`open`](Self::open). It + /// fails while the fence is closed already, whoever closed it, and a failure leaves the fence + /// as it was. fn close(&mut self) -> io::Result<()>; /// Opens the fence that [`close`](Self::close) closed, so that executions start again. @@ -200,9 +209,9 @@ impl CapturePreparation { /// /// The wait for the boot checks lasts 30 seconds at most. A capture check that fails reports /// a violation of the time ABI, which stops the VM in enforcing mode; in report-only mode, - /// the preparation goes on, as `nvx-time pre-capture` does. A preparation that fails opens - /// the fence again, and leaves the detectors' settings as they were: when it suppressed some - /// already, it restores their settings, and a failure to restore them is a + /// the preparation goes on, as `nvx-time pre-capture` does. A preparation that fails after it + /// closed the fence opens it again, and leaves the detectors' settings as they were: when it + /// suppressed some already, it restores their settings, and a failure to restore them is a /// [`CaptureStage::Release`] error. One after which the calling thread may still hold some of /// the permission for the time ABI's ports fails with [`TimeError::PortRelease`] instead, /// after which the thread must start neither a thread nor a process, which would inherit it. @@ -217,7 +226,7 @@ impl CapturePreparation { /// Implements [`prepare`](Self::prepare) on `system`. fn prepare_with(self, system: Box) -> Result, CaptureError> { let Self { mut fence, devices } = self; - // A fence that failed to close is dropped closed or not, but never opened. + // A fence that failed to close, which another may hold closed, is never opened. fence .close() .map_err(|error| CaptureError::new(CaptureStage::Fence, CaptureCause::Io(error)))?; @@ -476,7 +485,7 @@ impl fmt::Debug for Restored { #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] #[non_exhaustive] pub enum CaptureStage { - /// Closing the caller's fence of workload admission. + /// Closing the caller's fence of workload admission, which fails while it is closed already. Fence, /// Checking that nothing is mounted from the template's devices or an overlay. Mounts, diff --git a/nvx-rs/src/guest/clone/tests.rs b/nvx-rs/src/guest/clone/tests.rs index 324e1d64c..7a880c4a2 100644 --- a/nvx-rs/src/guest/clone/tests.rs +++ b/nvx-rs/src/guest/clone/tests.rs @@ -7,6 +7,7 @@ use std::fs; use std::io; use std::os::unix::fs::symlink; use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, PoisonError}; use super::mounts::{self, DeviceNumber}; @@ -82,12 +83,30 @@ impl Calls { } /// A fence that records when it closes and opens, and that fails to close with an error that a -/// test chooses. +/// test chooses, or while it is closed, whichever of its handles closed it. struct FakeFence { /// The log. calls: Calls, /// The error number with which the fence fails to close, if any. failure: Option, + /// Whether the fence is closed, which its handles share. + closed: Arc, +} + +impl FakeFence { + /// Returns another handle of the fence, which shares its state and its log. + fn handle(&self) -> Self { + Self { + calls: self.calls.clone(), + failure: None, + closed: Arc::clone(&self.closed), + } + } + + /// Returns whether the fence is closed. + fn is_closed(&self) -> bool { + self.closed.load(Ordering::SeqCst) + } } impl AdmissionFence for FakeFence { @@ -95,11 +114,18 @@ impl AdmissionFence for FakeFence { if let Some(errno) = self.failure { return Err(errno.into()); } + if self.closed.swap(true, Ordering::SeqCst) { + return Err(Errno::EBUSY.into()); + } self.calls.push(Call::Close); Ok(()) } fn open(&mut self) { + assert!( + self.closed.swap(false, Ordering::SeqCst), + "the fence opened while open" + ); self.calls.push(Call::Open); } } @@ -301,11 +327,12 @@ fn devices() -> Devices { } } -/// Returns a fence that records its calls in `calls`, and closes. +/// Returns an open fence that records its calls in `calls`. fn fence(calls: &Calls) -> FakeFence { FakeFence { calls: calls.clone(), failure: None, + closed: Arc::default(), } } @@ -560,26 +587,54 @@ fn a_mount_table_that_cannot_be_checked_blocks_a_capture() { fn a_fence_that_fails_to_close_stops_the_preparation() { let calls = Calls::default(); let fence = FakeFence { - calls: calls.clone(), - failure: Some(Errno::EBUSY), + failure: Some(Errno::EIO), + ..fence(&calls) }; let error = prepare(fence, FakeSystem::new(&calls)).unwrap_err(); assert_eq!(error.stage(), CaptureStage::Fence); let CaptureCause::Io(cause) = error.cause() else { panic!("{error:?}"); }; - assert_eq!(cause.raw_os_error(), Some(Errno::EBUSY as i32)); + assert_eq!(cause.raw_os_error(), Some(Errno::EIO as i32)); assert_eq!( error.to_string(), - format!( - "failed to fence workload admission: {}", - Errno::EBUSY.desc() - ) + format!("failed to fence workload admission: {}", Errno::EIO.desc()) ); // Nothing else runs, and the fence that did not close does not open. assert_eq!(calls.take(), Vec::::new()); } +#[test] +fn a_preparation_behind_a_fence_that_another_holds_closed_fails_and_leaves_it_closed() { + let calls = Calls::default(); + let gate = fence(&calls); + let barrier = prepare(gate.handle(), FakeSystem::new(&calls)).unwrap(); + assert_eq!(calls.take(), preparation_calls()); + // A second preparation behind the same gate, whose time ABI would find the first one's + // suppression pending, cannot close the fence, so it never opens it. + let mut system = FakeSystem::new(&calls); + system.preparation = Preparation::Pending; + let error = prepare(gate.handle(), system).unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Fence); + let CaptureCause::Io(cause) = error.cause() else { + panic!("{error:?}"); + }; + assert_eq!(cause.raw_os_error(), Some(Errno::EBUSY as i32)); + assert_eq!(calls.take(), Vec::::new()); + assert!(gate.is_closed()); + + // The first barrier holds the fence closed until it opens it, after which the next + // preparation closes it again. + barrier.cancel().unwrap(); + assert_eq!(calls.take(), [Call::Open, Call::CancelCapture]); + assert!(!gate.is_closed()); + let barrier = prepare(gate.handle(), FakeSystem::new(&calls)).unwrap(); + assert_eq!(calls.take(), preparation_calls()); + drop(barrier); + assert_eq!(calls.take(), [Call::Open, Call::CancelCapture]); + assert!(!gate.is_closed()); +} + #[test] fn a_failed_time_preparation_names_its_stage_and_opens_the_fence() { // Each failure, its stage, and whether the time ABI holds the detectors' suppression. @@ -749,6 +804,10 @@ fn a_restored_clone_gets_the_fence_closed_with_the_detectors_suppressed() { let (restore, mut fence) = clone.into_parts(); assert_eq!(restore, Restore::to(RESTORED)); assert_eq!(calls.take(), Vec::::new()); + // A preparation behind another handle of the fence cannot close it, so it opens nothing. + let error = prepare(fence.handle(), FakeSystem::new(&calls)).unwrap_err(); + assert_eq!(error.stage(), CaptureStage::Fence); + assert_eq!(calls.take(), Vec::::new()); // The caller opens the fence once repair acknowledged the restore. fence.open(); assert_eq!(calls.take(), [Call::Open]);