From 81bf240f10214287205e28d3f0e972f31085dafe Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 00:04:32 +0200 Subject: [PATCH 1/4] Read a launched row's image from the row's own path, resolved once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The supervisor's launch path resolved the requested program twice. `declared` followed one caller-writable link to name the `[programs]` row (and with it the row's capabilities), and the kernel then opened the launch's `argv[0]` — the same caller path — to load the image, because for a `/system` program the supervisor passed no image object. A caller re-pointing the link between the two reads ran its own bytes under the named row. For `update` that row carries the idle slot's partition claims, so any program could run its own code holding them. The fix reads the image from the row's own declared path, resolved once, and carries `argv[0]` beside it as a name only: - std gains `os::toyos::process::CommandExt::image_from`, which names the path the image is read from and forces the read even for a kernel-served path the kernel would otherwise open from `argv[0]`. The spawn then carries an object and the kernel never opens `argv[0]`; `argv[0]` stays the caller's name so an applet still learns which name it was invoked as. (rust fork wt-toyos-launchonce; gitlink bumped.) - The supervisor resolves the row on its file worker and calls `image_from(row.path)` before `prepare`. `row.path` is always a `/system` path for a `[programs]` row, which is read-only to userland, so the bytes a declared row runs cannot be swapped. This is a security boundary, so its evidence is a negative control and an independent oracle. `tests/launchoncecase` gives `toybox` a `roster` capability no plain job holds; `tests/toyos-rust-tests/src/bin/launch_toctou.rs` points a `/tmp` symlink at `/system/bin/toybox`, launches it through the launcher while a thread re-points the symlink at its own binary, and asserts its bytes never ran holding that capability. The `launch_toctou` QEMU machine test boots that config and judges the job. On the base (whole fix reverted) the caller's bytes ran holding the row's capability after two launches; with the fix 485 launches ran only the row's own bytes. A second, present isolation hole is filed, not fixed (owner: the per-program views track): the `launcher` connector is inherited down every shell-started subtree, the supervisor never checks which rows a caller may start, and a swap checks only the requester's digest — so any program can reach any declared row's authority. `issues/isolation/the-launcher-starts-any-row-for-any-holder.md`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...-launcher-starts-any-row-for-any-holder.md | 49 ++++++++ rust | 2 +- src/build.rs | 1 + tests/launchoncecase/system.toml | 49 ++++++++ .../toyos-rust-tests/src/bin/launch_toctou.rs | 108 ++++++++++++++++++ tests/toyos.rs | 49 ++++++++ userland/supervisor/src/main.rs | 31 +++-- 7 files changed, 281 insertions(+), 8 deletions(-) create mode 100644 issues/isolation/the-launcher-starts-any-row-for-any-holder.md create mode 100644 tests/launchoncecase/system.toml create mode 100644 tests/toyos-rust-tests/src/bin/launch_toctou.rs diff --git a/issues/isolation/the-launcher-starts-any-row-for-any-holder.md b/issues/isolation/the-launcher-starts-any-row-for-any-holder.md new file mode 100644 index 00000000000..474c8ff3a15 --- /dev/null +++ b/issues/isolation/the-launcher-starts-any-row-for-any-holder.md @@ -0,0 +1,49 @@ +--- +status: open +kind: defect +opened: 2026-10-03 +--- + +# The launcher starts any declared row for any holder of it + +A holder of a `launcher` connector can start **any** `[programs]` row and is +handed that row's whole authority. Three facts compose: + +- **`launcher` is inherited, not granted per child.** A direct spawn copies the + caller's `svc` namespace into the child (`inherited_namespace`, + `rust/library/std/src/sys/process/toyos.rs`), and std routes every declared + program through the launcher when the caller holds one and endowed nothing + itself (the routing rule in `Command::spawn`/`Command::launch`, same file). So + every program in a shell-started subtree holds the shell's `launcher`. +- **The supervisor never checks which rows a caller may start.** `serve_launch` + (`userland/supervisor/src/main.rs`) resolves `request.program` to a declared + row and starts it holding that row's `receives`, `devices`, `syscap` and + `slots` — for any caller that reached the port. A plain app launched from the + shell can therefore launch `/system/bin/swap` and receive the swap connector, + or `/system/bin/update` and receive the idle slot's partition claims. +- **`accept_swap` checks the requester's own digest, not its identity.** It + verifies the service exists, is a running `[boot] start` service, and that the + staged bytes match the requester's `digest` — never which program asked + (`accept_swap`, same file). The swap connector is gated to `/system/bin/swap` + by the build (`held_by_their_holders_alone`, `src/build.rs`), but fact one and + fact two reach that row anyway. So any program can rewrite any boot service's + binary. + +This is the service-namespace shape of the hole the per-program-views track +names ("the swap port leaking to whatever sshd spawns", PR #484's review, +`issues/isolation/every-program-sees-only-the-files-it-was-given.md`), and it is +distinct from where a swapped binary lives +(`issues/isolation/a-swapped-binary-lives-where-any-process-can-rewrite-it.md`): +that one is which bytes a declared swap runs, this one is who may invoke a +declared row at all. + +**Owner.** The per-program-views track +(`issues/isolation/every-program-sees-only-the-files-it-was-given.md`), which +badges a program's authority per row. + +**Exit condition.** `launcher` is badged per row: a launcher connector names +which rows its holder may start, `serve_launch` refuses a row the badge does not +name, and a swap gates on the requester's badge rather than on its digest alone. +A test fails until then: a program whose row grants no launcher badge for +`/system/bin/swap` launches it through an inherited `launcher` and the supervisor +refuses the launch. diff --git a/rust b/rust index a0d44493347..8edfa580210 160000 --- a/rust +++ b/rust @@ -1 +1 @@ -Subproject commit a0d44493347e5be082ed4b7507b53537bab2c884 +Subproject commit 8edfa5802108b2b5a2a314f37392ccbbc0ecff10 diff --git a/src/build.rs b/src/build.rs index 9a2c6d839b6..e8845736f0a 100644 --- a/src/build.rs +++ b/src/build.rs @@ -3098,6 +3098,7 @@ mod tests { "tests/lanleasecase/system.toml", "tests/lantalkcase/system.toml", "tests/latencycase/system.toml", + "tests/launchoncecase/system.toml", "tests/logstallcase/system.toml", "tests/metalcase/system.toml", "tests/metaldevicecase/system.toml", diff --git a/tests/launchoncecase/system.toml b/tests/launchoncecase/system.toml new file mode 100644 index 00000000000..96faa217b8b --- /dev/null +++ b/tests/launchoncecase/system.toml @@ -0,0 +1,49 @@ +# The boot `launch_toctou` runs on: a launcher, a declared row with a capability +# the caller's own job does not hold (`toybox`, here given `roster`), and the +# file servers, so a launch names a working directory a server judges — which is +# where the base's second path resolution happens. +# +# `launch_toctou` creates a caller-writable `/tmp` symlink that resolves to +# `/system/bin/toybox`, launches it through the launcher while a second thread +# re-points the symlink at its own binary, and asserts the supervisor never runs +# the caller's bytes under `toybox`'s row. + +[boot] +start = ["logkeeper", "diskserver", "fileserver", "test-runner"] + +# `every_boot_config_runs_logkeeper` refuses a boot config without it: the kernel +# keeps the record ring and writes no file, so such a boot's `/log` is empty. It +# claims no device and serves no port. +[programs.logkeeper] +service = true +syscap = ["logread"] + +# `roster` because the guest hands each binary a duplicate of its capability and +# the launch places children under itself; `launcher` because the subtree under +# test is started through it. +[programs.test-runner] +receives = ["launcher"] +syscap = ["dup", "roster"] + +# The declared row the caller's symlink resolves to. `roster` is the capability a +# launched instance of this row holds and a plain job does not: a win of the race +# runs the caller's binary holding it, which is the whole defect. +[programs.toybox] +syscap = ["roster"] + +# The block service: the NVMe controller this machine's DATA is on, driven from +# userland through its claim. Started again when it ends; the claim goes back +# with the process and is minted again for the next. +[programs.diskserver] +service = true +restart = true +serves = ["block"] +devices = ["pci:1b36:0010"] + +# The file servers: DATA, the log and the running slot's volume, one process +# each, serving every program the directories of its role. Started again when +# one ends, on the same ports. +[programs.fileserver] +restart = true +roles = ["data", "log", "boot"] +receives = ["block"] diff --git a/tests/toyos-rust-tests/src/bin/launch_toctou.rs b/tests/toyos-rust-tests/src/bin/launch_toctou.rs new file mode 100644 index 00000000000..350f711bd83 --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/launch_toctou.rs @@ -0,0 +1,108 @@ +//! A declared row's image comes from the row's own path, resolved once — never +//! from a caller-writable path re-read after the row was named. +//! +//! The supervisor's launch path resolves `request.program` twice on the base: once +//! to name the `[programs]` row (and with it the row's capabilities), and again +//! to read the image. A `/tmp` symlink resolves to the declared row at the +//! first and to the caller's own binary at the second, so the caller's bytes +//! run holding the row's capability. +//! +//! This binary is both halves. The default role is the attacker: it points a +//! `/tmp` symlink at `/system/bin/toybox` (a declared row this config gives the +//! `roster` capability a plain job never holds), launches it through the +//! launcher while a second thread re-points the symlink at this binary, and +//! waits each launched child. The `evil` role is what the re-point substitutes: +//! it exits [`EXPLOIT`] when it holds the row's `SysCap` — proof the caller's +//! bytes ran under the declared row — and `0` otherwise. +//! +//! On the fix the supervisor reads the image from the row's `/system` path (an +//! immutable, kernel-served mount) into an object, so the kernel never re-opens +//! the caller's symlink and the `evil` bytes never run under the row. The race +//! then never resolves to [`EXPLOIT`] and the attacker exits `0`. + +use std::process::{Command, Stdio}; +use std::time::{Duration, Instant}; + +use std::os::toyos::fs::symlink; + +use toyos::endow::{Endowments, SYSCAP_LABEL}; + +/// This binary's own path, the bytes the re-point substitutes for the row's. +const SELF_PATH: &str = "/system/bin/test_rs_launch_toctou"; +/// The declared row the symlink resolves to. `/system` is read-only to every +/// program, so the row's bytes cannot be rewritten — only which file the +/// caller's symlink names can. +const DECLARED: &str = "/system/bin/toybox"; +/// The caller-writable symlink raced between the two paths. +const LINK: &str = "/tmp/x"; + +/// The `evil` role's exit code when it ran holding the declared row's `SysCap`. +/// Distinct from every code toybox or a plain spawn produces. +const EXPLOIT: i32 = 123; + +/// How long the attacker keeps racing before it concludes the exploit never +/// landed. Well under the runner's one-job budget; the window the base leaves +/// open is the whole of the supervisor's `start`, so a win comes in far fewer +/// attempts than this bounds. +const RACE_BUDGET: Duration = Duration::from_secs(30); +const MAX_ATTEMPTS: u32 = 600; + +fn main() { + match std::env::args().nth(1).as_deref() { + Some("evil") => evil(), + _ => attacker(), + } +} + +/// The caller's bytes. Reached only when the supervisor read this binary's image +/// for a launch it resolved to the `toybox` row. +fn evil() -> ! { + // A launched instance of the `toybox` row is endowed that row's `SysCap`; + // a plain child the attacker spawns directly is not. So holding one here is + // proof these bytes ran under the row the caller never named for them. + let under_row = Endowments::get().holds(SYSCAP_LABEL); + std::process::exit(if under_row { EXPLOIT } else { 0 }); +} + +fn attacker() { + // Start the symlink on the declared row, so the first resolution names it. + let _ = std::fs::remove_file(LINK); + symlink(DECLARED, LINK).expect("create /tmp/x -> the declared row"); + + // Re-point forever between the row's path and this binary's. tmpfs displaces + // the existing entry, so each call is one atomic swap of what `/tmp/x` names. + std::thread::spawn(|| loop { + let _ = symlink(SELF_PATH, LINK); + let _ = symlink(DECLARED, LINK); + }); + + let deadline = Instant::now() + RACE_BUDGET; + let mut exploited = false; + let mut attempts = 0u32; + while attempts < MAX_ATTEMPTS && Instant::now() < deadline { + attempts += 1; + // A served working directory, judged by a file server mid-launch, is + // one of the places the base's second resolution waits — widening the + // window the re-point has to land in. + let spawned = Command::new(LINK) + .arg("evil") + .current_dir("/home") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn(); + let Ok(mut child) = spawned else { continue }; + if child.wait().ok().and_then(|s| s.code()) == Some(EXPLOIT) { + exploited = true; + break; + } + } + + assert!( + !exploited, + "launch_toctou: the caller's bytes ran holding the `toybox` row's capability after \ + {attempts} launch(es): the supervisor read a declared row's image from the caller's \ + path, re-read after the row was named" + ); + println!("launch_toctou: PASS ({attempts} launches, none ran the caller's bytes under the row)"); +} diff --git a/tests/toyos.rs b/tests/toyos.rs index 486edcdcd90..9a34f5c9a3e 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -98,6 +98,11 @@ const RUST_SKIP: &[&str] = &[ // Needs a launcher and a declared `cat` and shell, which `tests/testcases` // does not give: the `process_tree` metal row runs it on tests/proctreecase. "process_tree", + // Needs a launcher and a declared row with a capability a plain job lacks, + // and it is both the attacker and the bytes a re-point substitutes, so its + // own `/system/bin` entry must exist: the `launch_toctou` machine test runs + // it on tests/launchoncecase. + "launch_toctou", // It asserts nothing at all: it holds a `tests/lanleasecase` boot open for // twenty seconds. On a shared boot it would be twenty seconds of nothing. "lan_hold", @@ -196,6 +201,11 @@ const MACHINE_TESTS: &[&str] = &[ // no way to turn it back on, so only a machine QEMU reports stopping can // be asked. `machine_soft_off_decoded` reads the T14's own decode. "machine_shutdown", + // A launch resolves a declared row's image from the row's own path, not from + // the caller's. Its own boot, because it needs a launcher and a declared row + // with a capability a plain job lacks, and it hammers hundreds of launches — + // a verdict about which bytes ran, not how fast, so QEMU answers it. + "launch_toctou", ]; /// **The metal profile**: which registrations run on the ThinkPad T14, what @@ -2361,10 +2371,49 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { "iommu_virtio_platform" => common::iommu::iommu_virtio_platform(test_config), "nested_nmi_is_loud" => faults::nested_nmi_is_loud(test_config), "machine_shutdown" => power::machine_shutdown(test_config), + "launch_toctou" => launch_toctou(), other => Err(format!("unknown machine test {other}")), } } +/// A declared row's image is read from the row's own path, resolved once. +/// +/// `tests/launchoncecase` gives `toybox` a `roster` capability no plain job +/// holds. `test_rs_launch_toctou` points a `/tmp` symlink at `/system/bin/toybox`, +/// launches it through the launcher while a thread re-points the symlink at its +/// own binary, and asserts its bytes never ran holding that capability. On the +/// base they do — the supervisor resolves `request.program` to the row and then +/// re-reads it for the image — so the job exits non-zero. The verdict is which +/// bytes ran, a content fact QEMU answers; the race is only how it is reached. +fn launch_toctou() -> Result<(), String> { + let profile = qemu::Profile::Headless; + let bin = qemu::build_toyos_bin( + profile.arch(), + &compile::repo_root().join("tests/toyos-rust-tests"), + "launch_toctou", + ); + let options = BootOptions { profile, ..Default::default() }; + let mut qemu = QemuInstance::boot_with_options( + &compile::repo_root().join("tests/launchoncecase"), + &[], + &[("launch_toctou".to_string(), bin)], + options, + ); + serial::Serial::boot(&qemu).must_be_clean()?; + let result = qemu.run_test("test_rs_launch_toctou", Duration::from_secs(120)); + match result.exit_code { + Some(0) => { + eprintln!(" [launch] {}", result.stdout.lines().last().unwrap_or("").trim()); + Ok(()) + } + Some(code) => Err(format!("test_rs_launch_toctou exited {code}\n{}", result.stdout)), + None => Err(format!( + "test_rs_launch_toctou did not finish: {:?}\n{}", + result.error, result.stdout + )), + } +} + /// Every CPU's `CR0` and `CR4`, against what a CPU running this kernel must /// hold. /// diff --git a/userland/supervisor/src/main.rs b/userland/supervisor/src/main.rs index 4d3548aa508..c1065ea8e88 100644 --- a/userland/supervisor/src/main.rs +++ b/userland/supervisor/src/main.rs @@ -55,6 +55,7 @@ use std::cell::{Cell, RefCell}; use std::collections::{BTreeMap, VecDeque}; use std::os::toyos::process::{ChildExt, CommandExt}; +use std::path::Path; use std::process::{Child, Command, Stdio}; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; @@ -1470,10 +1471,13 @@ impl Supervisor<'_> { } }; - // **The caller's path, not the row's, and `argv[0]` is why.** `declared` - // has already established that the two name one binary, so this grants - // nothing extra — and `/system/bin/echo` spawned as `/system/bin/toybox` is a toybox that - // was never told which applet it is. + // **`argv[0]` is the caller's path; the image is the row's.** The caller's + // name has to reach the child so an applet learns which it is — + // `/system/bin/echo` run as `ls` is an `ls` — but the bytes come from the + // row's own declared path (`image_from`, below), never from this path + // re-read. `declared` may have reached the row through a caller-writable + // symlink, and loading from here would run whatever that link named at + // load time: the caller's own bytes, under the row's claims. let mut command = Command::new(request.program); // **Carried, not inherited.** A child of the launcher would otherwise get // the supervisor's environment and the supervisor's working directory, so `cd /tmp && ls` would @@ -1504,9 +1508,22 @@ impl Supervisor<'_> { let (system, path) = (self.system, request.program.to_string()); let found = self.files("a launch's files", move || { let resolved = resolve(system, &path); - let prepared = match resolved { - Resolved::Row(_) | Resolved::Package(_) => command.prepare().map(drop), - Resolved::NotDeclared | Resolved::Refused(_) => Ok(()), + // The image is read from the row's own declared path, resolved once + // (`resolve`/`declared`), never from `request.program` a second time: + // the kernel loads the object this builds rather than re-opening the + // launch's `argv[0]`, so a caller-writable symlink cannot be + // re-pointed between naming the row and loading its bytes. + let row_path = match &resolved { + Resolved::Row(row) => Some(row.path.clone()), + Resolved::Package(row) => Some(row.path.clone()), + Resolved::NotDeclared | Resolved::Refused(_) => None, + }; + let prepared = match row_path { + Some(row_path) => { + command.image_from(Path::new(&row_path)); + command.prepare().map(drop) + } + None => Ok(()), }; (resolved, prepared.map(|()| command)) }); From 227f0d31d9b231d7caa253fc9d7193fd68fbfb40 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 00:58:10 +0200 Subject: [PATCH 2/4] SYS_SPAWN opens the program's path, never argv[0], and a launched row runs from it Round 1 had the supervisor read a launched row's binary into a memory object of its own, so the kernel would load that and never open the caller's path. That costs one 2 MiB physically contiguous frame per live launched program, held for the child's life: measured under QEMU with eight live launched toybox instances, 20480 KiB each on the base and 22528 KiB with the copy. SpawnArgs now carries the program's path apart from argv. The kernel opens the path and names the child by argv[0], which nothing opens; libraries, the modules' answer and the log lines follow the path. std passes the program it resolved, or the one image_from names, and still reads a file server's program into an image. The supervisor's launcher names the row's own path, so a launched row is paged from the ROOT image as before the fix, and a link the caller re-points between `declared` and the load runs nothing it did not name. The race is a metal row on the proctreecase boot, judged by the job's exit, in place of a QEMU machine test with its own config; that boot's toybox row holds roster, the capability the race is judged by. process_tree's launch-refusal arm gains image_from's two refusals. The launcher issue states the owner's ruling as he gave it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...-launcher-starts-any-row-for-any-holder.md | 22 +-- kernel/src/loader/mod.rs | 13 +- kernel/src/syscall/dispatch.rs | 3 +- kernel/src/syscall/proc.rs | 3 +- rust | 2 +- src/build.rs | 1 - tests/launchoncecase/system.toml | 49 ------ tests/proctreecase/system.toml | 5 + .../src/bin/abuse_elf_loader.rs | 4 + .../src/bin/abuse_elf_segments.rs | 2 + .../src/bin/abuse_handle_table.rs | 4 + .../src/bin/abuse_page_straddle.rs | 2 + .../src/bin/abuse_spawn_argv.rs | 4 +- .../toyos-rust-tests/src/bin/launch_toctou.rs | 147 +++++++++--------- .../toyos-rust-tests/src/bin/process_tree.rs | 26 +++- .../src/bin/spawn_child_ends_first.rs | 2 + .../src/bin/spawn_image_object.rs | 2 + .../src/bin/spawn_lands_claimed.rs | 2 + tests/toyos.rs | 67 ++------ toyos-abi/src/syscall.rs | 18 ++- toyos-userbound/src/span.rs | 2 +- userland/supervisor/src/main.rs | 32 ++-- 22 files changed, 179 insertions(+), 233 deletions(-) delete mode 100644 tests/launchoncecase/system.toml diff --git a/issues/isolation/the-launcher-starts-any-row-for-any-holder.md b/issues/isolation/the-launcher-starts-any-row-for-any-holder.md index 474c8ff3a15..b0a18d188a7 100644 --- a/issues/isolation/the-launcher-starts-any-row-for-any-holder.md +++ b/issues/isolation/the-launcher-starts-any-row-for-any-holder.md @@ -29,21 +29,21 @@ handed that row's whole authority. Three facts compose: fact two reach that row anyway. So any program can rewrite any boot service's binary. -This is the service-namespace shape of the hole the per-program-views track -names ("the swap port leaking to whatever sshd spawns", PR #484's review, +The per-program-views track names the same shape in the service namespace ("the +swap port leaking to whatever sshd spawns", `issues/isolation/every-program-sees-only-the-files-it-was-given.md`), and it is distinct from where a swapped binary lives (`issues/isolation/a-swapped-binary-lives-where-any-process-can-rewrite-it.md`): that one is which bytes a declared swap runs, this one is who may invoke a declared row at all. -**Owner.** The per-program-views track -(`issues/isolation/every-program-sees-only-the-files-it-was-given.md`), which -badges a program's authority per row. +**The owner's ruling:** each program may start only the rows its own row lists; +swap and update of system programs only from the login session. -**Exit condition.** `launcher` is badged per row: a launcher connector names -which rows its holder may start, `serve_launch` refuses a row the badge does not -name, and a swap gates on the requester's badge rather than on its digest alone. -A test fails until then: a program whose row grants no launcher badge for -`/system/bin/swap` launches it through an inherited `launcher` and the supervisor -refuses the launch. +**Owner.** The orchestrator. + +**Exit condition.** A guest test that is red until both halves of the ruling +hold: a launch of a row the caller's own row does not list is refused — +`/system/bin/swap` and `/system/bin/update` through an inherited `launcher` +among them — and a swap and an update of a system program asked from outside +the login session are refused. diff --git a/kernel/src/loader/mod.rs b/kernel/src/loader/mod.rs index 277442a54bc..2526058146b 100644 --- a/kernel/src/loader/mod.rs +++ b/kernel/src/loader/mod.rs @@ -340,15 +340,16 @@ fn rela_dyn_from_sections( /// child's handle table around the child's handle to itself, once nothing is /// left to refuse. /// -/// `image` is the program's bytes when the caller read them itself, and then -/// `argv[0]` is only its name: nothing opens it, and its libraries come from +/// `path` is the program, opened here — or, with `image`, the bytes the caller +/// read from it itself, and then nothing opens it and its libraries come from /// `/system/lib` alone, since the directory it names is no volume of this -/// kernel's. +/// kernel's. `argv[0]` is only the name the child goes by. /// /// `Refusal`, not `-> !`, is the error type: every failure below owns a /// partly built process (address space, stack, kernel stack), and nothing /// unwinds, so the error must travel out as a value rather than strand it. pub fn spawn( + path: &str, argv: &[&str], commit: impl FnOnce(crate::object::HandleEntry) -> Result<(HandleTable, Endowments, H), crate::object::Refusal>, cwd: String, @@ -357,7 +358,7 @@ pub fn spawn( parent: Parent, ) -> Result<(Pid, H), crate::object::Refusal> { // An argv of only separators survives sys_spawn's split as an empty slice. - let Some(&path) = argv.first() else { + let Some(&name) = argv.first() else { return Err(SyscallError::InvalidArgument.into()); }; // Before anything is built; dropped on every way out below but the insert. @@ -646,7 +647,7 @@ pub fn spawn( let ((tid, dst), retire) = admission.land(guard.as_mut().unwrap(), |table, node| { table.insert(ProcessEntry::new( Arc::clone(&object), - start::make_name(path), + start::make_name(name), proc_data, Arc::clone(&syms), ThreadEntry::new(thread_data), @@ -902,7 +903,7 @@ pub fn spawn_supervisor() -> Pid { start::endow_self(&mut handles, &mut entries, &mut labels, own); Ok((handles, Endowments::new(entries, labels), ())) }; - match spawn(&[SUPERVISOR_PATH], commit, String::from("/"), Vec::new(), None, Parent::Root) { + match spawn(SUPERVISOR_PATH, &[SUPERVISOR_PATH], commit, String::from("/"), Vec::new(), None, Parent::Root) { Ok((pid, ())) => pid, Err(crate::object::Refusal::Error(e)) => panic!("spawn_supervisor: failed to spawn: {e:?}"), Err(crate::object::Refusal::Handle(e)) => panic!("spawn_supervisor: {e}"), diff --git a/kernel/src/syscall/dispatch.rs b/kernel/src/syscall/dispatch.rs index 6ac1a96ed78..3764111a12c 100644 --- a/kernel/src/syscall/dispatch.rs +++ b/kernel/src/syscall/dispatch.rs @@ -173,6 +173,7 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> Ok(parent) => parent, Err(refused) => return refused, }; + let path = match ctx.user_str(UserAddr::new(args.path_ptr), args.path_len) { Ok(s) => s, Err(e) => return e.to_u64() }; let text = match ctx.user_str(UserAddr::new(args.argv_ptr), args.argv_len) { Ok(s) => s, Err(e) => return e.to_u64() }; let cwd = match ctx.user_str(UserAddr::new(args.cwd_ptr), args.cwd_len).and_then(|p| spawn_cwd(&p)) { Ok(cwd) => cwd, @@ -219,7 +220,7 @@ pub(crate) fn syscall_dispatch(num: u64, a1: u64, a2: u64, a3: u64, a4: u64) -> alloc::vec::Vec::new() }; let argv: alloc::vec::Vec<&str> = text.split('\0').filter(|s| !s.is_empty()).collect(); - sys_spawn(&argv, pending, cwd, env, image, parent) + sys_spawn(&path, &argv, pending, cwd, env, image, parent) } SYS_PROCESS_WAIT => sys_process_wait(RawHandle(a1 as u32), a2), SYS_PROCESS_KILL => { diff --git a/kernel/src/syscall/proc.rs b/kernel/src/syscall/proc.rs index 80c97aef4f7..e856d42a1c1 100644 --- a/kernel/src/syscall/proc.rs +++ b/kernel/src/syscall/proc.rs @@ -46,6 +46,7 @@ pub(super) fn spawn_place(place: u64) -> Result { /// Start a program in `cwd` under `parent` and return the handle to it the spawn's commit put in the caller's table. pub(super) fn sys_spawn( + path: &str, args: &[&str], pending: crate::loader::PendingHandles, cwd: alloc::string::String, @@ -54,7 +55,7 @@ pub(super) fn sys_spawn( parent: process::Parent, ) -> u64 { // Nothing to clean up: spawn's frame owns the child's resources on error. - match process::spawn(args, |own| pending.commit(own), cwd, env, image, parent) { + match process::spawn(path, args, |own| pending.commit(own), cwd, env, image, parent) { Ok((_, handle)) => u64::from(handle.0), Err(e) => e.refuse(), } diff --git a/rust b/rust index 8edfa580210..724238524f4 160000 --- a/rust +++ b/rust @@ -1 +1 @@ -Subproject commit 8edfa5802108b2b5a2a314f37392ccbbc0ecff10 +Subproject commit 724238524f4dcddc202ffa570cf034c512fe3e1d diff --git a/src/build.rs b/src/build.rs index faa883f3785..bae3cc48b6c 100644 --- a/src/build.rs +++ b/src/build.rs @@ -3069,7 +3069,6 @@ mod tests { "tests/lanleasecase/system.toml", "tests/lantalkcase/system.toml", "tests/latencycase/system.toml", - "tests/launchoncecase/system.toml", "tests/logstallcase/system.toml", "tests/metalcase/system.toml", "tests/metaldevicecase/system.toml", diff --git a/tests/launchoncecase/system.toml b/tests/launchoncecase/system.toml deleted file mode 100644 index 96faa217b8b..00000000000 --- a/tests/launchoncecase/system.toml +++ /dev/null @@ -1,49 +0,0 @@ -# The boot `launch_toctou` runs on: a launcher, a declared row with a capability -# the caller's own job does not hold (`toybox`, here given `roster`), and the -# file servers, so a launch names a working directory a server judges — which is -# where the base's second path resolution happens. -# -# `launch_toctou` creates a caller-writable `/tmp` symlink that resolves to -# `/system/bin/toybox`, launches it through the launcher while a second thread -# re-points the symlink at its own binary, and asserts the supervisor never runs -# the caller's bytes under `toybox`'s row. - -[boot] -start = ["logkeeper", "diskserver", "fileserver", "test-runner"] - -# `every_boot_config_runs_logkeeper` refuses a boot config without it: the kernel -# keeps the record ring and writes no file, so such a boot's `/log` is empty. It -# claims no device and serves no port. -[programs.logkeeper] -service = true -syscap = ["logread"] - -# `roster` because the guest hands each binary a duplicate of its capability and -# the launch places children under itself; `launcher` because the subtree under -# test is started through it. -[programs.test-runner] -receives = ["launcher"] -syscap = ["dup", "roster"] - -# The declared row the caller's symlink resolves to. `roster` is the capability a -# launched instance of this row holds and a plain job does not: a win of the race -# runs the caller's binary holding it, which is the whole defect. -[programs.toybox] -syscap = ["roster"] - -# The block service: the NVMe controller this machine's DATA is on, driven from -# userland through its claim. Started again when it ends; the claim goes back -# with the process and is minted again for the next. -[programs.diskserver] -service = true -restart = true -serves = ["block"] -devices = ["pci:1b36:0010"] - -# The file servers: DATA, the log and the running slot's volume, one process -# each, serving every program the directories of its role. Started again when -# one ends, on the same ports. -[programs.fileserver] -restart = true -roles = ["data", "log", "boot"] -receives = ["block"] diff --git a/tests/proctreecase/system.toml b/tests/proctreecase/system.toml index 11d9f6f1ddb..55a90801fc0 100644 --- a/tests/proctreecase/system.toml +++ b/tests/proctreecase/system.toml @@ -3,6 +3,8 @@ # A `launcher`, because the subtree under test has children the supervisor launches; a # declared `cat`, which runs until its input ends; and a declared shell, which a # depth chain launches and which detaches a `cat`. +# +# `launch_toctou` rides it too: a launch of `toybox` through a link it re-points. [boot] start = ["logkeeper", "diskserver", "fileserver", "test-runner"] @@ -20,7 +22,10 @@ syscap = ["logread"] receives = ["launcher"] syscap = ["dup", "roster"] +# `roster`, which a child the job spawns itself never holds: a child holding +# it ran under this row, and `launch_toctou` asks which bytes that child was. [programs.toybox] +syscap = ["roster"] [programs.shell] receives = ["launcher"] diff --git a/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs b/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs index 0244faec25b..db472e79f53 100644 --- a/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs +++ b/tests/toyos-rust-tests/src/bin/abuse_elf_loader.rs @@ -230,6 +230,8 @@ fn spawn_path(path: &str) -> Result { let argv = format!("{path}\0"); unsafe { syscall::spawn(&SpawnArgs { + path_ptr: path.as_ptr() as u64, + path_len: path.len() as u64, argv_ptr: argv.as_ptr() as u64, argv_len: argv.len() as u64, slot_map_ptr: 0, @@ -283,6 +285,8 @@ fn spawn_image(path: &str, bytes: &[u8]) -> Result { let object = image_object(bytes); unsafe { syscall::spawn(&SpawnArgs { + path_ptr: path.as_ptr() as u64, + path_len: path.len() as u64, argv_ptr: argv.as_ptr() as u64, argv_len: argv.len() as u64, slot_map_ptr: 0, diff --git a/tests/toyos-rust-tests/src/bin/abuse_elf_segments.rs b/tests/toyos-rust-tests/src/bin/abuse_elf_segments.rs index fb9fcc85375..f1d5820ee93 100644 --- a/tests/toyos-rust-tests/src/bin/abuse_elf_segments.rs +++ b/tests/toyos-rust-tests/src/bin/abuse_elf_segments.rs @@ -144,6 +144,8 @@ fn spawn_as(path: &str, image: &[u8]) -> SyscallError { unsafe { core::ptr::copy_nonoverlapping(image.as_ptr(), object.as_ptr(), image.len()) }; unsafe { syscall::spawn(&SpawnArgs { + path_ptr: path.as_ptr() as u64, + path_len: path.len() as u64, argv_ptr: argv.as_ptr() as u64, argv_len: argv.len() as u64, slot_map_ptr: 0, diff --git a/tests/toyos-rust-tests/src/bin/abuse_handle_table.rs b/tests/toyos-rust-tests/src/bin/abuse_handle_table.rs index 5042587c153..997807ac353 100644 --- a/tests/toyos-rust-tests/src/bin/abuse_handle_table.rs +++ b/tests/toyos-rust-tests/src/bin/abuse_handle_table.rs @@ -75,6 +75,8 @@ fn main() { let spawn_with = |slot_map_count: u64, endow_count: u64| { let args = SpawnArgs { + path_ptr: argv as u64, + path_len: (ARGV0.len() - 1) as u64, argv_ptr: argv as u64, argv_len: ARGV0.len() as u64, slot_map_ptr: region as u64, @@ -185,6 +187,8 @@ fn spawn_endowed(entries: &[EndowEntry], labels: &[u8]) -> Result evil(), + Some(ROLE) => role(), _ => attacker(), } } -/// The caller's bytes. Reached only when the supervisor read this binary's image -/// for a launch it resolved to the `toybox` row. -fn evil() -> ! { - // A launched instance of the `toybox` row is endowed that row's `SysCap`; - // a plain child the attacker spawns directly is not. So holding one here is - // proof these bytes ran under the row the caller never named for them. +/// The caller's bytes, run by a launch. +fn role() -> ! { let under_row = Endowments::get().holds(SYSCAP_LABEL); - std::process::exit(if under_row { EXPLOIT } else { 0 }); + std::process::exit(if under_row { EXPLOIT } else { PLAIN }); } fn attacker() { - // Start the symlink on the declared row, so the first resolution names it. let _ = std::fs::remove_file(LINK); - symlink(DECLARED, LINK).expect("create /tmp/x -> the declared row"); + symlink(DECLARED, LINK).expect("create the link at the declared row"); - // Re-point forever between the row's path and this binary's. tmpfs displaces - // the existing entry, so each call is one atomic swap of what `/tmp/x` names. - std::thread::spawn(|| loop { - let _ = symlink(SELF_PATH, LINK); - let _ = symlink(DECLARED, LINK); + let stop = Arc::new(AtomicBool::new(false)); + let flipper = std::thread::spawn({ + let stop = Arc::clone(&stop); + move || { + // tmpfs displaces the existing entry, so each call is one swap of + // what the link names. + while !stop.load(Ordering::Relaxed) { + symlink(SELF_PATH, LINK).expect("re-point the link at this binary"); + symlink(DECLARED, LINK).expect("re-point the link at the declared row"); + } + } }); - let deadline = Instant::now() + RACE_BUDGET; - let mut exploited = false; - let mut attempts = 0u32; - while attempts < MAX_ATTEMPTS && Instant::now() < deadline { - attempts += 1; - // A served working directory, judged by a file server mid-launch, is - // one of the places the base's second resolution waits — widening the - // window the re-point has to land in. - let spawned = Command::new(LINK) - .arg("evil") - .current_dir("/home") + let (mut under_row, mut direct) = (0u32, 0u32); + for attempt in 1..=ATTEMPTS { + let mut child = Command::new(LINK) + .arg(ROLE) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) - .spawn(); - let Ok(mut child) = spawned else { continue }; - if child.wait().ok().and_then(|s| s.code()) == Some(EXPLOIT) { - exploited = true; - break; + .spawn() + .unwrap_or_else(|e| panic!("launch {attempt} of {LINK} was refused: {e}")); + let status = child.wait().unwrap_or_else(|e| panic!("launch {attempt} was not waited: {e}")); + match status.code() { + Some(PS_UNDER_ROW) => under_row += 1, + Some(PS_DIRECT) | Some(PLAIN) => direct += 1, + Some(EXPLOIT) => panic!( + "launch_toctou: launch {attempt} ran the caller's bytes holding the `toybox` \ + row's capability: the program a launched row runs was opened from the \ + caller's path" + ), + other => panic!("launch {attempt} exited {other:?}, which neither program answers"), } } - assert!( - !exploited, - "launch_toctou: the caller's bytes ran holding the `toybox` row's capability after \ - {attempts} launch(es): the supervisor read a declared row's image from the caller's \ - path, re-read after the row was named" - ); - println!("launch_toctou: PASS ({attempts} launches, none ran the caller's bytes under the row)"); + stop.store(true, Ordering::Relaxed); + flipper.join().expect("the re-point thread failed"); + assert!(under_row > 0, "none of {ATTEMPTS} launches ran the `toybox` row's program under its row"); + assert!(direct > 0, "the link never named this binary when the launcher asked in {ATTEMPTS} launches"); + println!("launch_toctou: PASS ({under_row} of {ATTEMPTS} launches ran the row's program under it, none the caller's)"); } diff --git a/tests/toyos-rust-tests/src/bin/process_tree.rs b/tests/toyos-rust-tests/src/bin/process_tree.rs index f79336d5922..1113d5b0fca 100644 --- a/tests/toyos-rust-tests/src/bin/process_tree.rs +++ b/tests/toyos-rust-tests/src/bin/process_tree.rs @@ -18,7 +18,9 @@ //! carries a copy of its place, so std refuses one under a place this process //! cannot duplicate, rather than spawning it directly. The supervisor starts a //! child only by a launch, so std refuses `under_supervisor` for a program no row -//! declares and for a command carrying an extra slot. A chain alternating spawn +//! declares and for a command carrying an extra slot; and a launch runs its row's +//! own program, so std refuses a command naming its image under the supervisor or +//! with a connector provided. A chain alternating spawn //! and launch — each link launches a shell, and the shell spawns the next link //! — stops where the kernel refuses a process more than `MAX_DEPTH` below //! the supervisor, and dies whole with its first link. And a `cat` a shell `detach`es @@ -28,6 +30,7 @@ use std::io::{BufRead, BufReader}; use std::os::toyos::process::{ChildExt, CommandExt}; +use std::path::Path; use std::process::{Child, ChildStdin, Command, Stdio}; use toyos::endow::{self, Endowments, SVC_LABEL, SYSCAP_LABEL}; @@ -212,6 +215,8 @@ fn under_b_is_gone(grown: &Grown) { fn spawn_under(place: RawHandle) -> Result { let argv = format!("{SELF_PATH}\0c"); let args = SpawnArgs { + path_ptr: SELF_PATH.as_ptr() as u64, + path_len: SELF_PATH.len() as u64, argv_ptr: argv.as_ptr() as u64, argv_len: argv.len() as u64, slot_map_ptr: 0, @@ -295,25 +300,30 @@ fn a_place_without_dup_is_refused() { println!(" a place without DUP: the launch is refused, and nothing starts"); } -/// The supervisor is reached only by a launch: std refuses `under_supervisor` for what the -/// launcher would not start under the supervisor, and starts nothing. +/// The supervisor is reached only by a launch of its row's own program: std refuses +/// `under_supervisor` or `provide` for what the launcher would not start, and starts nothing. fn the_supervisor_is_asked_only_by_a_launch() { let undeclared = Command::new(SELF_PATH).arg("c").under_supervisor().spawn(); - refused(undeclared, "a program no row declares"); + refused(undeclared, "under_supervisor for a program no row declares"); let (_read, write) = toyos::pipe_pair().expect("a pipe of our own"); let extra = Command::new(HELD).inherit_handle(5, write.as_handle().0).under_supervisor().spawn(); - refused(extra, "a command carrying an extra slot"); - println!(" the supervisor is asked only by a launch: the two others are refused"); + refused(extra, "under_supervisor for a command carrying an extra slot"); + + let named = Command::new(HELD).image_from(Path::new(HELD)).under_supervisor().spawn(); + refused(named, "under_supervisor for a command naming its image"); + let provided = Command::new(HELD).image_from(Path::new(HELD)).provide("x", write.as_handle().0).spawn(); + refused(provided, "provide for a command naming its image"); + println!(" the supervisor is asked only by a launch of its row's own program: the others are refused"); } fn refused(spawned: std::io::Result, what: &str) { match spawned { Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {} - Err(e) => panic!("under_supervisor for {what} was refused as {e:?}, not PermissionDenied"), + Err(e) => panic!("{what} was refused as {e:?}, not PermissionDenied"), Ok(mut child) => { let _ = child.kill(); - panic!("under_supervisor for {what} started it"); + panic!("{what} started it"); } } } diff --git a/tests/toyos-rust-tests/src/bin/spawn_child_ends_first.rs b/tests/toyos-rust-tests/src/bin/spawn_child_ends_first.rs index 660f7cef2de..06b55b66531 100644 --- a/tests/toyos-rust-tests/src/bin/spawn_child_ends_first.rs +++ b/tests/toyos-rust-tests/src/bin/spawn_child_ends_first.rs @@ -85,6 +85,8 @@ fn hold_the_next_spawn() { fn spawn(role: &str, slot_map: &[[u32; 2]]) -> Result { let argv = format!("{SELF_PATH}\0{role}"); let args = SpawnArgs { + path_ptr: SELF_PATH.as_ptr() as u64, + path_len: SELF_PATH.len() as u64, argv_ptr: argv.as_ptr() as u64, argv_len: argv.len() as u64, slot_map_ptr: slot_map.as_ptr() as u64, diff --git a/tests/toyos-rust-tests/src/bin/spawn_image_object.rs b/tests/toyos-rust-tests/src/bin/spawn_image_object.rs index a4c86de3b8f..983cacfa5f0 100644 --- a/tests/toyos-rust-tests/src/bin/spawn_image_object.rs +++ b/tests/toyos-rust-tests/src/bin/spawn_image_object.rs @@ -33,6 +33,8 @@ fn spawn(image: RawHandle, len: u64) -> Result { // SAFETY: every pointer names a live local for the whole call. unsafe { syscall::spawn(&SpawnArgs { + path_ptr: SELF.as_ptr() as u64, + path_len: SELF.len() as u64, argv_ptr: argv.as_ptr() as u64, argv_len: argv.len() as u64, slot_map_ptr: 0, diff --git a/tests/toyos-rust-tests/src/bin/spawn_lands_claimed.rs b/tests/toyos-rust-tests/src/bin/spawn_lands_claimed.rs index 7cdd26b0aa5..5c7b6a3fde1 100644 --- a/tests/toyos-rust-tests/src/bin/spawn_lands_claimed.rs +++ b/tests/toyos-rust-tests/src/bin/spawn_lands_claimed.rs @@ -76,6 +76,8 @@ fn test() { fn spawn_under(place: RawHandle) -> Result { let argv = format!("{SELF_PATH}\0child"); let args = SpawnArgs { + path_ptr: SELF_PATH.as_ptr() as u64, + path_len: SELF_PATH.len() as u64, argv_ptr: argv.as_ptr() as u64, argv_len: argv.len() as u64, slot_map_ptr: 0, diff --git a/tests/toyos.rs b/tests/toyos.rs index 9a34f5c9a3e..f76c849b38e 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -98,10 +98,9 @@ const RUST_SKIP: &[&str] = &[ // Needs a launcher and a declared `cat` and shell, which `tests/testcases` // does not give: the `process_tree` metal row runs it on tests/proctreecase. "process_tree", - // Needs a launcher and a declared row with a capability a plain job lacks, - // and it is both the attacker and the bytes a re-point substitutes, so its - // own `/system/bin` entry must exist: the `launch_toctou` machine test runs - // it on tests/launchoncecase. + // Needs a launcher and a declared `toybox` holding `roster`, which + // `tests/testcases` does not give: the `launch_toctou` metal row runs it on + // tests/proctreecase. "launch_toctou", // It asserts nothing at all: it holds a `tests/lanleasecase` boot open for // twenty seconds. On a shared boot it would be twenty seconds of nothing. @@ -201,11 +200,6 @@ const MACHINE_TESTS: &[&str] = &[ // no way to turn it back on, so only a machine QEMU reports stopping can // be asked. `machine_soft_off_decoded` reads the T14's own decode. "machine_shutdown", - // A launch resolves a declared row's image from the row's own path, not from - // the caller's. Its own boot, because it needs a launcher and a declared row - // with a capability a plain job lacks, and it hammers hundreds of launches — - // a verdict about which bytes ran, not how fast, so QEMU answers it. - "launch_toctou", ]; /// **The metal profile**: which registrations run on the ThinkPad T14, what @@ -592,6 +586,12 @@ const METAL: &[(&str, metal::Metal)] = &[ "process_tree", metal::Metal { arms: PROCTREECASE, judge: |b| process_tree(b[0]) }, ), + ( + // A launched row runs its own program, never what the caller's path + // names when it is opened again. + "launch_toctou", + metal::Metal { arms: PROCTREECASE, judge: |b| b[0].job_passed("test_rs_launch_toctou") }, + ), // ---- one image: tests/metalcase ---- ( "metal_sim_scanout_wc", @@ -766,9 +766,13 @@ const USB_RESET_BOOTS: &[metal::Arm] = &[ const METALCASE: &[metal::Arm] = &[metal::once("metalcase", "tests/metalcase", &[], &[])]; /// A launcher and a declared `cat` and shell, which `process_tree`'s subtree -/// launches. -const PROCTREECASE: &[metal::Arm] = - &[metal::once("proctreecase", "tests/proctreecase", &[], &["test_rs_process_tree"])]; +/// launches, and a `toybox` row holding `roster`, which `launch_toctou` races. +const PROCTREECASE: &[metal::Arm] = &[metal::once( + "proctreecase", + "tests/proctreecase", + &[], + &["test_rs_process_tree", "test_rs_launch_toctou"], +)]; /// netstack in front of the T14's I219 with its lease probe armed: netstack's exit code /// is the lease's verdict, read out of the kernel's own `exit:` record, and its @@ -2371,49 +2375,10 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { "iommu_virtio_platform" => common::iommu::iommu_virtio_platform(test_config), "nested_nmi_is_loud" => faults::nested_nmi_is_loud(test_config), "machine_shutdown" => power::machine_shutdown(test_config), - "launch_toctou" => launch_toctou(), other => Err(format!("unknown machine test {other}")), } } -/// A declared row's image is read from the row's own path, resolved once. -/// -/// `tests/launchoncecase` gives `toybox` a `roster` capability no plain job -/// holds. `test_rs_launch_toctou` points a `/tmp` symlink at `/system/bin/toybox`, -/// launches it through the launcher while a thread re-points the symlink at its -/// own binary, and asserts its bytes never ran holding that capability. On the -/// base they do — the supervisor resolves `request.program` to the row and then -/// re-reads it for the image — so the job exits non-zero. The verdict is which -/// bytes ran, a content fact QEMU answers; the race is only how it is reached. -fn launch_toctou() -> Result<(), String> { - let profile = qemu::Profile::Headless; - let bin = qemu::build_toyos_bin( - profile.arch(), - &compile::repo_root().join("tests/toyos-rust-tests"), - "launch_toctou", - ); - let options = BootOptions { profile, ..Default::default() }; - let mut qemu = QemuInstance::boot_with_options( - &compile::repo_root().join("tests/launchoncecase"), - &[], - &[("launch_toctou".to_string(), bin)], - options, - ); - serial::Serial::boot(&qemu).must_be_clean()?; - let result = qemu.run_test("test_rs_launch_toctou", Duration::from_secs(120)); - match result.exit_code { - Some(0) => { - eprintln!(" [launch] {}", result.stdout.lines().last().unwrap_or("").trim()); - Ok(()) - } - Some(code) => Err(format!("test_rs_launch_toctou exited {code}\n{}", result.stdout)), - None => Err(format!( - "test_rs_launch_toctou did not finish: {:?}\n{}", - result.error, result.stdout - )), - } -} - /// Every CPU's `CR0` and `CR4`, against what a CPU running this kernel must /// hold. /// diff --git a/toyos-abi/src/syscall.rs b/toyos-abi/src/syscall.rs index 61e5e1a8272..493b2833bde 100644 --- a/toyos-abi/src/syscall.rs +++ b/toyos-abi/src/syscall.rs @@ -287,6 +287,12 @@ pub const WNOHANG: u64 = 1; #[repr(C)] #[derive(Clone, Copy)] pub struct SpawnArgs { + /// The program: the file the kernel opens and pages the child from — its + /// libraries are found beside it — or, with an `image`, the path the caller + /// read that from. **Never `argv[0]`**, which is the child's first argument + /// and the name it goes by, and which nothing opens. + pub path_ptr: u64, + pub path_len: u64, pub argv_ptr: u64, pub argv_len: u64, /// `[[child_slot: u32, parent_handle: RawHandle]]`, duplicated into the @@ -312,13 +318,12 @@ pub struct SpawnArgs { /// The program's bytes, in a shared memory object the caller made and /// read the program into: `image` is a handle to it carrying `MAP`, and /// `image_len` how many of its first bytes the program is — or 0, for a - /// program the kernel opens at `argv[0]` itself. `PermissionDenied` for a + /// program the kernel opens at `path` itself. `PermissionDenied` for a /// handle without `MAP`; `InvalidArgument` for a length the object does not /// hold, or an object that is no memory the kernel allocated. The object /// stays the caller's: what the kernel reads of it, and when, is - /// `kernel/src/file_backing.rs`'s (`SharedImage`). `argv[0]` names the - /// program and is opened by nobody, and its libraries are found in - /// `/system/lib` alone. + /// `kernel/src/file_backing.rs`'s (`SharedImage`). `path` is then opened by + /// nobody, and the libraries are found in `/system/lib` alone. pub image: u64, pub image_len: u64, /// The process the child is placed under, whose end takes it down: a @@ -333,7 +338,7 @@ pub struct SpawnArgs { pub place: u64, } -const _: () = assert!(core::mem::size_of::() == 120); +const _: () = assert!(core::mem::size_of::() == 136); /// One `(label, handle)` pair of a process's endowment table. /// @@ -854,8 +859,7 @@ pub fn get_env(buf: &mut [u8]) -> usize { syscall(SYS_GET_ENV, buf.as_mut_ptr() as u64, buf.len() as u64, 0, 0) as usize } -/// Spawn a new process. The `SpawnArgs` struct contains argv, the slot map, -/// env, the endowments and the working directory the child starts in. +/// Spawn a new process. /// /// Answers a `Process` handle carrying `WAIT|MANAGE|READ|DUP|TRANSFER`. A /// caller that wants nothing to do with the child closes it; a caller that diff --git a/toyos-userbound/src/span.rs b/toyos-userbound/src/span.rs index 0462f49a543..2832d221c3c 100644 --- a/toyos-userbound/src/span.rs +++ b/toyos-userbound/src/span.rs @@ -107,7 +107,7 @@ mod tests { ("Stat", 24, 8), ("SchedInfo", 24, 8), ("FramebufferInfo", 32, 4), - ("SpawnArgs", 120, 8), + ("SpawnArgs", 136, 8), ("NamespaceBuild", 56, 8), ("InboxSetup", 16, 8), ("ProcessStats", 128, 8), diff --git a/userland/supervisor/src/main.rs b/userland/supervisor/src/main.rs index c1065ea8e88..0af8366260f 100644 --- a/userland/supervisor/src/main.rs +++ b/userland/supervisor/src/main.rs @@ -1471,13 +1471,12 @@ impl Supervisor<'_> { } }; - // **`argv[0]` is the caller's path; the image is the row's.** The caller's - // name has to reach the child so an applet learns which it is — - // `/system/bin/echo` run as `ls` is an `ls` — but the bytes come from the - // row's own declared path (`image_from`, below), never from this path - // re-read. `declared` may have reached the row through a caller-writable - // symlink, and loading from here would run whatever that link named at - // load time: the caller's own bytes, under the row's claims. + // **`argv[0]` is the caller's path; the program is the row's.** An applet + // learns which it is from its name — `/system/bin/echo` run as `ls` is an + // `ls` — but the bytes are the row's `path` (`image_from`, below), never + // this one opened again: `declared` may have reached the row through a + // link the caller can re-point, and its own bytes would run under the + // row's claims. let mut command = Command::new(request.program); // **Carried, not inherited.** A child of the launcher would otherwise get // the supervisor's environment and the supervisor's working directory, so `cd /tmp && ls` would @@ -1508,22 +1507,11 @@ impl Supervisor<'_> { let (system, path) = (self.system, request.program.to_string()); let found = self.files("a launch's files", move || { let resolved = resolve(system, &path); - // The image is read from the row's own declared path, resolved once - // (`resolve`/`declared`), never from `request.program` a second time: - // the kernel loads the object this builds rather than re-opening the - // launch's `argv[0]`, so a caller-writable symlink cannot be - // re-pointed between naming the row and loading its bytes. - let row_path = match &resolved { - Resolved::Row(row) => Some(row.path.clone()), - Resolved::Package(row) => Some(row.path.clone()), - Resolved::NotDeclared | Resolved::Refused(_) => None, - }; - let prepared = match row_path { - Some(row_path) => { - command.image_from(Path::new(&row_path)); - command.prepare().map(drop) + let prepared = match &resolved { + Resolved::Row(Program { path, .. }) | Resolved::Package(Program { path, .. }) => { + command.image_from(Path::new(path)).prepare().map(drop) } - None => Ok(()), + Resolved::NotDeclared | Resolved::Refused(_) => Ok(()), }; (resolved, prepared.map(|()| command)) }); From e9a47d0b7ebe2828772d1f0adfca3f1ed76c8625 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 01:03:04 +0200 Subject: [PATCH 3/4] launch_toctou captures its children's output A direct spawn holds no slot it was not given, and toybox's ps writes its refusal to stderr, so a null stderr ended that child with the handle-fault exit before it could answer. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- tests/toyos-rust-tests/src/bin/launch_toctou.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/toyos-rust-tests/src/bin/launch_toctou.rs b/tests/toyos-rust-tests/src/bin/launch_toctou.rs index fa06754f878..ece6849b4b0 100644 --- a/tests/toyos-rust-tests/src/bin/launch_toctou.rs +++ b/tests/toyos-rust-tests/src/bin/launch_toctou.rs @@ -81,15 +81,14 @@ fn attacker() { let (mut under_row, mut direct) = (0u32, 0u32); for attempt in 1..=ATTEMPTS { - let mut child = Command::new(LINK) + // Captured rather than null: a direct spawn holds no slot it was not + // given, and `ps` says why it refused on stderr. + let output = Command::new(LINK) .arg(ROLE) .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - .unwrap_or_else(|e| panic!("launch {attempt} of {LINK} was refused: {e}")); - let status = child.wait().unwrap_or_else(|e| panic!("launch {attempt} was not waited: {e}")); - match status.code() { + .output() + .unwrap_or_else(|e| panic!("launch {attempt} of {LINK} was refused or not waited: {e}")); + match output.status.code() { Some(PS_UNDER_ROW) => under_row += 1, Some(PS_DIRECT) | Some(PLAIN) => direct += 1, Some(EXPLOIT) => panic!( From 05164ccb3aa0694e3c8d243f76b67cb13503b181 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 01:41:38 +0200 Subject: [PATCH 4/4] process_tree: a command naming its image is spawned directly, launcher or not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2's BLOCKER: std counts `image_from` in the routing rule's `decided`, and nothing tested it. The supervisor, the only caller, holds no `launcher`, and `process_tree` covered only the two refusals. The new arm runs `/system/bin/cat` named onto this binary's image from the job, which holds a `launcher` and on whose boot `cat` is declared: only `decided` keeps it from routing as a launch of `cat`'s row. It is judged by which bytes ran — this binary's `named` role exits 42 having checked `argv[0]` is still `cat`; toybox's `cat` of a file `named` exits otherwise. `abuse_spawn_argv` gains the oversized `path_len` arm beside argv and env, and `launch_toctou` loses a `remove_file` that did nothing on a fresh boot. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- .../src/bin/abuse_spawn_argv.rs | 16 ++++++---- .../toyos-rust-tests/src/bin/launch_toctou.rs | 1 - .../toyos-rust-tests/src/bin/process_tree.rs | 30 ++++++++++++++++++- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/tests/toyos-rust-tests/src/bin/abuse_spawn_argv.rs b/tests/toyos-rust-tests/src/bin/abuse_spawn_argv.rs index 76ca5d1c736..c024e634b2e 100644 --- a/tests/toyos-rust-tests/src/bin/abuse_spawn_argv.rs +++ b/tests/toyos-rust-tests/src/bin/abuse_spawn_argv.rs @@ -1,9 +1,9 @@ -//! The kernel must survive a process that hands `SYS_SPAWN` absurd argv/env. +//! The kernel must survive a process that hands `SYS_SPAWN` absurd path/argv/env. //! -//! `argv_len` and `env_len` are raw `u64` fields of a user-supplied -//! `SpawnArgs`. The argv blob is split into a `Vec<&str>` (16 bytes per token) -//! and the env blob is copied with `to_vec()`, so either one sizes a kernel -//! allocation. An argv with no non-empty token also has no `argv[0]`. +//! `path_len`, `argv_len` and `env_len` are raw `u64` fields of a user-supplied +//! `SpawnArgs`. The path is copied, the argv blob is split into a `Vec<&str>` +//! (16 bytes per token) and the env blob is copied with `to_vec()`, so each one +//! sizes a kernel allocation. An argv with no non-empty token also has no `argv[0]`. use std::process::Command; @@ -72,6 +72,10 @@ fn main() { .expect_err("a 2 MiB env blob must be rejected"); assert_eq!(err, SyscallError::InvalidArgument, "wrong error for oversized env"); + let err = unsafe { syscall::spawn(&SpawnArgs { path_len: BLOB as u64, argv_len: 2, ..base }) } + .expect_err("a 2 MiB path must be rejected"); + assert_eq!(err, SyscallError::InvalidArgument, "wrong error for oversized path"); + // The bound is a limit, not a ban: an argv the kernel will accept still // has to reach the loader and fail there for its own reason. let err = unsafe { @@ -97,5 +101,5 @@ fn main() { assert!(status.success(), "/system/bin/echo exited {status:?}"); unsafe { syscall::munmap(region, REGION) }.expect("munmap"); - println!("oversized spawn argv/env rejected, spawn still usable"); + println!("oversized spawn path/argv/env rejected, spawn still usable"); } diff --git a/tests/toyos-rust-tests/src/bin/launch_toctou.rs b/tests/toyos-rust-tests/src/bin/launch_toctou.rs index ece6849b4b0..5b8be7597c1 100644 --- a/tests/toyos-rust-tests/src/bin/launch_toctou.rs +++ b/tests/toyos-rust-tests/src/bin/launch_toctou.rs @@ -63,7 +63,6 @@ fn role() -> ! { } fn attacker() { - let _ = std::fs::remove_file(LINK); symlink(DECLARED, LINK).expect("create the link at the declared row"); let stop = Arc::new(AtomicBool::new(false)); diff --git a/tests/toyos-rust-tests/src/bin/process_tree.rs b/tests/toyos-rust-tests/src/bin/process_tree.rs index 1113d5b0fca..50812b81799 100644 --- a/tests/toyos-rust-tests/src/bin/process_tree.rs +++ b/tests/toyos-rust-tests/src/bin/process_tree.rs @@ -20,7 +20,8 @@ //! child only by a launch, so std refuses `under_supervisor` for a program no row //! declares and for a command carrying an extra slot; and a launch runs its row's //! own program, so std refuses a command naming its image under the supervisor or -//! with a connector provided. A chain alternating spawn +//! with a connector provided, and spawns one naming its image directly, under +//! its program's name, launcher or not. A chain alternating spawn //! and launch — each link launches a shell, and the shell spawns the next link //! — stops where the kernel refuses a process more than `MAX_DEPTH` below //! the supervisor, and dies whole with its first link. And a `cat` a shell `detach`es @@ -58,6 +59,11 @@ const MSG_GROWN: u32 = 1; /// A to B: fault. const MSG_FAULT: u32 = 2; +/// The role of this binary run as [`HELD`] by naming its image, and the exit +/// that says this binary's bytes ran rather than `cat`'s. +const NAMED: &str = "named"; +const NAMED_EXIT: i32 = 42; + /// `process::KILLED_EXIT_CODE`. const KILLED: i32 = 137; /// What `syscall::kill_process(-1)` publishes for a Ring 3 CPU fault. @@ -87,6 +93,10 @@ fn main() { Some("b") => b(), Some("c") => park(), Some("link") => link(args[2].parse().expect("a link's index")), + Some(NAMED) => { + assert_eq!(args[0], HELD, "a command naming its image did not keep its program as argv[0]"); + std::process::exit(NAMED_EXIT) + } Some(other) => panic!("unknown role {other:?}"), None => test(), } @@ -100,6 +110,7 @@ fn test() { a_pipe_is_no_place(); a_place_without_dup_is_refused(); the_supervisor_is_asked_only_by_a_launch(); + a_named_image_is_spawned_directly(); a_chain_stops_at_max_depth_and_dies_whole(); a_detached_program_outlives_its_shell(); println!("process_tree: PASS"); @@ -317,6 +328,23 @@ fn the_supervisor_is_asked_only_by_a_launch() { println!(" the supervisor is asked only by a launch of its row's own program: the others are refused"); } +/// This process holds a `launcher` and `cat` is declared, so only the named +/// image keeps this command from routing as a launch of `cat`'s row. +fn a_named_image_is_spawned_directly() { + let status = Command::new(HELD) + .image_from(Path::new(SELF_PATH)) + .arg(NAMED) + .stdin(Stdio::null()) + .status() + .expect("spawn a command naming its image"); + assert_eq!( + status.code(), + Some(NAMED_EXIT), + "a command naming its image ran something else ({status:?}): it was launched as its row's program" + ); + println!(" a command naming its image runs that image directly, under its program's name"); +} + fn refused(spawned: std::io::Result, what: &str) { match spawned { Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => {}