From 0a62c9e4e0617a3312ff7982af442185581961ed Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Mon, 9 Nov 2026 20:03:59 -0800 Subject: [PATCH 1/2] tests: give the runtime adapter its own announcer launcher The runtime adapter's guest worker starts the announcer of a clone's listeners after the process-free application with nvx::guest::ingress::Announcer::start, and classifies its StartError. nvx-ingress start is to spawn its announcer itself, so that Announcer, PROGRAM and StartError leave nvx (SL6.13c), and the adapter is their last other user until SL5.2 deletes the adapter. Give the worker a launcher of its own, Announcement, with the launch that Announcer::start gave it: it binds a UDP socket to the highest free privileged source port, from 1023 down to 512, runs /sbin/nvx-ingress inherit GATEWAY_IPV4 through nvx::guest::spawn with that socket as its standard input, in a session of its own, with every signal's default disposition and none blocked, waits until the program runs, and stops and collects the announcer when the worker tears its root down or drops it. AnnounceFailure carries the launcher's io::Error, and the guest-session fake fails with it, so both still end the clone with ACTIVATION_NETWORK. The worker's new unit tests check the launch, the stop and the drop, and a failed exec, which leaves no child: they take over the cases of nvx's Announcer tests, which the next commit retires with Announcer. The case matrix gives them a fate: they retire with the adapter, and it points to the tests of nvx-ingress start's own launch. Validated on nvx-kvm-test (linux-kvm, virtual machine), where the adapter's protocol-only, host-only and musl guest-only configurations pass their CI checks, the new unit tests among them, and on nvx-mshv-test (linux-mshv, virtual machine), where vm_clone's scenario of the clones of every network policy passes: the host reaches a clone's listener once the announcer that Announcement started announced it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/acceptance-case-matrix.md | 3 +- tests/runtime-adapter/README.md | 10 +- tests/runtime-adapter/src/worker.rs | 111 ++++++++++++++++-- .../runtime-adapter/src/worker/activation.rs | 7 +- .../src/worker/activation/tests.rs | 6 +- tests/runtime-adapter/src/worker/tests.rs | 110 ++++++++++++++++- tests/runtime-adapter/tests/guest_session.rs | 12 +- 7 files changed, 229 insertions(+), 30 deletions(-) diff --git a/doc/acceptance-case-matrix.md b/doc/acceptance-case-matrix.md index dc3b03d4a..e19623a27 100644 --- a/doc/acceptance-case-matrix.md +++ b/doc/acceptance-case-matrix.md @@ -27,7 +27,7 @@ its own in turn. | Section | Tests | Kept | Rewritten | Retired | | --- | ---: | ---: | ---: | ---: | | [The adapter's scenarios](#the-adapters-scenarios) | 157 | 0 | 114 | 43 | -| [The adapter's other tests](#the-adapters-other-tests) | 166 | 0 | 9 | 157 | +| [The adapter's other tests](#the-adapters-other-tests) | 169 | 0 | 9 | 160 | | [MicroVM scenarios](#microvm-scenarios) | 52 | 49 | 3 | 0 | | [CI](#ci) | 69 | 43 | 3 | 23 | | [Docs](#docs) | 34 | 33 | 1 | 0 | @@ -277,6 +277,7 @@ check the adapter's own code, and CI runs in its configurations of the adapter. | `src/worker/executions/tests.rs` | `a_cancellation_after_the_timeout_leaves_a_program_that_ended_before_it_its_own_end` | retired | The adapter's guest worker, over the exec engine that leaves (#1087) | | `src/worker/mapping/tests.rs` | `refusals_name_their_kind_and_path`, `the_probes_take_named_targets_and_paths_in_the_export` | retired | The adapter's guest worker and its probes | | `src/worker/probes/tests.rs` | `a_mount_table_is_read_with_its_escapes`, `escapes_are_three_octal_digits`, `process_records_are_read_as_the_kernel_writes_them`, `the_memory_total_is_read_in_bytes`, `the_os_release_id_loses_its_quotes`, `unknown_probes_and_arguments_are_refused`, `the_sectors_read_are_the_third_field_of_a_disk_s_statistics`, `the_time_state_s_lines_are_facts_with_their_keys_in_the_facts_form`, `the_first_partition_is_reported_in_hexadecimal`, `the_cpus_that_a_restore_brought_online_are_those_of_its_record` | retired | The adapter's probes | +| `src/worker/tests.rs` | `announcer_launch_keeps_the_socket_and_monitors_its_owned_child`, `announcer_drop_stops_and_reaps_the_child_that_it_owns`, `announcer_failed_exec_reports_the_cause_and_leaves_no_child` | retired | The adapter's own launcher of a clone's announcer, which SL6.13c added when it deleted `nvx::guest::ingress::Announcer`. `nvx-ingress start`'s launch: `nvx-rs/src/guest/ingress/tests.rs` | | `src/worker/tests.rs` | `pid_1_goes_on_past_the_stops_of_the_threads_that_it_traces` | retired | The adapter's PID 1. nvx-init's: `nvx-rs/src/guest/init/tests.rs` | | `tests/guest_session.rs` | `the_guest_agent_answers_messages_that_span_records`, `a_reset_starts_the_new_epoch_with_a_new_message`, `the_guest_agent_ends_with_its_stream`, `the_guest_agent_fails_on_records_that_the_protocol_does_not_allow`, `the_guest_agent_refuses_messages_that_its_framing_does_not_allow`, `the_guest_agent_of_a_vm_without_a_layered_root_answers_probes_with_a_failure`, `the_guest_binary_serves_the_session_on_its_standard_streams`, `the_guest_agent_serves_a_tty_in_raw_mode`, `the_guest_binary_serves_a_tty_in_place_of_the_managed_agent`, `the_worker_assembles_the_root_serves_it_and_tears_it_down_on_stop`, `the_worker_assembles_an_image_over_ram_in_the_overlay_mode_of_its_configuration`, `the_worker_assembles_a_gpt_disk_with_the_first_partition_bound_of_its_configuration`, `the_worker_reports_the_stage_that_failed_and_ends_with_its_status`, `the_worker_checks_the_account_of_the_sandbox_s_workload_identity_in_the_assembled_root`, `the_worker_reports_a_teardown_that_failed_before_it_confirms_the_stop`, `the_worker_accepts_only_its_configuration_or_a_stop_before_its_root`, `a_broker_that_closes_only_its_end_of_the_stream_receives_what_the_worker_queued`, `the_worker_mounts_the_export_applies_the_mapping_table_in_order_and_starts_the_sandbox`, `a_table_of_zero_mappings_does_no_work`, `a_failed_mapping_latches_the_root_and_holds_the_sandbox_back`, `the_worker_refuses_parts_that_do_not_continue_the_table`, `the_worker_refuses_executions_and_the_start_out_of_order`, `the_guest_binary_runs_pid_1_and_the_worker_of_a_layered_root`, `a_test_that_gives_up_on_pid_1_ends_every_process_that_it_started`, `a_test_ends_every_process_that_outlives_pid_1`, `pid_1_collects_what_real_time_signals_leave_and_supervises_on`, `pid_1_finds_the_children_that_real_time_signals_end_whatever_their_names`, `the_worker_runs_executions_at_once_with_frame_counted_credit_and_one_output_pool`, `a_stalled_sink_stalls_only_its_own_execution_while_timers_cancellations_and_control_go_on`, `the_worker_refuses_probes_while_it_runs_executions`, `a_full_stream_to_the_host_holds_back_only_output_while_cancellations_and_stops_go_on`, `executions_that_write_without_end_take_turns_at_the_stream_to_the_host`, `the_worker_refuses_starts_beyond_its_limits_with_a_failed_launch`, `the_worker_ends_its_session_on_messages_about_executions_that_break_their_rules`, `a_reset_or_a_later_generation_abandons_executions_whose_programs_end`, `the_worker_ends_its_executions_when_the_host_stops_it`, `the_worker_collects_the_status_of_a_program_that_outlives_its_execution`, `the_worker_ends_what_is_left_of_the_process_group_of_a_program_that_ended`, `an_execution_s_settlement_ends_a_descendant_that_left_its_process_group`, `an_execution_that_does_not_settle_keeps_its_place_until_the_host_stops_the_worker`, `an_execution_runs_in_its_working_directory_with_exactly_its_environment`, `an_execution_enters_the_root_as_the_identity_that_its_start_names`, `an_execution_enters_the_root_with_the_publication_of_listeners_that_its_configuration_names`, `the_worker_refuses_at_stage_0_a_start_whose_identity_breaks_the_rules`, `the_worker_collects_the_status_of_a_program_that_a_real_time_signal_ended`, `an_execution_that_starts_with_the_root_counts_its_timeout_from_its_own_start`, `a_host_that_a_test_drops_while_its_broker_pauses_ends_the_worker_and_its_programs`, `the_worker_remembers_the_generations_that_the_host_started`, `the_worker_runs_executions_on_a_tty`, `a_clone_answers_the_restore_s_reset_and_activates_with_the_record_of_its_activate`, `a_clone_takes_nothing_but_its_activate_before_it_is_ready`, `a_clone_s_activation_fails_finally_with_the_status_of_the_failure_s_class`, `application_failures_follow_export_gateway_and_view_order`, `a_clone_fails_on_an_activate_longer_than_a_record_before_it_takes_its_body`, `a_clone_that_the_host_does_not_activate_in_time_ends_with_the_deadline_s_status`, `a_clone_s_application_ends_by_the_activation_s_deadline`, `a_clone_takes_what_came_during_its_application_before_it_is_ready`, `a_clone_whose_stream_takes_nothing_still_ends_once_its_activation_failed`, `a_clone_whose_broker_closes_the_stream_before_it_is_ready_is_abandoned`, `a_clone_whose_broker_is_gone_still_ends_as_its_activation_tells`, `a_ready_clone_fails_on_an_activate_that_is_not_a_retry`, `a_clone_s_executions_name_their_own_identity_and_take_the_hostname_of_its_record`, `a_clone_s_executions_enter_the_root_with_the_publication_of_its_record_s_network`, `a_clone_whose_announcer_ends_ends_with_the_network_s_status`, `a_clone_reports_when_its_events_happened_after_the_acknowledgement`, `a_clone_maps_its_record_s_table_from_its_export_and_leaves_the_proxy_to_the_host` | retired | The adapter's guest agent and worker. NVX's guest session, nvx-init and managed agent: `nvx-rs/tests/nvxs/guest.rs`, `nvx-rs/src/guest/init/tests.rs` and its `tests/`, and `nvx-rs/src/guest/managed_agent/**/tests.rs` | | `tests/independent_build.rs` | `the_adapter_is_a_workspace_of_its_own_with_its_own_lockfile`, `the_adapter_declares_the_minimum_rust_version_of_nvx`, `the_host_binary_reports_the_contract_that_it_was_built_against`, `the_guest_binary_reports_the_contract_that_it_was_built_against` | retired | The adapter's own workspace, manifest and binaries | diff --git a/tests/runtime-adapter/README.md b/tests/runtime-adapter/README.md index 42eca7760..e5bc05e64 100644 --- a/tests/runtime-adapter/README.md +++ b/tests/runtime-adapter/README.md @@ -855,8 +855,8 @@ after the repair's acknowledgement of the restore: file, with every bind owned by the root. The library starts no process. After successful application this temporary adapter uses the returned publication and gateway to start its announcer, - `/sbin/nvx-ingress inherit GATEWAY_IPV4`, with - `nvx::guest::ingress::Announcer::start`: it binds a UDP socket to a + `/sbin/nvx-ingress inherit GATEWAY_IPV4`, with its own launcher, through + `nvx::guest::spawn::Command`: it binds a UDP socket to a privileged source port, from which alone OpenVMM's gateway takes announcements, and runs the program with that socket as its standard input, to the template's gateway, waiting until the program runs, but for no @@ -2178,7 +2178,11 @@ arguments that the worker refuses. Those of fake root does, hand an execution the host's cancellation once its timeout's instant passed, after its program ended before it, as a worker that did not run meanwhile would, and check that the program's end stays its own. -Those of `worker` run PID 1's collection alone, in a run of the test binary of +Those of `worker` check, on x86-64, the adapter's own launcher of a clone's +announcer, which runs its program with its socket as its standard input, in a +session of its own, stops it and collects its status when stopped or dropped, +and reports a failed exec, after which no child remains; and they run PID 1's +collection alone, in a run of the test binary of its own, with a thread of each of two children that it traces, which stop before the worker ends, one with a signal whose report `nix` decodes and one with a real-time signal, and check that PID 1 takes both stops and goes on diff --git a/tests/runtime-adapter/src/worker.rs b/tests/runtime-adapter/src/worker.rs index dd80fe88c..c031c4f1d 100644 --- a/tests/runtime-adapter/src/worker.rs +++ b/tests/runtime-adapter/src/worker.rs @@ -301,6 +301,8 @@ pub mod probes; use std::fs::{self, File, OpenOptions}; use std::io::{self, Read, Write}; +#[cfg(target_arch = "x86_64")] +use std::net::{Ipv4Addr, UdpSocket}; use std::os::fd::{AsFd, BorrowedFd}; use std::os::unix::fs::OpenOptionsExt; use std::os::unix::process::ExitStatusExt; @@ -323,8 +325,6 @@ use nvx::guest::clone::ApplyError; #[cfg(target_arch = "x86_64")] use nvx::guest::clone::{self, Activation, RootTemplate}; use nvx::guest::entry::{self, Entry, Identity, Prepared, Publication}; -#[cfg(target_arch = "x86_64")] -use nvx::guest::ingress::{Announcer, StartError}; use nvx::guest::mappings::{Mapping, SourceRoot}; use nvx::guest::nvxs::GuestConnectionError; use nvx::guest::process::Child; @@ -1138,7 +1138,7 @@ fn run_probe(operations: &mut dyn RootOperations, body: &[u8]) -> Message { pub struct LibraryRoot { /// The temporary adapter's announcer, started by this caller only after library application. #[cfg(target_arch = "x86_64")] - announcer: Option, + announcer: Option, /// The process ID of PID 1's unrelated child, which the probes check, if PID 1 named one. sibling: Option, /// What the bootstrap prepared, once it ran. @@ -1386,7 +1386,11 @@ impl RootOperations for LibraryRoot { /// down with its view binds. A root whose teardown failed keeps its mounts. fn teardown(&mut self) -> Result<(), String> { #[cfg(target_arch = "x86_64")] - let stopped = self.announcer.take().map(Announcer::stop).transpose(); + let stopped = self + .announcer + .take() + .map(|mut announcement| announcement.stop()) + .transpose(); self.export = None; let torn = match self.root.take() { Some(root) => root.teardown(), @@ -1406,7 +1410,7 @@ impl RootOperations for LibraryRoot { fn announcer(&self) -> Option> { #[cfg(target_arch = "x86_64")] { - self.announcer.as_ref().map(Announcer::pidfd) + self.announcer.as_ref().and_then(|child| child.0.pidfd()) } #[cfg(not(target_arch = "x86_64"))] { @@ -1414,12 +1418,11 @@ impl RootOperations for LibraryRoot { } } - /// Collects the status of the announcer that this caller started, with - /// `nvx::guest::ingress::Announcer::try_wait`. + /// Collects the status of the announcer that this caller started. fn announcer_ended(&mut self) -> Option { #[cfg(target_arch = "x86_64")] { - match self.announcer.as_mut()?.try_wait() { + match self.announcer.as_mut()?.0.try_wait() { Ok(None) => None, Ok(Some(status)) => Some(format!( "the announcer of the workload's listeners ended: {status}" @@ -1450,7 +1453,7 @@ impl RootOperations for LibraryRoot { let gateway = applied .gateway() .expect("application checked a published gateway"); - let announcer = match Announcer::start(gateway) { + let announcer = match Announcement::start(gateway) { Ok(announcer) => announcer, Err(error) => { let teardown = applied.teardown().err(); @@ -1468,11 +1471,11 @@ impl RootOperations for LibraryRoot { /// A temporary adapter caller's announcer failure, after successful library application. #[cfg(target_arch = "x86_64")] #[derive(Debug)] -struct AnnounceFailure { +pub struct AnnounceFailure { /// Why the caller could not start ingress. - error: StartError, + pub error: io::Error, /// Any failure while closing the export and tearing the applied root down. - teardown: Option, + pub teardown: Option, } #[cfg(target_arch = "x86_64")] @@ -1493,6 +1496,90 @@ impl std::error::Error for AnnounceFailure { } } +/// The temporary adapter's announcer, stopped and reaped before its applied root is dropped. +#[cfg(target_arch = "x86_64")] +#[derive(Debug)] +struct Announcement(Child); + +#[cfg(target_arch = "x86_64")] +impl Announcement { + /// Binds the announcer's privileged socket before launching its program. + fn start(gateway: Ipv4Addr) -> io::Result { + for port in (512..=1023).rev() { + match UdpSocket::bind((Ipv4Addr::UNSPECIFIED, port)) { + Ok(socket) => { + let command = spawn::Command::new( + c"/sbin/nvx-ingress".to_owned(), + vec![ + c"nvx-ingress".to_owned(), + c"inherit".to_owned(), + std::ffi::CString::new(gateway.to_string()) + .expect("an IPv4 address has no NUL"), + ], + ); + return Self::launch(command, socket); + } + Err(error) if error.raw_os_error() == Some(Errno::EADDRINUSE as i32) => {} + Err(error) => return Err(error), + } + } + Err(io::Error::new( + io::ErrorKind::AddrInUse, + "no privileged source port, from 1023 down to 512, is free for the announcer's socket", + )) + } + + /// Launches `command` with the bound socket and takes responsibility for its child. + fn launch(command: spawn::Command, socket: UdpSocket) -> io::Result { + let spawned = command + .stdin(socket) + .new_session() + .reset_signals() + .spawn()?; + let id = spawned.id(); + let launched = spawned + .into_report() + .wait() + .and_then(|failure| match failure { + None => Child::with_pidfd(id), + Some(failure) => Err(io::Error::other(failure)), + }); + match launched { + Ok(child) => Ok(Self(child)), + Err(error) => { + let mut failed = Self(Child::new(id)?); + match failed.stop() { + Ok(_) => Err(error), + Err(cleanup) => Err(io::Error::new( + error.kind(), + format!("{error}; cannot stop the failed announcer: {cleanup}"), + )), + } + } + } + } + + /// Stops only this uncollected child, whose process ID cannot yet have been reused. + fn stop(&mut self) -> io::Result { + if self.0.status().is_none() { + match signal::kill(pid(self.0.id()), Signal::SIGKILL) { + Ok(()) | Err(Errno::ESRCH) => {} + Err(error) => return Err(error.into()), + } + } + self.0.wait() + } +} + +#[cfg(target_arch = "x86_64")] +impl Drop for Announcement { + fn drop(&mut self) { + if let Err(error) = self.stop() { + eprintln!("runtime-adapter-guest: cannot stop the announcer: {error}"); + } + } +} + impl EntryOperations for LibraryRoot { /// Runs `Entry::prepare` on the assembled root. fn prepare(&mut self, entrance: &Entrance) -> Result { diff --git a/tests/runtime-adapter/src/worker/activation.rs b/tests/runtime-adapter/src/worker/activation.rs index fa907cea1..bdf900530 100644 --- a/tests/runtime-adapter/src/worker/activation.rs +++ b/tests/runtime-adapter/src/worker/activation.rs @@ -88,7 +88,7 @@ pub const ACTIVATION_PROBE: &str = "activation"; /// publishes the workload's listeners but the template names no gateway, as /// [`ApplyError::Network`] tells, or the announcer of those listeners, which the worker starts /// once the application succeeded, cannot start, as its -/// [`StartError`](nvx::guest::ingress::StartError) tells. The worker also ends a clone whose +/// [`AnnounceFailure`](super::AnnounceFailure) tells. The worker also ends a clone whose /// announcer ended with it, whether the clone was ready or not, outside the activation; /// - [`ACTIVATION_ORDER`](status::ACTIVATION_ORDER): the worker's own order completed an /// activation that did not run, or started the sandbox before the clone was ready, or again. @@ -103,10 +103,7 @@ pub fn failure_status(failure: &ActivationFailure) -> u8 { | ActivationCause::Changed => status::ACTIVATION_PROTOCOL, ActivationCause::Record(_) | ActivationCause::Cold => status::ACTIVATION_RECORD, ActivationCause::Resources { .. } => status::ACTIVATION_RESOURCES, - ActivationCause::Failed(error) - if error.is::() - || error.is::() => - { + ActivationCause::Failed(error) if error.is::() => { status::ACTIVATION_NETWORK } ActivationCause::Failed(error) => match error.downcast_ref::() { diff --git a/tests/runtime-adapter/src/worker/activation/tests.rs b/tests/runtime-adapter/src/worker/activation/tests.rs index 1c62ee4ba..5199ea632 100644 --- a/tests/runtime-adapter/src/worker/activation/tests.rs +++ b/tests/runtime-adapter/src/worker/activation/tests.rs @@ -7,7 +7,6 @@ use nvx::activation::{ }; use nvx::guest::clone::NetworkError; use nvx::guest::entry; -use nvx::guest::ingress::StartError; use nvx::guest::mappings::MappingError; use nvx::guest::root::{self, Assembly, Device, Error as RootError, Role}; use nvx::protocol::root::DISTRO_MMIO_BASE; @@ -194,7 +193,10 @@ fn each_class_of_failure_ends_the_clone_with_its_own_status() { let network = activating().complete(now(), Err(gateway.into())); cases.push((failed(network), status::ACTIVATION_NETWORK)); let unannounced = super::super::AnnounceFailure { - error: StartError::NoPrivilegedPort, + error: io::Error::new( + io::ErrorKind::AddrInUse, + "no privileged source port is free", + ), teardown: None, }; let network = activating().complete(now(), Err(unannounced.into())); diff --git a/tests/runtime-adapter/src/worker/tests.rs b/tests/runtime-adapter/src/worker/tests.rs index 037d2083b..6d938ff51 100644 --- a/tests/runtime-adapter/src/worker/tests.rs +++ b/tests/runtime-adapter/src/worker/tests.rs @@ -1,5 +1,5 @@ -//! Unit tests of PID 1's collection, which waits for any child. Each test runs alone, in a run of -//! this test binary of its own, among no other test's children. +//! Checks of announcer ownership and PID 1's collection. Tests that wait for any child run +//! alone in a test process of their own. use std::env; use std::fs; @@ -22,6 +22,112 @@ const UNTRACEABLE: i32 = 99; /// Longest wait for a process to reach a state. const TIMEOUT: Duration = Duration::from_secs(30); +/// A shell command that stands for the adapter's announcer program. +#[cfg(target_arch = "x86_64")] +fn announcement_command(script: &str) -> nvx::guest::spawn::Command { + nvx::guest::spawn::Command::new( + c"/bin/sh".to_owned(), + vec![ + c"sh".to_owned(), + c"-c".to_owned(), + std::ffi::CString::new(script).unwrap(), + c"nvx-ingress".to_owned(), + c"inherit".to_owned(), + c"10.0.0.1".to_owned(), + ], + ) +} + +#[test] +#[cfg(target_arch = "x86_64")] +fn announcer_launch_keeps_the_socket_and_monitors_its_owned_child() { + use std::io::{BufRead, BufReader}; + use std::os::fd::AsRawFd; + use std::os::unix::net::UnixStream; + use std::os::unix::process::ExitStatusExt; + + let socket = std::net::UdpSocket::bind("127.0.0.1:0").unwrap(); + let inherited = fs::read_link(format!("/proc/self/fd/{}", socket.as_raw_fd())).unwrap(); + let (read, write) = UnixStream::pair().unwrap(); + read.set_read_timeout(Some(TIMEOUT)).unwrap(); + let command = announcement_command( + "read -r id comm state ppid pgrp session tty rest = report.lines().collect(); + let ids: Vec<_> = lines[0].split_whitespace().collect(); + assert_eq!(ids.len(), 4, "{report}"); + assert_eq!(ids[1], ids[0], "{report}"); + assert_eq!(ids[2], ids[0], "{report}"); + assert_eq!(ids[3], "0", "{report}"); + assert_eq!(lines[1], inherited.to_str().unwrap(), "{report}"); + assert_eq!(lines[2..], ["inherit", "10.0.0.1"], "{report}"); + assert!(announcement.0.try_wait().unwrap().is_none()); + assert_eq!( + announcement.stop().unwrap().signal(), + Some(Signal::SIGKILL as i32) + ); +} + +#[test] +#[cfg(target_arch = "x86_64")] +fn announcer_drop_stops_and_reaps_the_child_that_it_owns() { + use std::os::fd::AsFd; + + let socket = std::net::UdpSocket::bind("127.0.0.1:0").unwrap(); + let announcement = + super::Announcement::launch(announcement_command("exec sleep 60"), socket).unwrap(); + let pidfd = announcement + .0 + .pidfd() + .unwrap() + .try_clone_to_owned() + .unwrap(); + drop(announcement); + assert_eq!( + wait::waitid( + Id::PIDFd(pidfd.as_fd()), + WaitPidFlag::WEXITED | WaitPidFlag::WNOHANG, + ), + Err(nix::errno::Errno::ECHILD), + ); +} + +#[test] +#[cfg(target_arch = "x86_64")] +fn announcer_failed_exec_reports_the_cause_and_leaves_no_child() { + const TEST: &str = "worker::tests::announcer_failed_exec_reports_the_cause_and_leaves_no_child"; + if env::var(ROLE).as_deref() != Ok("pid-1") { + isolated(TEST); + return; + } + let socket = std::net::UdpSocket::bind("127.0.0.1:0").unwrap(); + let command = nvx::guest::spawn::Command::new( + c"/proc/self/fd/announcer-missing".to_owned(), + vec![c"nvx-ingress".to_owned()], + ); + let error = super::Announcement::launch(command, socket).unwrap_err(); + let failure = error + .get_ref() + .unwrap() + .downcast_ref::() + .unwrap(); + assert_eq!(failure.stage(), nvx::guest::spawn::LaunchStage::Exec); + assert_eq!(failure.raw_os_error(), nix::libc::ENOENT); + assert_eq!( + wait::waitpid(Pid::from_raw(-1), Some(WaitPidFlag::WNOHANG)), + Err(nix::errno::Errno::ECHILD), + ); +} + #[test] fn pid_1_goes_on_past_the_stops_of_the_threads_that_it_traces() { const TEST: &str = "worker::tests::pid_1_goes_on_past_the_stops_of_the_threads_that_it_traces"; diff --git a/tests/runtime-adapter/tests/guest_session.rs b/tests/runtime-adapter/tests/guest_session.rs index c62e3fafb..62cedb08d 100644 --- a/tests/runtime-adapter/tests/guest_session.rs +++ b/tests/runtime-adapter/tests/guest_session.rs @@ -1406,8 +1406,7 @@ impl RootOperations for FakeRoot { /// device, if any, the gateway refusal before view effects, the view's hostname, generation of /// the machine ID, and write of the resolver file when requested. After application /// succeeds, it stands for this caller's announcer start when listeners are published, and - /// fails with that start's bare `StartError`: unlike the worker, which wraps it with its - /// teardown of the applied root, the fake holds no root to tear down. + /// fails with an adapter-owned `AnnounceFailure`: the fake holds no root to tear down. /// These fail if the test fails /// [`Operation::MachineId`], [`Operation::MountExport`], [`Operation::Resolver`], /// [`Operation::View`], and [`Operation::Announce`]. @@ -1420,7 +1419,6 @@ impl RootOperations for FakeRoot { ) -> Result<(), Box> { use nvx::activation::{self, MachineIdMode}; use nvx::guest::clone::{ApplyError, NetworkError}; - use nvx::guest::ingress::StartError; use nvx::guest::root::{DeviceStep, Error, Mount, Role}; let applied = (record.clone(), template.clone()); @@ -1490,8 +1488,12 @@ impl RootOperations for FakeRoot { })?; } if published { - self.result(Operation::Announce) - .map_err(|problem| StartError::Spawn(io::Error::other(problem)))?; + self.result(Operation::Announce).map_err(|problem| { + runtime_adapter::worker::AnnounceFailure { + error: io::Error::other(problem), + teardown: None, + } + })?; self.announcer = Some(match self.ending.take() { Some(read) => (read, None), None => { From 797e0ac096c8b419cc27c184047e9ef88e6786da Mon Sep 17 00:00:00 2001 From: Enrique Saurez Date: Mon, 9 Nov 2026 20:03:59 -0800 Subject: [PATCH 2/2] guest: let nvx-ingress start spawn its announcer itself nvx-ingress start was the last user in nvx of Announcer, the library spawner of the announcer of a clone's listeners, which depends on the guest::spawn and guest::process modules that later PRs delete. start now binds the announcer's socket through the shared bind_privileged search, to the highest free privileged source port from 1023 down to 512, and spawns /sbin/nvx-ingress inherit GATEWAY_IPV4 with std::process::Command, with that socket as its standard input, and a pre-exec hook of the private OS layer, sys::configure_announcer, which gives it a session of its own and every signal's default disposition, none blocked, as Announcer::start did. It still exits with 0 once the program runs, and with 1 when it does not start, without waiting for any announcement. The message of a failed start no longer names the launch stage, which the standard library does not report. The standard library's spawn collects the status of a child that does not run the program, and panics when the kernel collected it first, as it does every child's when the caller ignores SIGCHLD, a disposition that exec keeps. So start restores SIGCHLD's default disposition with sys::reset_child_signal before it spawns, as nvx-init does before it collects its children, and still exits with 1 then, as it did with Announcer::start, which ignored a failure of that collection. Announcer, PROGRAM and StartError go with their tests, and guest::ingress, whose only public item is now its hidden main, becomes hidden. nvx-init's restored announcer, which took its path from PROGRAM, names /sbin/nvx-ingress itself; it still owns its announcer. The launch's new tests run a script in place of the program, and check its arguments, its socket as standard input, its own session without a terminal, its signals, which none of the caller's ignored or blocked ones reach, and a program that does not run, also from a caller that ignores SIGCHLD, where the spawn panics without the reset. They take over the cases of the retired Announcer tests that still apply; the cases of Announcer's pidfd, stop, drop and StartError go with them. ingress_start_fails_when_the_announcer_does_not_start takes the new message of a failed exec. Validated on nvx-kvm-test (linux-kvm, virtual machine), on feature/nvx-refactor 7bc238eb: format, the Lint step's five Clippy runs, -p nvx --lib --no-default-features and the musl guest run, documentation, public API, test layout, MSRV and macOS checks pass, and so do the launch's tests, guest_tools's ingress cases and nvx-init's restore tests on the GNU and musl targets. The SIGCHLD case fails without the reset, with the standard library's panic, and passes with it. The whole musl guest suite fails only the 26 tests that it failed on that host's Linux 6.6 at the earlier base 8b775211. On nvx-mshv-test (linux-mshv, virtual machine), with OpenVMM, a kernel and an initramfs built from this change on 3340bbd7, test-microvm's untiered-network-policy and private-network-ingress pass: the untiered restores whose policies publish the guest's listeners report their announcer running, and the host reaches the guest's server through the port that it publishes. Before the SIGCHLD reset, on 8b775211, host-loopback-policy, platform-network-policy and network-snapshot passed there too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/README.md | 2 +- nvx-rs/public-api/linux.txt | 19 +- nvx-rs/src/guest/ingress/announcer.rs | 198 -------------------- nvx-rs/src/guest/ingress/announcer/tests.rs | 129 ------------- nvx-rs/src/guest/ingress/mod.rs | 116 ++++++++---- nvx-rs/src/guest/ingress/tests.rs | 135 ++++++++++++- nvx-rs/src/guest/init.rs | 2 +- nvx-rs/src/guest/mod.rs | 8 +- nvx-rs/src/guest/sys/mod.rs | 2 +- nvx-rs/src/guest/sys/socket.rs | 5 +- nvx-rs/src/guest/sys/spawn.rs | 16 ++ nvx-rs/tests/guest_tools.rs | 2 +- 12 files changed, 240 insertions(+), 394 deletions(-) delete mode 100644 nvx-rs/src/guest/ingress/announcer.rs delete mode 100644 nvx-rs/src/guest/ingress/announcer/tests.rs diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 559cd00d8..32db5aac9 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -3392,7 +3392,7 @@ disk's block device. | `nvx-exit [CODE]` | Stops the VM with an 8-bit exit code through OpenVMM's control port 0x604 (x86_64) | | `nvx-device-io file ...`, `nvx-device-io network ...`, `nvx-device-io map PATH TEXT` | The benchmark helper that measures file and UDP round-trip I/O and prints one JSON line, and reports a file benchmark's progress on stderr; `map` writes TEXT at the start of a file through a shared mapping and holds the dirty mapping until its standard input ends, for the microVM scenarios (x86_64) | | `nvx-managed-agent CONTROL_TTY ROOTFS HOSTNAME UID GID USER HOME` | Serves the managed control session and supervises sequential workloads (x86_64) | -| `nvx-ingress [inherit\|start] GATEWAY_IPV4` | Announces reachable TCP listeners and settled UDP sockets to the portable-network gateway, from a privileged source port that it binds, or, with `inherit`, from its standard input, a socket that the caller bound to one after the clone's process-free application; `start` starts `inherit` as `Announcer::start` does and exits once it runs, or with status 1 if it does not start, for a guest agent that cannot hold the announcer; `test DIRECTORY` runs synthetic procfs snapshots | +| `nvx-ingress [inherit\|start] GATEWAY_IPV4` | Announces reachable TCP listeners and settled UDP sockets to the portable-network gateway, from a privileged source port that it binds, or, with `inherit`, from its standard input, a socket that the caller bound to one after the clone's process-free application; `start` binds such a socket and starts `inherit` on it in a session of its own, then exits once its program runs, or with status 1 if it does not start, for a guest agent that cannot hold the announcer; `test DIRECTORY` runs synthetic procfs snapshots | | `nvx-time [--report-only] COMMAND` | Enforces NVX time ABI v1: runs its conformance checks, repairs the wall clock after a restore, publishes the time state, and runs the discipline and violation-watcher daemon; `test NAME ...` runs its decoder and process test hooks (x86_64) | | `nvx-time-probe latency\|warp\|clock ...` | Diagnoses clock-read latency, cross-CPU TSC warp and offset, the clocks, CPUID, clock events, kernel messages, and optionally MSRs (x86_64) | | `nvx-init TIME_DAEMON [ANNOUNCER]` | Takes PID 1 over from a layered root's early boot, with its time daemon and optional announcer: gives itself and them an out-of-memory score adjustment of -1000 and a nice value of -10, collects every child's status, and, after `sync`, ends the VM with status 125 when a helper ends or 0 on `SIGTERM`; elsewhere it exits with 2 (x86_64) | diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index e4a8135f9..00e2038b4 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -1279,24 +1279,7 @@ field nvx::guest::gpt::DeviceError::Size::0 (hidden) variant nvx::guest::gpt::DeviceError::Table (hidden) field nvx::guest::gpt::DeviceError::Table::0 (hidden) fn nvx::guest::gpt::read_disk (hidden) -mod nvx::guest::ingress -struct nvx::guest::ingress::Announcer -method nvx::guest::ingress::Announcer::id -method nvx::guest::ingress::Announcer::pidfd -method nvx::guest::ingress::Announcer::start -method nvx::guest::ingress::Announcer::stop -method nvx::guest::ingress::Announcer::try_wait -const nvx::guest::ingress::PROGRAM -enum nvx::guest::ingress::StartError -variant nvx::guest::ingress::StartError::Bind -field nvx::guest::ingress::StartError::Bind::0 -variant nvx::guest::ingress::StartError::Launch -field nvx::guest::ingress::StartError::Launch::0 -variant nvx::guest::ingress::StartError::NoPrivilegedPort -variant nvx::guest::ingress::StartError::Socket -field nvx::guest::ingress::StartError::Socket::0 -variant nvx::guest::ingress::StartError::Spawn -field nvx::guest::ingress::StartError::Spawn::0 +mod nvx::guest::ingress (hidden) fn nvx::guest::ingress::main (hidden) mod nvx::guest::init (hidden) fn nvx::guest::init::main (hidden) diff --git a/nvx-rs/src/guest/ingress/announcer.rs b/nvx-rs/src/guest/ingress/announcer.rs deleted file mode 100644 index c10866cfa..000000000 --- a/nvx-rs/src/guest/ingress/announcer.rs +++ /dev/null @@ -1,198 +0,0 @@ -//! The announcer of a clone's listeners, which its caller starts after application, as the -//! [module documentation](super) describes. - -use std::ffi::{CStr, CString}; -use std::fmt; -use std::io; -use std::net::Ipv4Addr; -use std::os::fd::BorrowedFd; -use std::process::ExitStatus; - -use super::bind_privileged; -use crate::guest::process::Child; -use crate::guest::spawn::{Command, LaunchFailure}; -use crate::guest::sys::{self, Errno, ProcessId}; - -/// The path of the announcer's program in NVX's guests, which NVX's init runs at a cold boot, and -/// [`Announcer::start`] in a clone. -pub const PROGRAM: &CStr = c"/sbin/nvx-ingress"; - -/// The announcer of the guest's listeners, `nvx-ingress inherit GATEWAY_IPV4`, which -/// [`start`](Self::start) started as a child of the caller, whose status it collects for the -/// caller. -/// -/// The announcer runs until it fails, or until [`stop`](Self::stop) ends it: its -/// [pidfd](Self::pidfd) polls readable once it ended, and [`try_wait`](Self::try_wait) then -/// collects its status. Dropping it ends it as `stop` does, ignoring failures. -#[derive(Debug)] -pub struct Announcer { - /// The announcer's process, which a pidfd names. - child: Child, -} - -impl Announcer { - /// Starts the announcer of the guest's listeners, which announces them to `gateway`, the - /// IPv4 gateway of the guest's NIC, as [`PROGRAM`] does at a cold boot: - /// - /// 1. it creates an IPv4 UDP socket, and binds it to the highest free privileged source - /// port, from 1023 down to 512, as the program binds its own at a cold boot, which needs - /// the guest's root user, or `CAP_NET_BIND_SERVICE`: the gateway takes announcements only - /// from such a port, which no workload without that privilege can bind; - /// 2. it spawns [`PROGRAM`] as `nvx-ingress inherit GATEWAY_IPV4`, with the socket as its - /// standard input, the caller's standard output and error, every signal's default - /// disposition, and none blocked, in a session of its own, without a controlling terminal, - /// and waits until the program runs, as its launch report tells, but no longer: nothing - /// waits for the program's first scan, nor for any announcement. - /// - /// Fails with a [`StartError`] if the socket cannot be created or bound, or the program does - /// not start, once it collected the status of a child that did not run the program. - pub fn start(gateway: Ipv4Addr) -> Result { - let socket = sys::UdpSocket::ipv4().map_err(|errno| StartError::Socket(errno.into()))?; - match bind_privileged(|port| socket.bind_any(port)) { - Ok(Some(_)) => Self::spawn(PROGRAM, gateway, socket), - Ok(None) => Err(StartError::NoPrivilegedPort), - Err(errno) => Err(StartError::Bind(errno.into())), - } - } - - /// Spawns `program` as `nvx-ingress inherit GATEWAY_IPV4`, with `socket` as its standard - /// input, as [`start`](Self::start) does once it bound the socket. - fn spawn( - program: &CStr, - gateway: Ipv4Addr, - socket: sys::UdpSocket, - ) -> Result { - let arguments = vec![ - c"nvx-ingress".to_owned(), - c"inherit".to_owned(), - CString::new(gateway.to_string()).expect("an address has no NUL"), - ]; - let spawned = Command::new(program.to_owned(), arguments) - .stdin(socket) - .new_session() - .reset_signals() - .spawn() - .map_err(StartError::Spawn)?; - let id = spawned.id(); - // Nothing but this start collects the status of the child, which keeps its process ID - // until then. - let launched = spawned.into_report().wait(); - let started = match launched { - Ok(None) => Child::with_pidfd(id).map_err(StartError::Spawn), - Ok(Some(failure)) => Err(StartError::Launch(failure)), - Err(error) => Err(StartError::Spawn(error)), - }; - match started { - Ok(child) => Ok(Self { child }), - Err(error) => { - if let Some(process) = ProcessId::from_positive(i64::from(id)) { - // A child whose launch failed has exited already. - let _ = sys::kill_process(process); - } - if let Ok(mut child) = Child::new(id) { - let _ = child.wait(); - } - Err(error) - } - } - } - - /// Returns the announcer's process ID. - pub fn id(&self) -> u32 { - self.child.id() - } - - /// Lends the announcer's pidfd, which polls readable once the announcer ended. - pub fn pidfd(&self) -> BorrowedFd<'_> { - self.child - .pidfd() - .expect("an announcer is named by its pidfd") - } - - /// Collects the announcer's status if it ended, without blocking, and returns it, or `None` - /// while it runs. Once it has a status, this returns it again. - pub fn try_wait(&mut self) -> io::Result> { - self.child.try_wait() - } - - /// Ends the announcer with `SIGKILL`, unless it ended already, and returns its status, which - /// it collects. - pub fn stop(mut self) -> io::Result { - self.end() - } - - /// Ends the announcer, as [`stop`](Self::stop) does. - fn end(&mut self) -> io::Result { - if self.child.status().is_none() { - match sys::kill_through_pidfd(self.pidfd(), false) { - // An announcer that ended, but whose status nothing collected yet, takes the - // signal as a zombie does. - Ok(()) | Err(Errno::ESRCH) => {} - Err(errno) => return Err(errno.into()), - } - } - self.child.wait() - } -} - -impl Drop for Announcer { - fn drop(&mut self) { - let _ = self.end(); - } -} - -/// Why [`Announcer::start`] failed. -#[derive(Debug)] -#[non_exhaustive] -pub enum StartError { - /// The announcer's UDP socket could not be created, as this error tells. - Socket(io::Error), - /// The announcer's socket could not be bound to a privileged source port, for another reason - /// than the port's use, as this error tells: as without the privilege to bind one. - Bind(io::Error), - /// Every privileged source port, from 1023 down to 512, is in use. - NoPrivilegedPort, - /// The announcer could not be spawned, or its launch could not be followed, as this error - /// tells. - Spawn(io::Error), - /// The announcer's launch failed at a stage, as when its program does not exist, which its - /// exec stage then tells. - Launch(LaunchFailure), -} - -impl fmt::Display for StartError { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - let program = PROGRAM.to_string_lossy(); - match self { - Self::Socket(error) => { - write!( - formatter, - "the announcer's socket cannot be created: {error}" - ) - } - Self::Bind(error) => write!( - formatter, - "the announcer's socket cannot be bound to a privileged source port: {error}" - ), - Self::NoPrivilegedPort => formatter.write_str( - "no privileged source port, from 1023 down to 512, is free for the announcer's \ - socket", - ), - Self::Spawn(error) => write!(formatter, "{program} cannot be started: {error}"), - Self::Launch(failure) => write!(formatter, "{program} did not start: {failure}"), - } - } -} - -impl std::error::Error for StartError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Self::Socket(error) | Self::Bind(error) | Self::Spawn(error) => Some(error), - Self::Launch(failure) => Some(failure), - Self::NoPrivilegedPort => None, - } - } -} - -#[cfg(test)] -mod tests; diff --git a/nvx-rs/src/guest/ingress/announcer/tests.rs b/nvx-rs/src/guest/ingress/announcer/tests.rs deleted file mode 100644 index cdfa2ff00..000000000 --- a/nvx-rs/src/guest/ingress/announcer/tests.rs +++ /dev/null @@ -1,129 +0,0 @@ -use std::fs; -use std::os::unix::ffi::OsStrExt; -use std::os::unix::fs::PermissionsExt; -use std::os::unix::process::ExitStatusExt; -use std::path::Path; -use std::time::{Duration, Instant}; - -use super::*; -use crate::guest::spawn::LaunchStage; - -/// How long a test waits for an announcer to do what it expects. -const PATIENCE: Duration = Duration::from_secs(10); - -/// The gateway of the tests' announcers. -const GATEWAY: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 1); - -/// Writes an executable shell script at `path` whose body is `script`, which stands for the -/// announcer's program, and returns its path. -fn program(directory: &Path, name: &str, script: &str) -> CString { - let path = directory.join(name); - fs::write(&path, format!("#!/bin/sh\n{script}\n")).unwrap(); - fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).unwrap(); - CString::new(path.as_os_str().as_bytes()).unwrap() -} - -/// Returns a socket that stands for the announcer's, bound to a port that needs no privilege. -fn socket() -> sys::UdpSocket { - let socket = sys::UdpSocket::ipv4().unwrap(); - socket.bind_any(0).unwrap(); - socket -} - -/// Returns whether `announcer`'s pidfd polls readable within `milliseconds`. -fn ended(announcer: &Announcer, milliseconds: u16) -> bool { - sys::readable_or_hung_up(&[announcer.pidfd()], milliseconds).unwrap() == [true] -} - -#[test] -fn an_announcer_runs_its_program_with_its_socket_until_it_is_stopped() { - let directory = tempfile::tempdir().unwrap(); - let report = directory.path().join("report"); - let script = format!( - "{{ printf '%s\\n' \"$0\" \"$@\"; stat -L -c %F /dev/stdin; }} >{0}.tmp\n\ - mv {0}.tmp {0}\nexec sleep 60", - report.display() - ); - let program = program(directory.path(), "announcer", &script); - let announcer = Announcer::spawn(&program, GATEWAY, socket()).unwrap(); - let deadline = Instant::now() + PATIENCE; - while !report.exists() { - assert!(Instant::now() < deadline, "the announcer reported nothing"); - std::thread::sleep(Duration::from_millis(10)); - } - // The program takes the gateway after `inherit`, and the socket as its standard input. - let reported = fs::read_to_string(&report).unwrap(); - let lines: Vec<_> = reported.lines().collect(); - assert_eq!(lines[1..], ["inherit", "10.0.0.1", "socket"], "{reported}"); - assert!(!ended(&announcer, 0)); - let status = announcer.stop().unwrap(); - assert_eq!(status.signal(), Some(nix::libc::SIGKILL)); -} - -#[test] -fn an_announcer_that_ends_polls_readable_and_keeps_its_status() { - let directory = tempfile::tempdir().unwrap(); - let program = program(directory.path(), "announcer", "exit 3"); - let mut announcer = Announcer::spawn(&program, GATEWAY, socket()).unwrap(); - assert!(ended(&announcer, 10_000)); - let status = announcer.try_wait().unwrap().unwrap(); - assert_eq!(status.code(), Some(3)); - assert_eq!(announcer.try_wait().unwrap(), Some(status)); - // Its stop signals nothing, and returns the status again. - assert_eq!(announcer.stop().unwrap(), status); -} - -#[test] -fn a_dropped_announcer_ends() { - let directory = tempfile::tempdir().unwrap(); - let program = program(directory.path(), "announcer", "exec sleep 60"); - let announcer = Announcer::spawn(&program, GATEWAY, socket()).unwrap(); - let process = ProcessId::from_positive(i64::from(announcer.id())).unwrap(); - drop(announcer); - // The drop ended it and collected its status, so its process ID names no process. - assert!(!sys::process_exists(process)); -} - -#[test] -fn an_announcer_whose_program_does_not_run_does_not_start() { - let directory = tempfile::tempdir().unwrap(); - let missing = CString::new(directory.path().join("missing").as_os_str().as_bytes()).unwrap(); - let error = Announcer::spawn(&missing, GATEWAY, socket()).unwrap_err(); - let StartError::Launch(failure) = &error else { - panic!("{error}"); - }; - assert_eq!(failure.stage(), LaunchStage::Exec); - assert_eq!(failure.raw_os_error(), nix::libc::ENOENT); - assert!( - error - .to_string() - .starts_with("/sbin/nvx-ingress did not start: the exec launch stage failed: "), - "{error}" - ); -} - -#[test] -fn start_errors_name_their_cause() { - let denied = io::Error::from(Errno::EACCES); - for (error, expected) in [ - ( - StartError::Socket(io::Error::from(Errno::EMFILE)), - "the announcer's socket cannot be created: ", - ), - ( - StartError::Bind(denied), - "the announcer's socket cannot be bound to a privileged source port: ", - ), - ( - StartError::NoPrivilegedPort, - "no privileged source port, from 1023 down to 512, is free for the announcer's \ - socket", - ), - ( - StartError::Spawn(io::Error::from(Errno::EAGAIN)), - "/sbin/nvx-ingress cannot be started: ", - ), - ] { - assert!(error.to_string().starts_with(expected), "{error}"); - } -} diff --git a/nvx-rs/src/guest/ingress/mod.rs b/nvx-rs/src/guest/ingress/mod.rs index 0e3c96d04..63be46ab5 100644 --- a/nvx-rs/src/guest/ingress/mod.rs +++ b/nvx-rs/src/guest/ingress/mod.rs @@ -8,23 +8,26 @@ //! captured under a policy that publishes nothing, so its init started no //! announcer, with a policy of its own, which may publish the clone's //! listeners. After the process-free application succeeds, its caller starts -//! the announcer with [`Announcer::start`]: as root, it binds its UDP socket to a -//! privileged source port, and runs `nvx-ingress inherit GATEWAY_IPV4`, which -//! announces from that socket, its standard input, rather than from one of its -//! own. A failure to bind the socket, or to run the program, is the start's -//! own, before the clone is ready, and the caller watches the announcer that -//! started, and must end the clone once it ended: the gateway then keeps -//! publishing the listeners that it last announced, and learns of no later -//! change of them. +//! the announcer: as root, it binds a UDP socket to a privileged source port, +//! and runs `nvx-ingress inherit GATEWAY_IPV4`, which announces from that +//! socket, its standard input, rather than from one of its own. A failure to +//! bind the socket, or to run the program, is the start's own, before the +//! clone is ready, and the caller watches the announcer that started, and +//! must end the clone once it ended: the gateway then keeps publishing the +//! listeners that it last announced, and learns of no later change of them. //! //! A guest agent that cannot hold the announcer, such as the shell script //! that repairs the restore of a snapshot without a tier whose new policy //! publishes the guest's listeners, runs `nvx-ingress start GATEWAY_IPV4` -//! instead. It starts the announcer as [`Announcer::start`] does, and exits -//! once the announcer's socket is bound and its program runs, which leaves the -//! announcer to init, or to the nearest subreaper, as at a cold boot; it fails, -//! with status 1, when the announcer does not start, so that the agent learns -//! of the failure before the restore is acknowledged. +//! instead. It binds its socket to the highest free privileged source port, +//! from 1023 down to 512, and runs `/sbin/nvx-ingress inherit GATEWAY_IPV4` +//! with that socket as its standard input, the caller's standard output and +//! error, every signal's default disposition, and none blocked, in a session +//! of its own, without a controlling terminal. It exits once the program +//! runs, but waits for neither its first scan nor any announcement, which +//! leaves the announcer to init, or to the nearest subreaper, as at a cold +//! boot; it fails, with status 1, when the announcer does not start, so that +//! the agent learns of the failure before the restore is acknowledged. //! //! Every 100 ms it reads the kernel's socket tables. A TCP socket in the //! LISTEN state is a listener, and so is an unconnected UDP socket that @@ -58,18 +61,18 @@ use std::ffi::{CStr, CString}; use std::io::{self, Read, Write}; +use std::net::Ipv4Addr; +use std::os::fd::OwnedFd; use std::os::unix::ffi::OsStrExt; +use std::os::unix::process::CommandExt; use std::path::{Path, PathBuf}; -use std::process::ExitCode; +use std::process::{Command, ExitCode}; use super::parse; use super::status::{CommandError, run_main}; use super::sys::{self, Errno, FileDescriptor, Mode, OpenFlags}; -pub use announcer::{Announcer, PROGRAM, StartError}; - mod address; -mod announcer; mod collect; mod diag; mod encode; @@ -199,12 +202,12 @@ struct UdpSocket { /// Runs `nvx-ingress` with the process's arguments: `GATEWAY_IPV4`, the announcer, from a socket /// of its own; `inherit GATEWAY_IPV4`, the announcer, from the socket that it inherited as its /// standard input, which must be an IPv4 datagram socket bound to a privileged source port from -/// 512 to 1023; `start GATEWAY_IPV4`, which starts `inherit GATEWAY_IPV4` as -/// [`Announcer::start`] does and exits once it runs; or `test DIRECTORY`. Test mode exits with 0 -/// after the last snapshot, or with 1 when a snapshot fails. The announcer runs until it fails, -/// and exits with 1 when it cannot open or bind its socket, or take the one that it inherited, or -/// with 2 for a usage error. `start` exits with 0 once the announcer runs, or with 1 when it does -/// not start. +/// 512 to 1023; `start GATEWAY_IPV4`, which starts `inherit GATEWAY_IPV4` from a socket that it +/// binds, in a session of its own, and exits once it runs; or `test DIRECTORY`. Test mode exits +/// with 0 after the last snapshot, or with 1 when a snapshot fails. The announcer runs until it +/// fails, and exits with 1 when it cannot open or bind its socket, or take the one that it +/// inherited, or with 2 for a usage error. `start` exits with 0 once the announcer runs, or with +/// 1 when it does not start. #[doc(hidden)] pub fn main() -> ExitCode { run_main(|args| run(args, &mut sys::Stdout::new(), &mut io::stderr())) @@ -231,29 +234,66 @@ fn run(args: &[CString], out: &mut dyn Write, err: &mut dyn Write) -> Result<(), } } -/// Starts the announcer of the guest's listeners to `gateway` as [`Announcer::start`] does, and -/// returns once its socket is bound and its program runs, leaving it to run on its own: the -/// process exits then, which passes the announcer to init, or to the nearest subreaper. +/// Starts the announcer of the guest's listeners to `gateway`, as the [module documentation](self) +/// describes, and returns once its program runs, leaving it to run on its own: the process exits +/// then, which passes the announcer to init, or to the nearest subreaper. fn start(gateway: &CStr, err: &mut dyn Write) -> Result<(), CommandError> { let Some(gateway) = sender::parse_gateway(gateway) else { return Err(usage(err)); }; - match Announcer::start(gateway) { - Ok(announcer) => { - // Dropping the announcer would stop it, which only its owner may do. - std::mem::forget(announcer); - Ok(()) - } - Err(error) => { - sys::report( - err, - &[b"nvx-ingress: ", error.to_string().as_bytes(), b"\n"], - ); - Err(CommandError::Operation) + let started = bind_announcer_socket() + .and_then(|socket| launch(Path::new("/sbin/nvx-ingress"), gateway, socket)); + started.map_err(|problem| { + sys::report(err, &[b"nvx-ingress: ", problem.as_bytes(), b"\n"]); + CommandError::Operation + }) +} + +/// Creates the announcer's socket and binds it to the highest free privileged source port, as +/// [`start`] does, or fails with a description of the problem. +fn bind_announcer_socket() -> Result { + let socket = sys::UdpSocket::ipv4().map_err(|errno| { + let error = io::Error::from(errno); + format!("the announcer's socket cannot be created: {error}") + })?; + match bind_privileged(|port| socket.bind_any(port)) { + Ok(Some(_)) => Ok(socket), + Ok(None) => Err(String::from( + "no privileged source port, from 1023 down to 512, is free for the announcer's socket", + )), + Err(errno) => { + let error = io::Error::from(errno); + Err(format!( + "the announcer's socket cannot be bound to a privileged source port: {error}" + )) } } } +/// Spawns `program` as `nvx-ingress inherit GATEWAY_IPV4`, with `socket` as its standard input, +/// the caller's standard output and error, every signal's default disposition, and none blocked, +/// in a session of its own, and returns once the program runs, as [`start`] does once it bound +/// the socket. It first restores the default disposition of `SIGCHLD`, which the caller may have +/// inherited ignored. Fails with a description of the problem when the program does not start. +fn launch(program: &Path, gateway: Ipv4Addr, socket: sys::UdpSocket) -> Result<(), String> { + let mut command = Command::new(program); + command + .arg0("nvx-ingress") + .arg("inherit") + .arg(gateway.to_string()) + .stdin(OwnedFd::from(socket)); + sys::configure_announcer(&mut command); + // The spawn collects the status of a child that does not run the program, and panics when + // the kernel collected it already, as it collects every child's while `SIGCHLD` is ignored. + sys::reset_child_signal(); + match command.spawn() { + // The announcer outlives the caller, which leaves its status to init, or to the nearest + // subreaper. + Ok(_announcer) => Ok(()), + Err(error) => Err(format!("{} did not start: {error}", program.display())), + } +} + /// Prints the usage message and returns the usage error. fn usage(err: &mut dyn Write) -> CommandError { sys::report(err, &[b"usage: nvx-ingress [inherit|start] GATEWAY_IPV4\n"]); diff --git a/nvx-rs/src/guest/ingress/tests.rs b/nvx-rs/src/guest/ingress/tests.rs index f13b2930b..200d11aa7 100644 --- a/nvx-rs/src/guest/ingress/tests.rs +++ b/nvx-rs/src/guest/ingress/tests.rs @@ -1,7 +1,140 @@ -//! Checks of the shared file helpers. +//! Checks of the shared file helpers, and of the launch of the announcer that `start` runs. + +use std::os::unix::fs::PermissionsExt; + +use nix::sys::signal::SigSet; +use nix::sys::wait::WaitStatus; use super::*; +/// The gateway of the tests' announcers. +const GATEWAY: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 1); + +/// Writes an executable shell script in `directory` whose body is `script`, which stands for the +/// announcer's program, and returns its path. +fn announcer(directory: &Path, script: &str) -> PathBuf { + let path = directory.join("announcer"); + std::fs::write(&path, format!("#!/bin/sh\n{script}\n")).unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap(); + path +} + +/// Returns a socket that stands for the announcer's, bound to a port that needs no privilege. +fn socket() -> sys::UdpSocket { + let socket = sys::UdpSocket::ipv4().unwrap(); + socket.bind_any(0).unwrap(); + socket +} + +/// Launches the announcer `script` as `start` does, and waits until it exits with 0, which the +/// test process, that runs alone, collects as its only child. +fn launch_and_wait(directory: &Path, script: &str) { + launch(&announcer(directory, script), GATEWAY, socket()).unwrap(); + assert!(matches!( + nix::sys::wait::wait().unwrap(), + WaitStatus::Exited(_, 0) + )); +} + +#[test] +fn an_announcer_runs_its_program_with_its_socket_in_a_session_of_its_own() { + // The test collects the status of any child, so it runs in a process of its own. + if sys::tests::isolate_process_test( + "NVX_INGRESS_LAUNCH_TEST_CHILD", + "guest::ingress::tests::an_announcer_runs_its_program_with_its_socket_in_a_session_of_its_own", + ) { + return; + } + let directory = tempfile::tempdir().unwrap(); + let report = directory.path().join("report"); + let script = format!( + "read -r pid comm state ppid group session terminal rest '{}'", + report.display() + ); + launch_and_wait(directory.path(), &script); + let reported = std::fs::read_to_string(&report).unwrap(); + let lines: Vec<_> = reported.lines().collect(); + let ids: Vec<_> = lines[0].split(' ').collect(); + // It leads a session and a process group of its own, without a controlling terminal. + assert_eq!(ids[1..], [ids[0], ids[0], "0"], "{reported}"); + // The program takes the gateway after `inherit`, and the socket as its standard input. + assert_eq!(lines[1..], ["inherit", "10.0.0.1", "socket"], "{reported}"); +} + +#[test] +fn an_announcer_starts_with_every_signal_s_default_disposition_and_none_blocked() { + // Ignoring a signal changes the whole process, so the test runs in a process of its own. + if sys::tests::isolate_process_test( + "NVX_INGRESS_SIGNALS_TEST_CHILD", + "guest::ingress::tests::an_announcer_starts_with_every_signal_s_default_disposition_and_none_blocked", + ) { + return; + } + // The caller ignores every signal that it may, but `SIGCHLD`, which would have the kernel + // collect the announcer's status, and blocks every signal. + let ignored: Vec<_> = sys::Signal::iterator() + .filter(|signal| { + ![ + sys::Signal::SIGKILL, + sys::Signal::SIGSTOP, + sys::Signal::SIGCHLD, + ] + .contains(signal) + }) + .collect(); + sys::ignore_signals(&ignored); + SigSet::all().thread_block().unwrap(); + let directory = tempfile::tempdir().unwrap(); + let report = directory.path().join("report"); + let script = format!("exec cat /proc/self/status >'{}'", report.display()); + launch_and_wait(directory.path(), &script); + let status = std::fs::read_to_string(&report).unwrap(); + for name in ["SigIgn:", "SigBlk:"] { + let set = status + .lines() + .find_map(|line| line.strip_prefix(name)) + .unwrap_or_else(|| panic!("{name} is missing from {status}")); + assert_eq!(u64::from_str_radix(set.trim(), 16), Ok(0), "{status}"); + } +} + +#[test] +fn an_announcer_whose_program_does_not_run_does_not_start() { + let directory = tempfile::tempdir().unwrap(); + let missing = directory.path().join("missing"); + let problem = launch(&missing, GATEWAY, socket()).unwrap_err(); + let error = io::Error::from(Errno::ENOENT); + assert_eq!( + problem, + format!("{} did not start: {error}", missing.display()) + ); +} + +#[test] +fn an_announcer_does_not_start_without_its_program_when_its_caller_ignores_sigchld() { + // Ignoring `SIGCHLD` changes the whole process, so the test runs in a process of its own. + if sys::tests::isolate_process_test( + "NVX_INGRESS_IGNORED_SIGCHLD_TEST_CHILD", + "guest::ingress::tests::an_announcer_does_not_start_without_its_program_when_its_caller_ignores_sigchld", + ) { + return; + } + // A caller inherits an ignored `SIGCHLD` across `exec`, which has the kernel collect the + // child that does not run the program. + sys::tests::reap_children_automatically(true).unwrap(); + let directory = tempfile::tempdir().unwrap(); + let missing = directory.path().join("missing"); + let problem = launch(&missing, GATEWAY, socket()).unwrap_err(); + let error = io::Error::from(Errno::ENOENT); + assert_eq!( + problem, + format!("{} did not start: {error}", missing.display()) + ); + assert!(!sys::tests::has_child()); +} + /// Verifies that decimal files hold exactly one plain number, and their failure outcomes. #[test] fn decimal_files_hold_one_plain_number() { diff --git a/nvx-rs/src/guest/init.rs b/nvx-rs/src/guest/init.rs index cc203229d..00632ecc1 100644 --- a/nvx-rs/src/guest/init.rs +++ b/nvx-rs/src/guest/init.rs @@ -1033,7 +1033,7 @@ impl Runtime { } let gateway = gateway .ok_or_else(|| crate::Error::unsupported("published listeners require a gateway"))?; - let program = super::ingress::PROGRAM.to_owned(); + let program = c"/sbin/nvx-ingress".to_owned(); #[cfg(test)] let program = tests::restore::announcer_program(program); let arguments = [ diff --git a/nvx-rs/src/guest/mod.rs b/nvx-rs/src/guest/mod.rs index fdfe762a5..9b9fd61dd 100644 --- a/nvx-rs/src/guest/mod.rs +++ b/nvx-rs/src/guest/mod.rs @@ -34,9 +34,10 @@ //! Two of them also offer their steps as typed calls, which any guest init or agent runs in its //! own process and their programs run on: on x86-64, `time`'s `TimeAbi` runs the time ABI's //! cold-boot step, capture, and restore finish, and [`reseed`] reseeds the kernel's random number -//! generator with a restore's fresh entropy, which starts with the new [`GenerationId`]. And -//! [`ingress`] starts its program, the announcer of the guest's listeners, from a socket that it -//! binds for it. Activation reports the publication and gateway to its caller instead. The +//! generator with a restore's fresh entropy, which starts with the new [`GenerationId`]. The +//! program of the hidden `ingress` module, the announcer of the guest's listeners, offers no call: +//! activation reports the publication and gateway to its caller, which runs the program, or has +//! `nvx-ingress start` run it from a socket that it binds for it. The //! libraries need none of NVX's control protocol, managed agent, or workload identity, and NVX's //! managed agent opens its control terminal, applies its host mapping table, and supervises its //! workloads through them. On x86-64, the hidden `init` module holds @@ -122,6 +123,7 @@ pub mod exec; pub mod exit; #[doc(hidden)] pub mod gpt; +#[doc(hidden)] pub mod ingress; #[cfg(target_arch = "x86_64")] #[doc(hidden)] diff --git a/nvx-rs/src/guest/sys/mod.rs b/nvx-rs/src/guest/sys/mod.rs index 6e0d5e80d..196b2fe1e 100644 --- a/nvx-rs/src/guest/sys/mod.rs +++ b/nvx-rs/src/guest/sys/mod.rs @@ -149,7 +149,7 @@ pub(crate) use sock_diag::{ pub(crate) use socket::{UdpSocket, connect_datagram}; #[cfg(target_arch = "x86_64")] pub(crate) use spawn::reset_signals; -pub(crate) use spawn::{ChildSpec, PreparedChild, REPORT_LENGTH, enter_root}; +pub(crate) use spawn::{ChildSpec, PreparedChild, REPORT_LENGTH, configure_announcer, enter_root}; pub(crate) use terminal::{configure_raw, drain}; pub(crate) use timer::Timer; diff --git a/nvx-rs/src/guest/sys/socket.rs b/nvx-rs/src/guest/sys/socket.rs index dcff34fe1..c74139bdf 100644 --- a/nvx-rs/src/guest/sys/socket.rs +++ b/nvx-rs/src/guest/sys/socket.rs @@ -20,7 +20,7 @@ pub(crate) fn connect_datagram(peer: SocketAddrV4) -> Result for OwnedFd { fn from(socket: UdpSocket) -> Self { socket.0 diff --git a/nvx-rs/src/guest/sys/spawn.rs b/nvx-rs/src/guest/sys/spawn.rs index 7cf0590f0..4c0461287 100644 --- a/nvx-rs/src/guest/sys/spawn.rs +++ b/nvx-rs/src/guest/sys/spawn.rs @@ -27,6 +27,7 @@ use std::ffi::{CStr, CString}; use std::os::fd::RawFd; +use std::os::unix::process::CommandExt; use nix::fcntl::{FcntlArg, FdFlag}; use nix::libc::{c_char, c_int, c_uint, c_ulong}; @@ -1009,6 +1010,21 @@ pub(crate) fn reset_signals() -> Result<(), Errno> { signal_mask(nix::libc::SIG_SETMASK, 0) } +/// Makes `command`, the announcer's that `nvx-ingress start` runs, start its program in a session +/// of its own, without a controlling terminal, with every signal's default disposition, and none +/// blocked, as [`reset_signals`] leaves them, whatever the caller ignores or blocks. +pub(crate) fn configure_announcer(command: &mut std::process::Command) { + // SAFETY: the hook runs in the child between fork and exec, where it captures nothing, and + // makes only system calls, which neither allocate nor lock. + unsafe { + command.pre_exec(|| { + nix::unistd::setsid()?; + reset_signals()?; + Ok(()) + }); + } +} + /// Changes the calling thread's signal mask with `set`, a set of [`SIGNALS`] signals in which /// bit `n - 1` stands for signal `n`, as `how` says. fn signal_mask(how: c_int, set: u64) -> Result<(), Errno> { diff --git a/nvx-rs/tests/guest_tools.rs b/nvx-rs/tests/guest_tools.rs index a168d2618..589985bd1 100644 --- a/nvx-rs/tests/guest_tools.rs +++ b/nvx-rs/tests/guest_tools.rs @@ -962,7 +962,7 @@ fn ingress_start_fails_when_the_announcer_does_not_start() { [ "nvx-ingress: the announcer's socket cannot be bound to a privileged source port: ", "nvx-ingress: no privileged source port, from 1023 down to 512, is free", - "nvx-ingress: /sbin/nvx-ingress did not start: the exec launch stage failed: ", + "nvx-ingress: /sbin/nvx-ingress did not start: ", ] .iter() .any(|prefix| stderr.starts_with(prefix))