diff --git a/CHANGELOG.md b/CHANGELOG.md index ae048e6..e39bce2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,60 @@ Versioning](https://semver.org/spec/v2.0.0.html) once it reaches chain-assembly base copy and the bake's rootfs baseline clone streamed a full copy even on btrfs, XFS, and ZFS 2.2+. Found by @jrimmer in #321. +### Per-child rootfs backings: concurrent children can no longer corrupt each other + +A snapshot's rootfs is one ext4 and Firecracker reopens that path verbatim for +every child, so children of a tag shared a filesystem. With the drive opened +read-write — how every tag was baked — two concurrent children were two guest +kernels writing one filesystem with no coordinator: package files picking up +other files' bytes, `EBADMSG` on `/var/lib/dpkg` entries, damage that reads as a +random build failure rather than a sandbox error. + +Each child now gets its own reflink clone of the tag's rootfs and is re-pointed +at it with `PATCH /drives/rootfs` before it runs. The ordering is load-bearing: +the child is loaded `resume_vm: false`, re-pointed, and only then resumed, so no +boot-time write (journal replay, `/var/log`, agent startup) can reach the shared +base first. Measured on v1.17.0, the base stays byte-identical through boot +while the backing changes. + +**Fail closed.** After the PATCH, forkd reads `GET /vm/config` back and resumes +the child only if Firecracker reports the child's own backing as its rootfs. A +child whose rootfs cannot be re-pointed and verified is not resumed — it would +run against the shared base — so the failure surfaces instead of drifting into +corruption. Backings live in the child's work dir and are reclaimed with it. + +**No Firecracker upgrade is required.** Re-pointing a drive on a restored VM +works on every stock release measured — v1.5.1, v1.7.0, v1.10.1, v1.12.1, +v1.14.1 and v1.17.0 (aarch64): the guest's writes move to the new file and +`/vm/config` reports it. So existing snapshots keep loading and nothing needs +re-baking. + +**Read-only rootfs is shared, not copied.** Whether the drive is writable is +read from the restored vmstate itself (`/vm/config`), so a read-only (squashfs) +tag — including one baked before this change — costs no copy per child. + +**Branches and workspace suspends own their rootfs.** A snapshot of a restored +child used to record the child's backing, a file in a work dir that dies with +the child: the new tag failed to load (`Failed to restore from snapshot … No +such file`) once its source was gone, and until then every child restored from +it wrote the source's live disk. The paused source's drive is now pointed at a +clone the new tag owns for the length of the snapshot, recorded as the tag's +`rootfs`, and pointed home before the source resumes; a source whose drive +cannot be pointed home is killed rather than resumed onto the tag's disk. This +covers full, diff and live BRANCH and `POST /v1/workspaces/:name/suspend`. + +Constraints worth knowing: a reflink clone is free only where the filesystem +supports it (XFS/btrfs, or ZFS 2.2+ block cloning); elsewhere it is a full copy +per child, made in parallel in the children's restore threads. A live BRANCH +makes its clone inside its pause window, so on a filesystem without reflink the +pause includes a full rootfs copy. Snapshots with no recorded rootfs keep the +previous single-call behaviour. + +Restore work dirs, and so the backings in them, live under the controller's +temp dir. On a host where `/tmp` is tmpfs, set `TMPDIR` to a directory on the +snapshot root's filesystem, so backings are not charged to RAM and the clone +has both ends on one filesystem. + ### Upgrade note: legacy sandbox rows block startup The controller now persists a boot identity (start time + boot id) for every diff --git a/crates/forkd-cli/src/main.rs b/crates/forkd-cli/src/main.rs index a3434a5..cc9aab6 100644 --- a/crates/forkd-cli/src/main.rs +++ b/crates/forkd-cli/src/main.rs @@ -4830,6 +4830,14 @@ mod tests { .unwrap(); } + /// Pids the sweep tests park backups under, standing for bakes that have + /// exited. They must never name a live process, or the sweep leaves the + /// backup alone as a concurrent bake's: small made-up pids like 1000 are + /// often live on a CI runner. The kernel never hands out a pid above + /// `PID_MAX_LIMIT` (2^22 on 64-bit), so these cannot be. + const DEAD_PID_A: u32 = 4_194_305; + const DEAD_PID_B: u32 = 4_194_306; + /// Review #295 approval follow-up (2026-09-02): a `kill -9` between /// the preserve-rename and the publish strands the backup under the /// dead run's pid, leaving the tag with no `rootfs.ext4`. The sweep @@ -4839,8 +4847,12 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let snap_dir = tmp.path().join("tag"); std::fs::create_dir_all(&snap_dir).unwrap(); - let older = tmp.path().join("tag.rootfs.ext4.prev-1000"); - let newer = tmp.path().join("tag.rootfs.ext4.prev-2000"); + let older = tmp + .path() + .join(format!("tag.rootfs.ext4.prev-{DEAD_PID_A}")); + let newer = tmp + .path() + .join(format!("tag.rootfs.ext4.prev-{DEAD_PID_B}")); write_with_mtime(&older, b"OLD-ROOTFS", 1_700_000_000); // Ordering is by park time (ctime), so park the newer one later. std::thread::sleep(std::time::Duration::from_millis(20)); @@ -4871,8 +4883,12 @@ mod tests { std::fs::create_dir_all(&snap_dir).unwrap(); let live = snap_dir.join("rootfs.ext4"); std::fs::write(&live, b"CURRENT-GOOD").unwrap(); - let a = tmp.path().join("tag.rootfs.ext4.prev-1000"); - let b = tmp.path().join("tag.rootfs.ext4.prev-2000"); + let a = tmp + .path() + .join(format!("tag.rootfs.ext4.prev-{DEAD_PID_A}")); + let b = tmp + .path() + .join(format!("tag.rootfs.ext4.prev-{DEAD_PID_B}")); write_with_mtime(&a, b"STALE-A", 1_700_000_000); write_with_mtime(&b, b"STALE-B", 1_700_000_900); let now = std::time::SystemTime::now() @@ -4905,7 +4921,9 @@ mod tests { let snap_dir = tmp.path().join("tag"); std::fs::create_dir_all(&snap_dir).unwrap(); write_with_mtime(&snap_dir.join("snapshot.json"), b"{}", 1_700_000_000); - let backup = tmp.path().join("tag.rootfs.ext4.prev-1000"); + let backup = tmp + .path() + .join(format!("tag.rootfs.ext4.prev-{DEAD_PID_A}")); write_with_mtime(&backup, b"LAST-PUBLISHED", 1_700_000_000); std::fs::write(snap_dir.join("rootfs.ext4"), b"UNPUBLISHED-DIRTY-CLONE").unwrap(); @@ -4959,11 +4977,11 @@ mod tests { let held = tmp .path() .join("snapshots") - .join("tag.rootfs.ext4.prev-1000"); + .join(format!("tag.rootfs.ext4.prev-{DEAD_PID_A}")); let other = tmp .path() .join("snapshots") - .join("other.rootfs.ext4.prev-1000"); + .join(format!("other.rootfs.ext4.prev-{DEAD_PID_A}")); std::fs::write(&held, b"MINE").unwrap(); std::fs::write(&other, b"OTHER-TAG").unwrap(); diff --git a/crates/forkd-controller/src/http.rs b/crates/forkd-controller/src/http.rs index 1b55999..fc05929 100644 --- a/crates/forkd-controller/src/http.rs +++ b/crates/forkd-controller/src/http.rs @@ -482,11 +482,22 @@ async fn create_snapshot( // Volumes via daemon snapshot API will land in a follow-up commit; // for now snapshots created through the daemon are volume-less. // Use the CLI's `forkd snapshot --volume` for tag-shared caches. - let snap = vm.snapshot_to( + let mut snap = vm.snapshot_to( snap_dir_for_task.join("vmstate"), snap_dir_for_task.join("memory.bin"), Vec::new(), )?; + // Record the rootfs this parent booted from. Restore gives every child + // its own reflink backing cloned from this path, and with no path there + // is nothing to clone, so children fall back to sharing one file — the + // daemon's snapshots were the last ones doing that. Branches and + // workspace suspends record a clone of their own (see + // `Vm::detour_rootfs_into`). + snap.rootfs = Some( + cfg.rootfs + .canonicalize() + .unwrap_or_else(|_| cfg.rootfs.clone()), + ); // Persist Snapshot metadata so subsequent forks read back the same // (possibly volume-bearing) snapshot description. let meta = serde_json::to_vec_pretty(&snap)?; @@ -1359,7 +1370,10 @@ async fn create_sandbox( // ran concurrently (e.g. two branches of the same source). Mix the // netns offset in so concurrent batches get distinct work_dirs. let work_dir = - std::env::temp_dir().join(format!("forkd-daemon-{tag_for_work_dir}-o{netns_offset}")); + crate::state::work_dir_root().join(format!( + "{}{tag_for_work_dir}-o{netns_offset}", + crate::state::WORK_DIR_PREFIX + )); // v0.5 chain assembly. If `chain` is set, hash-verify each // parent against the recorded parent_content_hash (this is // the foot-gun guard committed to in the design doc), then @@ -1975,7 +1989,7 @@ async fn branch_sandbox( // our mmap. #[cfg(target_os = "linux")] if live_mode { - let (live_pause_ms, worker) = run_live_branch_setup( + let (live_pause_ms, worker, branch_rootfs) = run_live_branch_setup( &vm, &snap_dir_for_task, &dst_mem, @@ -2007,7 +2021,7 @@ async fn branch_sandbox( volumes: source_volumes, parent_tag: None, parent_content_hash: None, - rootfs: None, + rootfs: branch_rootfs, }); } #[cfg(not(target_os = "linux"))] @@ -2038,6 +2052,15 @@ async fn branch_sandbox( let pause_start = std::time::Instant::now(); vm.pause()?; + // A restored child's rootfs is its own backing, which dies + // with it. Point the drive at a copy this tag owns for the + // snapshot, so the vmstate records a durable path, and back + // before the source resumes (see `Vm::detour_rootfs_into`). + let rootfs_detour = vm + .detour_rootfs_into(&snap_dir_for_task) + .context("give the branch its own rootfs")?; + let branch_rootfs = rootfs_detour.as_ref().map(|d| d.path().to_path_buf()); + // Phase 1a measurement hook: take a Diff snapshot first // (captures pages dirtied since restore; clears the dirty // bitmap). Discarded after metrics. The subsequent Full @@ -2093,6 +2116,9 @@ async fn branch_sandbox( diff_snap.physical_size_bytes, diff_snap.logical_size_bytes, )); + if let Some(detour) = rootfs_detour { + detour.finish()?; + } let resume_result = vm.resume(); pause_ms = Some(pause_start.elapsed().as_millis() as u64); @@ -2138,16 +2164,20 @@ async fn branch_sandbox( volumes: diff_snap.volumes, parent_tag: None, parent_content_hash: None, - rootfs: None, + rootfs: branch_rootfs, } } else { - let snap = vm.snapshot_to( + let mut snap = vm.snapshot_to( snap_dir_for_task.join("vmstate"), snap_dir_for_task.join("memory.bin"), // Inherit volumes from the source snapshot so grandchildren // re-attach the same persistent disks the source had. source_volumes, )?; + if let Some(detour) = rootfs_detour { + detour.finish()?; + } + snap.rootfs = branch_rootfs; // resume() may fail after a successful snapshot. The snapshot file // is intact and usable; the source sandbox is in an unknown state // (most likely still paused). We log and continue rather than @@ -2607,7 +2637,7 @@ fn run_live_branch_setup( dst_mem: &std::path::Path, source_volumes: Vec, id_for_log: &str, -) -> anyhow::Result<(u64, LiveBranchWorker)> { +) -> anyhow::Result<(u64, LiveBranchWorker, Option)> { use std::os::fd::AsRawFd; let memfd = vm.memfd_handle().ok_or_else(|| { @@ -2699,12 +2729,23 @@ fn run_live_branch_setup( // resume. PauseGuard's Drop resumes on early return. let pause_start = std::time::Instant::now(); let pause_guard = vm.pause_guard()?; + // Declared after the pause guard so an early return drops it first: the + // drive goes home before the guard resumes the source. The clone runs + // inside the pause window — near-free with reflink, a full rootfs copy + // without it — because the copy has to match the vmstate's instant. + let rootfs_detour = vm + .detour_rootfs_into(snap_dir) + .context("give the branch its own rootfs")?; + let branch_rootfs = rootfs_detour.as_ref().map(|d| d.path().to_path_buf()); vm.snapshot_vmstate_only( snap_dir.join("vmstate"), dst_mem.to_path_buf(), source_volumes, ) .context("vmstate-only snapshot during live BRANCH")?; + if let Some(detour) = rootfs_detour { + detour.finish()?; + } pause_guard.resume().context("resume after vmstate dump")?; let pause_ms = pause_start.elapsed().as_millis() as u64; @@ -2720,6 +2761,7 @@ fn run_live_branch_setup( mmap_guard, wp_branch, }, + branch_rootfs, )) } @@ -3216,6 +3258,12 @@ async fn suspend_workspace( None }; vm.pause()?; + // As in `branch_sandbox`: the state snapshot must own its rootfs, + // not record the live child's backing, which dies with the child. + let rootfs_detour = vm + .detour_rootfs_into(&snap_dir_for_task) + .context("give the workspace state its own rootfs")?; + let state_rootfs = rootfs_detour.as_ref().map(|d| d.path().to_path_buf()); let snap = if diff_mode { let diff_path = std::env::temp_dir().join(format!( "forkd-ws-diff-{}-{}.bin", @@ -3227,6 +3275,9 @@ async fn suspend_workspace( diff_path.clone(), Vec::new(), )?; + if let Some(detour) = rootfs_detour { + detour.finish()?; + } vm.resume()?; pause_ms = Some(pause_start.elapsed().as_millis() as u64); if let Some(h) = cp_handle { @@ -3241,14 +3292,18 @@ async fn suspend_workspace( volumes: diff_snap.volumes, parent_tag: None, parent_content_hash: None, - rootfs: None, + rootfs: state_rootfs, } } else { - let snap = vm.snapshot_to( + let mut snap = vm.snapshot_to( snap_dir_for_task.join("vmstate"), snap_dir_for_task.join("memory.bin"), Vec::new(), )?; + if let Some(detour) = rootfs_detour { + detour.finish()?; + } + snap.rootfs = state_rootfs; vm.resume()?; pause_ms = Some(pause_start.elapsed().as_millis() as u64); snap diff --git a/crates/forkd-controller/src/state.rs b/crates/forkd-controller/src/state.rs index b1997ac..10a35f4 100644 --- a/crates/forkd-controller/src/state.rs +++ b/crates/forkd-controller/src/state.rs @@ -39,6 +39,20 @@ pub struct Registry { path: PathBuf, } +/// The directory a restore's work dir is named for, under [`work_dir_root`]. +/// One helper for both ends: the spawn path creates these, and the startup +/// sweep reclaims the rootfs backings stranded in them, so the two must agree +/// on where to look even when `TMPDIR` changes. +pub(crate) const WORK_DIR_PREFIX: &str = "forkd-daemon-"; + +/// Where restore work dirs live: the controller's temp dir. A child's rootfs +/// backing lives in its work dir, so on a host where `/tmp` is tmpfs, point +/// `TMPDIR` at the snapshot root's filesystem — both so the backings are not +/// charged to RAM and so the reflink clone has both ends on one filesystem. +pub(crate) fn work_dir_root() -> PathBuf { + std::env::temp_dir() +} + impl Registry { pub fn load_or_init(path: impl Into) -> Result { let path: PathBuf = path.into(); @@ -322,7 +336,128 @@ impl Registry { /// If some kills failed, those entries remain in the registry and /// the allocator may still collide with them — the caller should /// log the failure count and the operator should investigate. + /// The work dir a Firecracker process was started with, read from its own + /// `--api-sock` argument. `None` when the process is already gone or the + /// argument is unreadable — callers skip the reclaim rather than guess a + /// path, since removing the wrong directory would take a live VM's sockets + /// with it. + fn child_work_dir_from_cmdline(pid: u32) -> Option { + let raw = std::fs::read(format!("/proc/{pid}/cmdline")).ok()?; + let args: Vec = raw + .split(|b| *b == 0) + .filter(|s| !s.is_empty()) + .map(|s| String::from_utf8_lossy(s).into_owned()) + .collect(); + let sock = args + .iter() + .position(|a| a == "--api-sock") + .and_then(|i| args.get(i + 1))?; + Some(std::path::Path::new(sock).parent()?.to_path_buf()) + } + + /// Is `name` a restored child's rootfs backing (`child-.`)? + /// + /// The same dir also holds `child-.sock` and `child-.console`, and the + /// stem is the tag's rootfs filename — not always `rootfs.ext4` — so match + /// on shape rather than a fixed suffix. + fn is_child_backing(name: &str) -> bool { + let Some(rest) = name.strip_prefix("child-") else { + return false; + }; + let Some((index, ext)) = rest.split_once('.') else { + return false; + }; + !index.is_empty() + && index.chars().all(|c| c.is_ascii_digit()) + && !ext.is_empty() + && ext != "sock" + && ext != "console" + } + + /// Reclaim the rootfs backings a reaped child left behind. + /// + /// A child killed while the controller is up has its backing removed by + /// `Vm::drop`. This covers the other order — controller crash, then startup + /// reap — where nothing else collects them: the watchdog leaves the dir + /// alone because the socket is present, and the socket is present because + /// the orphan held it until now. Only backings in a directory belonging to + /// a process this reap has just confirmed dead are touched, so a live VM + /// cannot be caught; the dir itself is left to the watchdog's own sweep. + fn reclaim_child_backings(work_dir: &std::path::Path) { + let Ok(entries) = std::fs::read_dir(work_dir) else { + return; + }; + for entry in entries.flatten() { + let name = entry.file_name().to_string_lossy().into_owned(); + if Self::is_child_backing(&name) { + let _ = std::fs::remove_file(entry.path()); + } + } + } + + /// Remove backings left in directories no live Firecracker is using. + /// + /// Covers what the per-kill reclaim cannot: a row retired *without* a kill + /// because its process is already gone — the ordinary shape of a controller + /// restart, where systemd kills the unit's cgroup and takes the children with + /// it. Nothing reaches `Vm::drop` in that ordering, so nothing removes the + /// backing, and the watchdog leaves the directory alone because the socket + /// file is still there. + /// + /// The gate is process ownership, not file presence. A directory is skipped + /// while any live Firecracker's `--api-sock` points inside it, so a running + /// VM's backing cannot be caught; and if any live Firecracker cannot be read, + /// nothing is removed at all, because ownership can no longer be established. + /// Leaking a backing costs disk. Deleting a live VM's backing destroys that + /// VM's disk mid-job, so every ambiguity resolves toward keeping it. + fn sweep_stranded_backings() { + let mut owned = std::collections::HashSet::new(); + let Ok(pids) = std::fs::read_dir("/proc") else { + return; + }; + for entry in pids.flatten() { + let Ok(pid) = entry.file_name().to_string_lossy().parse::() else { + continue; + }; + let Ok(raw) = std::fs::read(format!("/proc/{pid}/cmdline")) else { + continue; + }; + if !raw.windows(12).any(|w| w == b"firecracker\0") { + continue; + } + match Self::child_work_dir_from_cmdline(pid) { + Some(dir) => { + owned.insert(dir); + } + // A live Firecracker whose directory we cannot read means we + // cannot say what is owned, so nothing is swept this time. + None => return, + } + } + let Ok(entries) = std::fs::read_dir(work_dir_root()) else { + return; + }; + for entry in entries.flatten() { + if !entry.path().is_dir() { + continue; + } + let name = entry.file_name().to_string_lossy().into_owned(); + if !name.starts_with(WORK_DIR_PREFIX) { + continue; + } + let dir = entry.path(); + if !owned.contains(&dir) { + Self::reclaim_child_backings(&dir); + } + } + } + pub(crate) fn kill_orphans(&self) -> Result { + // Sweep before the reap: in the restart case the children are already + // dead, so their rows are retired without a kill and `Vm::drop` never + // runs for them. Their directories are unowned by now, which is exactly + // what the sweep requires. + Self::sweep_stranded_backings(); // One collected orphan: (sandbox id, pid, recorded start time, // recorded boot id). type OrphanRow = (String, u32, Option, Option); @@ -471,6 +606,10 @@ impl Registry { pid = pid, "killing orphaned Firecracker process on startup (pidfd signal, identity verified)" ); + // Read the child's work dir now: once the process is gone + // there is nothing left in /proc to read it from, and the + // rootfs backing inside would be stranded for good. + let orphan_work_dir = Self::child_work_dir_from_cmdline(pid); match pidfd_send_kill(pidfd) { Ok(()) => { // Wait for the process to actually exit (bounded). @@ -478,6 +617,9 @@ impl Registry { // hold netns/tap resources past the kill return. if wait_for_death(pid, std::time::Duration::from_secs(5)) { self.inner.lock().sandboxes.remove(&id); + if let Some(dir) = &orphan_work_dir { + Self::reclaim_child_backings(dir); + } killed += 1; } else { tracing::error!( @@ -1138,6 +1280,53 @@ mod tests { ); } + /// The predicate decides what a startup reap deletes, and the work dir holds + /// sockets and consoles under the same `child-.` shape. The stem is the + /// tag's rootfs filename, so it is not always `rootfs.ext4`. + #[test] + fn child_backing_predicate_matches_backings_only() { + assert!(Registry::is_child_backing("child-1.rootfs.ext4")); + assert!(Registry::is_child_backing("child-12.python-3-12-slim.ext4")); + assert!( + !Registry::is_child_backing("child-1.sock"), + "sockets share the child-. shape and must survive the sweep" + ); + assert!( + !Registry::is_child_backing("child-1.console"), + "consoles share the child-. shape and must survive the sweep" + ); + assert!(!Registry::is_child_backing("memory-assembled.bin")); + assert!(!Registry::is_child_backing("child.rootfs.ext4")); + assert!(!Registry::is_child_backing("child-x.rootfs.ext4")); + assert!(!Registry::is_child_backing("child-1.")); + assert!(!Registry::is_child_backing("rootfs.ext4")); + + // Shape, not suffix: the comparison is against everything after the + // first dot, so these match and the sweep would delete them. Nothing + // creates such a file — the assertions are here so a reader knows the + // predicate is deliberately approximate rather than exact. + assert!(Registry::is_child_backing("child-1.sock.bak")); + assert!(Registry::is_child_backing("child-1.SOCK")); + + // The stem is the tag's rootfs filename, so a multi-dot or non-ext4 stem + // must keep matching: tightening this to `.ext4` would look like an + // improvement and would silently stop collecting those backings. + assert!(Registry::is_child_backing("child-1.tar.gz")); + assert!(Registry::is_child_backing("child-1.rootfs.squashfs")); + + // The index is never parsed, so leading zeros and absurd values pass. + assert!(Registry::is_child_backing("child-0.rootfs.ext4")); + assert!(Registry::is_child_backing("child-01.rootfs.ext4")); + assert!(Registry::is_child_backing("child-4294967296.rootfs.ext4")); + + // ASCII digits only. Generalising to `is_numeric()` would accept unicode + // digits and start deleting names we never created. + assert!(!Registry::is_child_backing("child-1\u{662}.rootfs.ext4")); + // Callers pass a file name, not a path. + assert!(!Registry::is_child_backing("child-1/rootfs.ext4")); + assert!(!Registry::is_child_backing("")); + } + #[test] #[cfg(target_os = "linux")] // PidReuse path requires /proc starttime comparison fn kill_orphans_prunes_alive_pid_entries() { diff --git a/crates/forkd-vmm/src/lib.rs b/crates/forkd-vmm/src/lib.rs index ce822ef..6590ce6 100644 --- a/crates/forkd-vmm/src/lib.rs +++ b/crates/forkd-vmm/src/lib.rs @@ -266,6 +266,12 @@ pub struct Vm { /// Drop. Phase 6's UFFD_WP arming will dup this fd to register a /// `userfaultfd` against the same VMA. pub memfd: Option, + /// This child's own writable rootfs backing, when it was restored from a + /// snapshot that recorded a rootfs (see `child_rootfs_backing`). Owned + /// here so it dies with the child: work dirs are not reclaimed on their + /// own, so a backing left inside one is a full rootfs copy that nothing + /// would ever collect. `None` for VMs booted directly. + backing: Option, } /// Accept one connection on an already-non-blocking `UnixListener`, @@ -1440,6 +1446,113 @@ const CHILD_OOM_SCORE_ADJ: i32 = 500; /// to an attacker-pre-created file under `/tmp/`. const VMSTATE_ONLY_MEM_PLACEHOLDER: &str = "/dev/null/forkd-vmstate-only-mem-ignored"; +/// Where a restored child's own rootfs backing lives. +/// +/// A snapshot's rootfs is one file, and Firecracker reopens it verbatim for +/// every child, so without a per-child copy two concurrent children write one +/// filesystem with no coordinator. The backing is a reflink clone of the +/// snapshot's rootfs (see `chain::reflink_copy`), which costs nothing where the +/// filesystem supports it. +/// +/// It lives in the child's work dir so that removing the dir reclaims it. A +/// crash that skips that cleanup leaves a straggler beside the sockets, which is +/// the same class the startup sweep already collects. +fn child_rootfs_backing(work_dir: &Path, child_index: usize, tag_rootfs: &Path) -> PathBuf { + let stem = tag_rootfs + .file_name() + .and_then(|s| s.to_str()) + .unwrap_or("rootfs.ext4"); + work_dir.join(format!("child-{child_index}.{stem}")) +} + +/// The rootfs drive as the running Firecracker reports it: its host path and +/// whether it is read-only. Read from `GET /vm/config`, which on a restored VM +/// reflects the vmstate — so the read-only flag is the one the snapshot was +/// baked with, whatever `snapshot.json` does or does not record. +fn rootfs_drive(sock: &Path) -> Result<(PathBuf, bool)> { + let raw = api_request(sock, "GET", "/vm/config", "", DEFAULT_API_TIMEOUT_SECS)?; + let config: serde_json::Value = + serde_json::from_str(&raw).context("parse GET /vm/config response")?; + let drive = config["drives"] + .as_array() + .and_then(|drives| drives.iter().find(|d| d["drive_id"] == "rootfs")) + .context("GET /vm/config lists no drive with drive_id `rootfs`")?; + let path = drive["path_on_host"] + .as_str() + .context("rootfs drive has no path_on_host")?; + Ok((PathBuf::from(path), drive["is_read_only"] == true)) +} + +/// Re-point the rootfs drive at `path`, then read the configuration back and +/// fail unless Firecracker reports `path` as the drive's host path. +/// +/// The read-back is what makes this fail closed rather than trust a 204: a +/// build that accepted the PATCH without taking it would otherwise leave the +/// child on the old file with nothing to say so. Measured on stock v1.5.1, +/// v1.7.0, v1.10.1, v1.12.1, v1.14.1 and v1.17.0: every one moves the guest's +/// writes to the new file on a restored VM and reports the new path. +fn repoint_rootfs(sock: &Path, path: &Path) -> Result<()> { + // `drive_id` is required in the body — Firecracker rejects the override + // without it, which reads as a body error rather than a capability error. + api_call( + sock, + "PATCH", + "/drives/rootfs", + &serde_json::json!({"drive_id": "rootfs", "path_on_host": path}).to_string(), + )?; + let (reported, _) = rootfs_drive(sock).context("read back the rootfs drive after PATCH")?; + if reported != path { + bail!( + "Firecracker accepted PATCH /drives/rootfs but still reports {} as the rootfs, \ + not {}", + reported.display(), + path.display() + ); + } + Ok(()) +} + +/// Load a restored child **paused**, give it its own clone of `tag_rootfs` at +/// `backing`, re-point its rootfs there, then resume it. Returns whether a +/// backing was made: a read-only rootfs cannot be written, so it is shared as +/// is and costs no copy. +/// +/// The order is load-bearing and was verified against Firecracker v1.17: +/// loading with `resume_vm: false` means the guest has not executed a single +/// instruction, so no boot-time write (journal replay, `/var/log`, agent +/// startup) can reach the shared base before the drive is re-pointed. Resuming +/// afterwards hands the guest a disk that is already its own — measured: the +/// base stays byte-identical through boot while the backing changes. +/// +/// **Fail closed.** A child whose rootfs could not be re-pointed must not be +/// resumed: it would run against the shared base, which is the corruption this +/// exists to prevent. Any failure here propagates, and the caller drops the +/// child (which kills its Firecracker) rather than run it unsafely. +fn load_paused_rebind_rootfs_and_resume( + sock: &Path, + load_body: &str, + tag_rootfs: &Path, + backing: &Path, +) -> Result { + api_call(sock, "PUT", "/snapshot/load", load_body) + .context("load child paused (resume_vm=false)")?; + let (_, read_only) = rootfs_drive(sock).context("read the restored child's rootfs drive")?; + if !read_only { + crate::chain::reflink_copy(tag_rootfs, backing) + .with_context(|| format!("child rootfs backing {}", backing.display()))?; + repoint_rootfs(sock, backing).with_context(|| { + format!( + "re-point child rootfs at its own backing {} — refusing to resume a child \ + that would write the shared snapshot rootfs", + backing.display() + ) + })?; + } + api_call(sock, "PATCH", "/vm", r#"{"state":"Resumed"}"#) + .context("resume child after re-pointing its rootfs")?; + Ok(!read_only) +} + fn api_call(sock: &Path, method: &str, path: &str, body: &str) -> Result<()> { api_call_with_timeout(sock, method, path, body, DEFAULT_API_TIMEOUT_SECS) } @@ -1458,6 +1571,18 @@ fn api_call_with_timeout( body: &str, timeout_secs: u64, ) -> Result<()> { + api_request(sock, method, path, body, timeout_secs).map(|_| ()) +} + +/// `api_call_with_timeout`, returning the response body for the callers that +/// read one (`GET /vm/config`). +fn api_request( + sock: &Path, + method: &str, + path: &str, + body: &str, + timeout_secs: u64, +) -> Result { let mut stream = UnixStream::connect(sock).with_context(|| format!("connect {}", sock.display()))?; let timeout = Duration::from_secs(timeout_secs); @@ -1508,11 +1633,11 @@ fn api_call_with_timeout( ) })?; + let body_start = response + .find("\r\n\r\n") + .map(|i| i + 4) + .unwrap_or(response.len()); if !(200..300).contains(&status_code) { - let body_start = response - .find("\r\n\r\n") - .map(|i| i + 4) - .unwrap_or(response.len()); bail!( "firecracker API {} {} returned {}: {}", method, @@ -1521,7 +1646,7 @@ fn api_call_with_timeout( response[body_start..].trim() ); } - Ok(()) + Ok(response[body_start..].to_string()) } /// True if `buf` contains a complete HTTP response (headers + body up to @@ -1792,6 +1917,7 @@ impl Vm { netns: None, cgroup: None, memfd: None, + backing: None, }) } @@ -1817,6 +1943,103 @@ impl Vm { } } +impl Vm { + /// Give a paused VM's rootfs a durable home for the snapshot about to be + /// taken of it: clone this child's own backing into `dir`, and point the + /// drive there so the vmstate records a path the new snapshot owns. + /// + /// Without this, a snapshot of a restored child records the child's + /// backing — a file in a work dir that dies with the child — so the new + /// tag stops loading once the child is gone, and every child restored from + /// it would write the source's live disk in the meantime. + /// + /// The VM must stay paused until the returned detour is + /// [`finish`](RootfsDetour::finish)ed, which points the drive back at the + /// child's own backing. Returns `None` for a VM with no backing of its own + /// (booted directly, or restored from a snapshot that recorded no rootfs, + /// or a read-only rootfs): its drive already points at a durable file. + pub fn detour_rootfs_into(&self, dir: &Path) -> Result>> { + let Some(home) = self.backing.clone() else { + return Ok(None); + }; + // `child-.`; the snapshot keeps the tag's name. + let name = home + .file_name() + .and_then(|n| n.to_str()) + .and_then(|n| n.split_once('.')) + .map(|(_, rest)| rest.to_string()) + .unwrap_or_else(|| "rootfs.ext4".to_string()); + let path = dir.join(name); + crate::chain::reflink_copy(&home, &path) + .with_context(|| format!("clone child rootfs into {}", path.display()))?; + // Built before the PATCH so that a failed PATCH still points the drive + // home on drop — Firecracker may have taken it before the read-back. + let detour = RootfsDetour { + vm: self, + home, + path, + finished: false, + }; + repoint_rootfs(&self.sock, &detour.path).context("point rootfs at the snapshot's copy")?; + Ok(Some(detour)) + } +} + +/// A paused VM whose rootfs drive points at a snapshot's copy instead of its +/// own backing. See [`Vm::detour_rootfs_into`]. +/// +/// Resuming the VM in this state would have it write the snapshot's rootfs, so +/// pointing it home is fail-closed: if that cannot be verified, the VM's +/// Firecracker is killed rather than left to be resumed onto another tag's +/// disk. Dropping an unfinished detour does the same, which covers every early +/// return between the detour and the resume. +pub struct RootfsDetour<'a> { + vm: &'a Vm, + home: PathBuf, + path: PathBuf, + finished: bool, +} + +impl RootfsDetour<'_> { + /// Where the snapshot's copy of the rootfs is — the path to record as its + /// `rootfs`. + pub fn path(&self) -> &Path { + &self.path + } + + /// Point the drive back at the VM's own backing. On error the VM has been + /// killed. + pub fn finish(mut self) -> Result<()> { + self.finished = true; + self.go_home() + } + + fn go_home(&self) -> Result<()> { + repoint_rootfs(&self.vm.sock, &self.home).map_err(|e| { + // SAFETY: kill(2) on a pid we spawned; no memory is touched. + unsafe { + libc::kill(self.vm.pid as libc::pid_t, libc::SIGKILL); + } + e.context(format!( + "point rootfs back at the child's own backing {} — the child was killed \ + rather than resumed onto the snapshot's copy {}", + self.home.display(), + self.path.display() + )) + }) + } +} + +impl Drop for RootfsDetour<'_> { + fn drop(&mut self) { + if !self.finished { + if let Err(e) = self.go_home() { + tracing::warn!(error = %format!("{e:#}"), "rootfs detour: could not return home"); + } + } + } +} + /// RAII guard that holds a VM paused. Drop calls `vm.resume()` unless /// the guard was [`commit`](Self::commit)ted (used when the caller has /// already explicitly resumed and doesn't want the implicit retry). @@ -2116,6 +2339,11 @@ impl Drop for Vm { if let Some(cg) = &self.cgroup { cgroup::cleanup(cg); } + // Reclaim this child's writable rootfs copy. Work dirs outlive their + // VMs, so leaving it to the dir would leak a full rootfs per spawn. + if let Some(backing) = self.backing.take() { + let _ = std::fs::remove_file(&backing); + } } } @@ -2211,6 +2439,9 @@ impl Snapshot { netns, cgroup: None, memfd: None, + // Set by the backing pass below, once the child + // exists to own it. + backing: None, })); } Err(e) => { @@ -2355,18 +2586,45 @@ impl Snapshot { } } + // Every child of a tag that recorded a rootfs is assigned its own + // backing path up front, and owns it from here: Drop removes it, so a + // failed or killed child reclaims the copy instead of leaving a full + // rootfs in a work dir nothing sweeps. The copy itself is made in the + // child's restore thread below, once the loaded vmstate has said + // whether the drive is writable at all. Snapshots with no recorded + // rootfs keep the previous behaviour. + let tag_rootfs = self.rootfs.clone(); + let mut backings: Vec> = vec![None; children.len()]; + if let Some(tag_rootfs) = &tag_rootfs { + for (i, slot) in children.iter_mut().enumerate() { + let Some(c) = slot else { continue }; + let backing = child_rootfs_backing(work_dir, i + 1, tag_rootfs); + c.backing = Some(backing.clone()); + backings[i] = Some(backing); + } + } + // Phase 2: parallel restore via threads. Each thread issues one // /snapshot/load PUT to its child's API socket. Body varies per // child only under MemfdShared (each child has its own memfd // path); for the File path, all children share the same JSON. + // + // With a backing the child is loaded PAUSED, given its copy, + // re-pointed at it, and only then resumed, so no boot-time write can + // reach the shared base; the copies run in these threads, in parallel. + // Without one (no recorded rootfs) the previous single-call behaviour + // is preserved. let restore_start = Instant::now(); - let mut handles: Vec<(usize, u32, thread::JoinHandle>)> = Vec::new(); + let mut handles: Vec<(usize, u32, thread::JoinHandle>)> = Vec::new(); for (i, slot) in children.iter().enumerate() { let Some(c) = slot else { continue; }; let sock = c.sock.clone(); let pid = c.pid; + let backing = backings[i].clone(); + let tag_rootfs = tag_rootfs.clone(); + let resume_on_load = backing.is_none(); let body = match &c.memfd { Some(region) => serde_json::json!({ "snapshot_path": &self.vmstate, @@ -2376,7 +2634,7 @@ impl Snapshot { "shared": true, }, "enable_diff_snapshots": opts.enable_diff_snapshots, - "resume_vm": true, + "resume_vm": resume_on_load, }) .to_string(), None => serde_json::json!({ @@ -2386,21 +2644,35 @@ impl Snapshot { "backend_type": "File", }, "enable_diff_snapshots": opts.enable_diff_snapshots, - "resume_vm": true, + "resume_vm": resume_on_load, }) .to_string(), }; handles.push(( i, pid, - thread::spawn(move || -> Result<()> { - api_call(&sock, "PUT", "/snapshot/load", &body) + thread::spawn(move || -> Result { + match (backing, tag_rootfs) { + (Some(backing), Some(tag_rootfs)) => load_paused_rebind_rootfs_and_resume( + &sock, + &body, + &tag_rootfs, + &backing, + ), + _ => api_call(&sock, "PUT", "/snapshot/load", &body).map(|()| false), + } }), )); } for (i, pid, h) in handles { match h.join() { - Ok(Ok(())) => {} + Ok(Ok(true)) => {} + // A read-only rootfs is shared as is; there is no backing to own. + Ok(Ok(false)) => { + if let Some(c) = children[i].as_mut() { + c.backing = None; + } + } Ok(Err(e)) => { failures.push(RestoreFailure { child_index: i + 1, @@ -2770,6 +3042,194 @@ mod tests { let _ = std::fs::remove_file(&sock); } + /// A stand-in for Firecracker's API socket: answers `GET /vm/config` with + /// one rootfs drive, and records every request. `honour_patch: false` + /// models a build that answers `PATCH /drives` with 204 and keeps the old + /// file — the case the read-back exists to catch. + #[cfg(target_os = "linux")] + struct FakeFirecracker { + sock: PathBuf, + requests: std::sync::Arc>>, + } + + #[cfg(target_os = "linux")] + impl FakeFirecracker { + fn start(name: &str, rootfs: &str, read_only: bool, honour_patch: bool) -> Self { + use std::io::BufRead; + use std::os::unix::net::UnixListener; + let sock = std::env::temp_dir().join(format!( + "forkd-fakefc-{name}-{}-{}.sock", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_nanos() + )); + let listener = UnixListener::bind(&sock).unwrap(); + let requests = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let log = requests.clone(); + let mut path_on_host = rootfs.to_string(); + thread::spawn(move || { + for stream in listener.incoming() { + let Ok(stream) = stream else { return }; + let mut reader = std::io::BufReader::new(stream); + let mut line = String::new(); + if reader.read_line(&mut line).is_err() { + continue; + } + let request_line = line.trim().to_string(); + let mut len = 0usize; + loop { + let mut h = String::new(); + reader.read_line(&mut h).unwrap(); + if h == "\r\n" || h.is_empty() { + break; + } + if let Some(v) = h.to_ascii_lowercase().strip_prefix("content-length:") { + len = v.trim().parse().unwrap(); + } + } + let mut body = vec![0u8; len]; + reader.read_exact(&mut body).unwrap(); + let body: serde_json::Value = + serde_json::from_slice(&body).unwrap_or(serde_json::Value::Null); + let (method, path) = request_line + .split_once(' ') + .map(|(m, rest)| (m, rest.split(' ').next().unwrap_or(""))) + .unwrap(); + log.lock().unwrap().push(format!("{method} {path}")); + let reply = match (method, path) { + ("GET", "/vm/config") => serde_json::json!({"drives": [{ + "drive_id": "rootfs", + "path_on_host": path_on_host, + "is_read_only": read_only, + }]}) + .to_string(), + ("PATCH", "/drives/rootfs") => { + if honour_patch { + path_on_host = body["path_on_host"].as_str().unwrap().to_string(); + } + String::new() + } + _ => String::new(), + }; + let status = if reply.is_empty() { + "204 No Content" + } else { + "200 OK" + }; + let mut stream = reader.into_inner(); + let _ = write!( + stream, + "HTTP/1.1 {status}\r\nContent-Length: {}\r\n\r\n{reply}", + reply.len() + ); + } + }); + Self { sock, requests } + } + + fn requests(&self) -> Vec { + self.requests.lock().unwrap().clone() + } + } + + #[cfg(target_os = "linux")] + impl Drop for FakeFirecracker { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.sock); + } + } + + #[test] + #[cfg(target_os = "linux")] + fn repoint_rootfs_accepts_a_drive_that_moved() { + let fc = FakeFirecracker::start("moved", "/snap/rootfs.ext4", false, true); + repoint_rootfs(&fc.sock, Path::new("/work/child-1.rootfs.ext4")).unwrap(); + assert_eq!( + rootfs_drive(&fc.sock).unwrap(), + (PathBuf::from("/work/child-1.rootfs.ext4"), false) + ); + } + + #[test] + #[cfg(target_os = "linux")] + fn repoint_rootfs_fails_closed_when_patch_is_not_taken() { + let fc = FakeFirecracker::start("ignored", "/snap/rootfs.ext4", false, false); + let err = repoint_rootfs(&fc.sock, Path::new("/work/child-1.rootfs.ext4")).unwrap_err(); + let msg = format!("{err:#}"); + assert!( + msg.contains("still reports /snap/rootfs.ext4"), + "error should name the path Firecracker kept: {msg}" + ); + } + + #[test] + #[cfg(target_os = "linux")] + fn restore_never_resumes_a_child_whose_rootfs_did_not_move() { + let dir = + std::env::temp_dir().join(format!("forkd-rebind-noresume-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let base = dir.join("rootfs.ext4"); + std::fs::write(&base, b"base").unwrap(); + let backing = dir.join("child-1.rootfs.ext4"); + let fc = FakeFirecracker::start("noresume", base.to_str().unwrap(), false, false); + let r = load_paused_rebind_rootfs_and_resume(&fc.sock, "{}", &base, &backing); + assert!(r.is_err(), "a child left on the shared base must fail"); + let requests = fc.requests(); + assert!( + !requests.iter().any(|r| r == "PATCH /vm"), + "must not resume a child still pointed at the shared base: {requests:?}" + ); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + #[cfg(target_os = "linux")] + fn restore_gives_a_writable_rootfs_its_own_backing() { + let dir = + std::env::temp_dir().join(format!("forkd-rebind-writable-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let base = dir.join("rootfs.ext4"); + std::fs::write(&base, b"base").unwrap(); + let backing = dir.join("child-1.rootfs.ext4"); + let fc = FakeFirecracker::start("writable", base.to_str().unwrap(), false, true); + let used = load_paused_rebind_rootfs_and_resume(&fc.sock, "{}", &base, &backing).unwrap(); + assert!(used); + assert_eq!(std::fs::read(&backing).unwrap(), b"base"); + assert_eq!( + fc.requests(), + [ + "PUT /snapshot/load", + "GET /vm/config", + "PATCH /drives/rootfs", + "GET /vm/config", + "PATCH /vm" + ] + ); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + #[cfg(target_os = "linux")] + fn restore_shares_a_read_only_rootfs_without_copying() { + let dir = + std::env::temp_dir().join(format!("forkd-rebind-readonly-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let base = dir.join("rootfs.squashfs"); + std::fs::write(&base, b"base").unwrap(); + let backing = dir.join("child-1.rootfs.squashfs"); + let fc = FakeFirecracker::start("readonly", base.to_str().unwrap(), true, true); + let used = load_paused_rebind_rootfs_and_resume(&fc.sock, "{}", &base, &backing).unwrap(); + assert!(!used); + assert!(!backing.exists(), "a read-only rootfs must not be copied"); + assert_eq!( + fc.requests(), + ["PUT /snapshot/load", "GET /vm/config", "PATCH /vm"] + ); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn wait_for_sock_returns_immediately_when_path_exists() { let sock = std::env::temp_dir().join(format!( diff --git a/docs/VENDORED-FIRECRACKER.md b/docs/VENDORED-FIRECRACKER.md index 0bcaa57..db77a79 100644 --- a/docs/VENDORED-FIRECRACKER.md +++ b/docs/VENDORED-FIRECRACKER.md @@ -10,7 +10,11 @@ The fix is a ~44-line patch adding an opt-in `shared: bool` field to `MemBackend ## Where the fork lives -[**`deeplethe/firecracker`**](https://github.com/deeplethe/firecracker), branch [`forkd-v0.4-mem-backend-shared-v1.12`](https://github.com/deeplethe/firecracker/tree/forkd-v0.4-mem-backend-shared-v1.12) (matches the FC version forkd produces snapshots with). +[**`deeplethe/firecracker`**](https://github.com/deeplethe/firecracker), branch [`forkd-v0.4-mem-backend-shared-v1.12`](https://github.com/deeplethe/firecracker/tree/forkd-v0.4-mem-backend-shared-v1.12) (matches the version that branch was forked from). + +**The fork is needed only for `--live`.** It exists to carry the opt-in `shared: bool` on `MemBackendConfig`, and `forkd-vmm` sends `shared` in exactly one place — the memfd path live-fork uses. Every other restore path, and the whole bake/restore/branch/exec surface, runs on **stock Firecracker**. The tested baseline is **v1.17.0**; per-child rootfs backings rely on `PATCH /drives` on a restored VM, which moves the device's storage on every stock release measured back to v1.5.1. So if you do not use live-fork, run upstream Firecracker and ignore this document. + +One thing to know before changing the version: a vmstate is version-pinned, so a new build may refuse to load snapshots written by an older one — plan to re-bake tags when you upgrade. - Forked from upstream `v1.12.0` tag. - Five commits on the branch (combined ~140 lines, all forkd-specific feature work):