From 3d86f30dd30aa71b210edef9007bcaed4b383415 Mon Sep 17 00:00:00 2001
From: Valentin Haudiquet <valentin.haudiquet@canonical.com>
Date: Fri, 30 Oct 2026 03:45:23 +0100
Subject: [PATCH] fix(supervise): kill and reap the machine when a run errors
 after the fork

A parent-side failure after the fork (a console stat or read that
went wrong, a wait that could not complete) used to return and leave
the VM booting unattended. PDEATHSIG only fires when the caller's
process exits; a library consumer that catches the error keeps
running, and the scratch rootfs goes away under the machine.
kill_machine is best effort: a machine that already exited reaps
through ECHILD. Also drops a duplicated fork-error block left behind
by the supervise split.
---
 crates/ecrou/src/supervise.rs | 57 ++++++++++++++++++++++++++++++-----
 crates/ecrou/src/vm.rs        | 13 ++++++--
 2 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/crates/ecrou/src/supervise.rs b/crates/ecrou/src/supervise.rs
index c72b2e2..d4253dc 100644
--- a/crates/ecrou/src/supervise.rs
+++ b/crates/ecrou/src/supervise.rs
@@ -67,13 +67,6 @@ pub(crate) fn spawn_machine(
         }
         bail("start_enter returned".to_string(), 125);
     }
-    if child < 0 {
-        return Err(Error::Other(
-            anyhow::Error::from(std::io::Error::last_os_error())
-                .context("forking the machine process"),
-        ));
-    }
-
     if child < 0 {
         return Err(Error::Other(
             anyhow::Error::from(std::io::Error::last_os_error())
@@ -189,6 +182,28 @@ pub(crate) fn stream_console(
     Ok(())
 }
 
+/// Kill the machine and collect it. Any parent-side failure after the
+/// fork must land here: a library caller keeps running after a run
+/// error, and the kill contract (PDEATHSIG) only fires when their
+/// process exits. Best effort by construction — a machine that already
+/// exited reaps through ECHILD and the call is a no-op.
+pub(crate) fn kill_machine(child: libc::pid_t) {
+    // Safety: the pid is our own child; SIGKILL is uncatchable and has
+    // no handler side effects to mind.
+    unsafe { libc::kill(child, libc::SIGKILL) };
+    let mut status: libc::c_int = 0;
+    // Safety: waitpid on our own child; the pointer targets valid
+    // storage.
+    loop {
+        let r = unsafe { libc::waitpid(child, &mut status, 0) };
+        // EINTR: interrupted, not ended — collect again.
+        if r == -1 && std::io::Error::last_os_error().kind() == std::io::ErrorKind::Interrupted {
+            continue;
+        }
+        break;
+    }
+}
+
 /// Collect the machine: its exit status is the payload's exit code (the
 /// guest init blob reports it and the VMM exits the child with it).
 pub(crate) fn wait_machine(
@@ -284,7 +299,11 @@ mod tests {
     #[test]
     fn a_signaled_machine_is_an_engine_failure() {
         let dir = tempfile::tempdir().unwrap();
-        for raw in [signaled(libc::SIGSEGV), signaled(libc::SIGKILL), signaled(libc::SIGTERM)] {
+        for raw in [
+            signaled(libc::SIGSEGV),
+            signaled(libc::SIGKILL),
+            signaled(libc::SIGTERM),
+        ] {
             let err = decode_status(raw, &dir.path().join("absent")).unwrap_err();
             assert!(
                 err.to_string().contains("engine or hypervisor failure"),
@@ -328,4 +347,26 @@ mod tests {
         std::fs::write(&exit_file, "not a number").unwrap();
         assert_eq!(decode_status(exited(42), &exit_file).unwrap(), 42);
     }
+
+    #[test]
+    fn kill_machine_reaps_a_living_child() {
+        // Safety: the child body only pauses and _exits; nothing
+        // allocates after the fork.
+        let child = unsafe { libc::fork() };
+        assert!(child >= 0, "fork: {}", std::io::Error::last_os_error());
+        if child == 0 {
+            loop {
+                unsafe { libc::pause() };
+            }
+        }
+        kill_machine(child);
+        // The child is reaped: waitpid has nothing left to report.
+        let mut status: libc::c_int = 0;
+        let r = unsafe { libc::waitpid(child, &mut status, libc::WNOHANG) };
+        assert_eq!(r, -1);
+        assert_eq!(
+            std::io::Error::last_os_error().raw_os_error(),
+            Some(libc::ECHILD)
+        );
+    }
 }
diff --git a/crates/ecrou/src/vm.rs b/crates/ecrou/src/vm.rs
index e727984..1940a01 100644
--- a/crates/ecrou/src/vm.rs
+++ b/crates/ecrou/src/vm.rs
@@ -286,9 +286,18 @@ pub fn run_prepared(rootfs: &PreparedRootfs, options: &RunOptions) -> Result<i32
         )
     } else {
         let reaped = std::sync::atomic::AtomicI32::new(-1);
-        crate::supervise::stream_console(&console_path, child, &reaped)?;
-        crate::supervise::wait_machine(child, &reaped, &exit_file_host)
+        crate::supervise::stream_console(&console_path, child, &reaped)
+            .and_then(|()| crate::supervise::wait_machine(child, &reaped, &exit_file_host))
     };
+    // A parent-side failure (a console stat or read that went wrong, a
+    // wait that could not complete) must not leave the machine booting
+    // unattended: a library caller keeps running after a run error,
+    // and the kill contract only reaches through their process exit.
+    // On a run that failed after the child already exited this is a
+    // no-op reaping.
+    if result.is_err() {
+        crate::supervise::kill_machine(child);
+    }
     tty_restore.restore();
     result
 }