cli: keep a cold run's console when OpenVMM exits before the peer check

nvx-rs/src/host/cli/run.rs

@@ -697,11 +697,12 @@ fn connect_running(
     endpoint: &str,
 ) -> Result<Option<Box<dyn Transport>>> {
     let pid = launched.process.reference().pid;
-    connect_before_exit(
-        endpoint,
-        || platform::connect_endpoint(endpoint, pid, Duration::from_millis(250)),
-        |timeout| launched.wait_status(timeout),
-    )
+    // On Linux, OpenVMM may write its console output and exit before the console's peer check.
+    #[cfg(target_os = "linux")]
+    let connect = || platform::connect_child_endpoint(endpoint, pid);
+    #[cfg(not(target_os = "linux"))]
+    let connect = || platform::connect_endpoint(endpoint, pid, Duration::from_millis(250));
+    connect_before_exit(endpoint, connect, |timeout| launched.wait_status(timeout))
 }
 
 /// Connects to the verified endpoint, or yields to the owned child's confirmed exit.

nvx-rs/src/host/platform/linux/endpoint.rs

@@ -139,7 +139,7 @@ impl ReadyListener {
                 ));
             }
             if verify_original_peer {
-                check_original_peer(&connection)?;
+                check_original_peer(&connection, false)?;
             }
             connection.set_nonblocking(true)?;
             self.connection = Some(connection);
@@ -178,7 +178,10 @@ impl ReadyListener {
 }
 
 /// The kernel returns a descriptor of the socket's original peer, not a new lookup of its PID.
