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..b0a18d188a7 --- /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. + +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. + +**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. + +**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 a0d44493347..724238524f4 160000 --- a/rust +++ b/rust @@ -1 +1 @@ -Subproject commit a0d44493347e5be082ed4b7507b53537bab2c884 +Subproject commit 724238524f4dcddc202ffa570cf034c512fe3e1d 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` (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; @@ -36,6 +36,8 @@ fn main() { } let base = SpawnArgs { + path_ptr: region as u64, + path_len: 1, argv_ptr: region as u64, argv_len: 0, slot_map_ptr: 0, @@ -70,12 +72,16 @@ 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 { syscall::spawn(&SpawnArgs { argv_len: 2, ..base }) } - .expect_err("argv[0] = \"a\" is not a program"); + .expect_err("\"a\" is not a program"); assert_eq!(err, SyscallError::NotFound, "wrong error for a short honest argv"); // No non-empty token: there is no argv[0] to load. @@ -95,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 new file mode 100644 index 00000000000..5b8be7597c1 --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/launch_toctou.rs @@ -0,0 +1,107 @@ +//! A launched row runs the row's own program, never the bytes a caller's path +//! names when it is opened again. +//! +//! The launcher names a `[programs]` row from the caller's path, following one +//! link, and the row's capabilities go to whatever it then starts. This binary +//! points a `/tmp` link at `/system/bin/toybox` — a row this boot gives the +//! `roster` capability — launches it under the name `ps` while a second thread +//! re-points the link between toybox and this binary, and judges every child by +//! its exit: +//! +//! - toybox's `ps` exits 0 only holding a `SysCap` with `ROSTER`, which only the +//! row gives: the row's own program under the row; +//! - `ps` without one exits 1: the link named this binary when the launcher +//! asked, so the caller spawned it directly, and it named toybox again when the +//! kernel opened it; +//! - this binary as [`ROLE`] exits [`PLAIN`] without a `SysCap` and [`EXPLOIT`] +//! with one — the caller's bytes under the row's claims, the defect. +//! +//! Every launch must start and be waited, and both of the first two must happen: +//! one shows a launch of the row runs, the other that the link really moved +//! while launches were made. + +use std::os::toyos::fs::symlink; +use std::process::{Command, Stdio}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +use toyos::endow::{Endowments, SYSCAP_LABEL}; + +/// This binary, the bytes a re-point substitutes for the row's. +const SELF_PATH: &str = "/system/bin/test_rs_launch_toctou"; +/// The declared row the link names. `/system` is read-only to every program, +/// so only which file the link names can move, never the row's bytes. +const DECLARED: &str = "/system/bin/toybox"; +/// The caller-writable link, named for the applet toybox runs as. +const LINK: &str = "/tmp/ps"; +/// The argument that makes a child of this binary [`role`] rather than the +/// attacker. +const ROLE: &str = "role"; + +/// This binary as a child, holding a `SysCap`: only a launch of the row gives one. +const EXPLOIT: i32 = 123; +/// This binary as a child, holding none. +const PLAIN: i32 = 77; +/// Toybox's `ps`, having read the roster. +const PS_UNDER_ROW: i32 = 0; +/// Toybox's `ps`, endowed no `SysCap`. +const PS_DIRECT: i32 = 1; + +const ATTEMPTS: u32 = 100; + +fn main() { + match std::env::args().nth(1).as_deref() { + Some(ROLE) => role(), + _ => attacker(), + } +} + +/// 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 { PLAIN }); +} + +fn attacker() { + symlink(DECLARED, LINK).expect("create the link at the declared row"); + + 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 (mut under_row, mut direct) = (0u32, 0u32); + for attempt in 1..=ATTEMPTS { + // 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()) + .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!( + "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"), + } + } + + 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..50812b81799 100644 --- a/tests/toyos-rust-tests/src/bin/process_tree.rs +++ b/tests/toyos-rust-tests/src/bin/process_tree.rs @@ -18,7 +18,10 @@ //! 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, 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 @@ -28,6 +31,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}; @@ -55,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. @@ -84,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(), } @@ -97,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"); @@ -212,6 +226,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 +311,47 @@ 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"); +} + +/// 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 => {} - 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 486edcdcd90..f76c849b38e 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -98,6 +98,10 @@ 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 `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. "lan_hold", @@ -582,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", @@ -756,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 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 4d3548aa508..0af8366260f 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,12 @@ 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 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 @@ -1504,8 +1507,10 @@ 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), + let prepared = match &resolved { + Resolved::Row(Program { path, .. }) | Resolved::Package(Program { path, .. }) => { + command.image_from(Path::new(path)).prepare().map(drop) + } Resolved::NotDeclared | Resolved::Refused(_) => Ok(()), }; (resolved, prepared.map(|()| command))