-fn check_original_peer(connection: &UnixStream) -> io::Result<()> {
+///
+/// A peer that has exited is refused, unless `accept_unreaped` accepts one that has not been
+/// reaped, which still holds its ID, as `connect_child_endpoint` requires.
+fn check_original_peer(connection: &UnixStream, accept_unreaped: bool) -> io::Result<()> {
     let peer = match getsockopt(connection, PeerPidfd) {
         Err(
             sys::Errno::ENOPROTOOPT
@@ -207,6 +210,9 @@ fn check_original_peer(connection: &UnixStream) -> io::Result<()> {
     }
     let events = descriptors[0].revents().unwrap_or(PollFlags::empty());
     if events.intersects(PollFlags::POLLIN | PollFlags::POLLHUP) {
+        if accept_unreaped && sys::PidFd::from(peer).holds_its_id()? {
+            return Ok(());
+        }
         return Err(io::Error::new(
             io::ErrorKind::PermissionDenied,
             "the original socket peer exited",
@@ -261,6 +267,29 @@ pub(crate) fn connect_endpoint(
     endpoint: &str,
     expected_pid: u32,
     _timeout: Duration,
+) -> io::Result<Box<dyn Transport>> {
+    connect_peer(endpoint, expected_pid, false)
+}
+
+/// Connects to the endpoint that the process `child` serves, as [`connect_endpoint`] does, for a
+/// caller that launched `child` and has not reaped it: a cold run, whose OpenVMM may write its
+/// boot console's output and exit before the run checks the console's peer.
+///
+/// It accepts an original socket peer that has exited but has not been reaped, and still refuses
+/// a reaped one. That is safe because the caller's own unreaped child keeps its PID, which no
+/// other process can reuse until the caller reaps it, so a peer that holds the child's PID is the
+/// child. Every other connection, as [`connect_endpoint`]'s, still refuses an exited peer.
+#[cfg(feature = "cli")]
+pub(crate) fn connect_child_endpoint(endpoint: &str, child: u32) -> io::Result<Box<dyn Transport>> {
+    connect_peer(endpoint, child, true)
+}
+
+/// Connects to the endpoint that the process `expected_pid` serves as the socket's original peer,
+/// which [`check_original_peer`] checks, with `accept_unreaped`.
+fn connect_peer(
+    endpoint: &str,
+    expected_pid: u32,
+    accept_unreaped: bool,
 ) -> io::Result<Box<dyn Transport>> {
     let stream = sys::connect_nonblocking(endpoint)?;
     let peer = sys::peer_pid(&stream)?;
@@ -272,7 +301,7 @@ pub(crate) fn connect_endpoint(
             ),
         ));
     }
-    check_original_peer(&stream)?;
+    check_original_peer(&stream, accept_unreaped)?;
     transport_from_handle(stream.into())
 }
 

nvx-rs/src/host/platform/linux/mod.rs

@@ -19,7 +19,7 @@ pub(crate) use self::endpoint::{
 };
 #[cfg(feature = "cli")]
 pub(crate) use self::endpoint::{
-    endpoint_server_pid_until, interrupt_transfers, transport_from_handle,
+    connect_child_endpoint, endpoint_server_pid_until, interrupt_transfers, transport_from_handle,
 };
 pub use self::identity::file_identity;
 pub(crate) use self::identity::{

nvx-rs/src/host/platform/linux/tests.rs

@@ -138,7 +138,72 @@ fn a_transferred_listener_cannot_authenticate_its_exited_creator() {
         println!("{verified} pid={pid}");
         return;
     }
+    run_with_transferred_listener(
+        "host::platform::linux::tests::a_transferred_listener_cannot_authenticate_its_exited_creator",
+        true,
+        verified,
+    );
+}
+
+#[cfg(feature = "cli")]
+#[test]
+fn a_child_endpoint_accepts_its_exited_child_until_the_child_is_reaped() {
+    use std::io::Write;
+    use std::os::fd::AsFd;
+    use std::os::unix::net::UnixListener;
+
+    use nix::sys::wait::{Id, WaitPidFlag, waitid, waitpid};
+    use nix::unistd::Pid;
+
+    let verified = "NVX_UNREAPED_CHILD_PEER_ACCEPTED";
+    if let Some(pid) = std::env::var_os("NVX_TEST_TRANSFERRED_CONTROL_LISTENER") {
+        let pid: u32 = pid.to_str().unwrap().parse().unwrap();
+        let endpoint = std::env::var("NVX_TEST_TRANSFERRED_CONTROL_ENDPOINT").unwrap();
+        let listener = UnixListener::from(std::io::stdin().as_fd().try_clone_to_owned().unwrap());
+        // The listener's creator is this process's child: it exits, and only this process reaps it.
+        let creator = Pid::from_raw(i32::try_from(pid).unwrap());
+        waitid(
+            Id::Pid(creator),
+            WaitPidFlag::WEXITED | WaitPidFlag::WNOWAIT,
+        )
+        .unwrap();
+        let error = connect_endpoint(&endpoint, pid, Duration::from_secs(2))
+            .err()
+            .expect("other connections must still refuse an exited peer");
+        assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied);
+        assert!(error.to_string().contains("original socket peer exited"));
+        drop(listener.accept().unwrap());
+        let mut transport = connect_child_endpoint(&endpoint, pid).unwrap();
+        let (mut peer, _) = listener.accept().unwrap();
+        peer.write_all(b"output").unwrap();
+        let mut output = [0; 6];
+        let count = transport
+            .read(&mut output, Some(Duration::from_secs(2)))
+            .unwrap();
+        assert_eq!(&output[..count], b"output");
+        waitpid(creator, None).unwrap();
+        let error = connect_child_endpoint(&endpoint, pid)
+            .err()
+            .expect("a reaped peer must not authenticate");
+        assert_eq!(error.kind(), std::io::ErrorKind::PermissionDenied);
+        assert!(error.to_string().contains("original socket peer exited"));
+        println!("{verified} pid={pid}");
+        return;
+    }
+    run_with_transferred_listener(
+        "host::platform::linux::tests::a_child_endpoint_accepts_its_exited_child_until_the_child_is_reaped",
+        false,
+        verified,
+    );
+}
 
+/// Runs this binary's test `test` in a Python process that a child of its own, the creator, gave
+/// a listener at a new socket before it exited, and checks that the test passed and printed
+/// `verified`. The test finds the listener on its standard input, the creator's PID in
+/// `NVX_TEST_TRANSFERRED_CONTROL_LISTENER`, and the socket's path in
+/// `NVX_TEST_TRANSFERRED_CONTROL_ENDPOINT`. The process reaps the creator first if `reap`, and
+/// leaves it to the test otherwise.
+fn run_with_transferred_listener(test: &str, reap: bool, verified: &str) {
     let directory = tempfile::tempdir().unwrap();
     let path = directory.path().join("control.sock");
     let mut command = Command::new("python3");
@@ -163,7 +228,8 @@ assert messages[0][:2] == (socket.SOL_SOCKET, socket.SCM_RIGHTS)
 descriptors = array.array("i")
 descriptors.frombytes(messages[0][2])
 assert len(descriptors) == 1
-assert os.waitpid(creator, 0) == (creator, 0)
+if sys.argv[4] == "reap":
+    assert os.waitpid(creator, 0) == (creator, 0)
 os.dup2(descriptors[0], 0)
 if descriptors[0] != 0:
     os.close(descriptors[0])
@@ -174,10 +240,9 @@ os.execv(sys.argv[2], [sys.argv[2], "--exact", sys.argv[3], "--nocapture", "--te
         ])
         .arg(&path)
         .arg(std::env::current_exe().unwrap())
-        .arg(
-            "host::platform::linux::tests::a_transferred_listener_cannot_authenticate_its_exited_creator",
-        )
-        .env(endpoint, &path)
+        .arg(test)
+        .arg(if reap { "reap" } else { "keep" })
+        .env("NVX_TEST_TRANSFERRED_CONTROL_ENDPOINT", &path)
         .stdin(Stdio::null())
         .stdout(Stdio::piped())
         .stderr(Stdio::piped());

nvx-rs/src/host/sys/process.rs

@@ -102,17 +102,42 @@ impl PidFd {
     ///
     /// Fails with `ESRCH` if the process has exited.
     pub(crate) fn kill(&self) -> io::Result<()> {
+        self.send_signal(Signal::SIGKILL as libc::c_int)?;
+        Ok(())
+    }
+
+    /// Returns whether the process still holds its ID: it runs, or it has exited and has not
+    /// been reaped, as signal 0 shows without signaling it. No other process can take the ID
+    /// before the process is reaped.
+    pub(crate) fn holds_its_id(&self) -> io::Result<bool> {
+        match self.send_signal(0) {
+            // Another user's process refuses the signal, but it holds its ID.
+            Ok(()) | Err(Errno::EPERM) => Ok(true),
+            Err(Errno::ESRCH) => Ok(false),
+            Err(error) => Err(error.into()),
+        }
+    }
+
+    /// Sends `signal` to the process, or with 0, only checks that it could.
+    fn send_signal(&self, signal: libc::c_int) -> Result<(), Errno> {
         // SAFETY: the descriptor is a valid pidfd, and a null siginfo with no flags is allowed.
         Errno::result(unsafe {
             libc::syscall(
                 libc::SYS_pidfd_send_signal,
                 self.0.as_raw_fd(),
-                Signal::SIGKILL as libc::c_int,
+                signal,
                 ptr::null::<libc::siginfo_t>(),
                 0,
             )
-        })?;
-        Ok(())
+        })
+        .map(drop)
+    }
+}
+
+impl From<OwnedFd> for PidFd {
+    /// Takes `descriptor`, which must be a pidfd, such as `SO_PEERPIDFD` returns.
+    fn from(descriptor: OwnedFd) -> Self {
+        Self(descriptor)
     }
 }