diff --git a/Cargo.lock b/Cargo.lock index c3c3ea6..e41f7e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -79,6 +79,12 @@ version = "2.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4388bee8683e3d04af747c73422af53102d2bd24d9eadb6cbc100baef4b43f8" +[[package]] +name = "bytes" +version = "1.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ae3f5d315924270530207e2a68396c3cc547f6dca3fbdca317cfb1a51edb593" + [[package]] name = "cfg-if" version = "1.0.4" @@ -285,6 +291,17 @@ version = "2.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "88904434abc2901f197fe8cc55f0445e7ded921dba5911dad2e2b39b48e663c4" +[[package]] +name = "mio" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "02bd0af71c67b473010cbbc60715ee815645a4dc942899111f494b4b737d6fda" +dependencies = [ + "libc", + "wasi", + "windows-sys", +] + [[package]] name = "mktemp-rs" version = "0.2.0" @@ -448,6 +465,16 @@ dependencies = [ "zmij", ] +[[package]] +name = "signal-hook-registry" +version = "1.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4db69cba1110affc0e9f7bcd48bbf87b3f4fc7c61fc9155afd4c469eb3d6c1b" +dependencies = [ + "errno", + "libc", +] + [[package]] name = "strsim" version = "0.11.1" @@ -521,7 +548,12 @@ version = "1.52.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8fc7f01b389ac15039e4dc9531aa973a135d7a4135281b12d7c1bc79fd57fffe" dependencies = [ + "bytes", + "libc", + "mio", "pin-project-lite", + "signal-hook-registry", + "windows-sys", ] [[package]] @@ -536,6 +568,12 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" +[[package]] +name = "wasi" +version = "0.11.1+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" + [[package]] name = "windows-link" version = "0.2.1" diff --git a/Cargo.toml b/Cargo.toml index 5588884..382a31a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ tokio = { version = "1.52.3", optional = true } tempfile = "3" [features] -async = ["dep:tokio"] +async = ["dep:tokio", "tokio/process"] [lib] name = "styrolite" diff --git a/bin/styrojail.rs b/bin/styrojail.rs index b63bb3d..f158def 100644 --- a/bin/styrojail.rs +++ b/bin/styrojail.rs @@ -1,6 +1,6 @@ use std::{env, path::PathBuf}; -use anyhow::{Result, anyhow}; +use anyhow::{Result, anyhow, bail}; use clap::Parser; use styrolite::config::{IdMapping, MountSpec as StyroMountSpec}; use styrolite::namespace::Namespace; @@ -207,5 +207,10 @@ fn main() -> Result<()> { let runner = Runner::new(&cli.styrolite_bin); runner.exec(req)?; - Err(anyhow!("styrolite exec failed")) + // exec() replaces this process image on success and returns Err on + // failure (propagated above), so reaching here is never expected. + bail!( + "styrolite runner '{}' returned without exec'ing the workload", + cli.styrolite_bin + ) } diff --git a/bin/styrolite.rs b/bin/styrolite.rs index 7f17084..ab3cc89 100644 --- a/bin/styrolite.rs +++ b/bin/styrolite.rs @@ -1,22 +1,27 @@ use std::{env, fs, path::PathBuf}; -use anyhow::Result; +use anyhow::{Context, Result, bail}; use env_logger::{Env, fmt::TimestampPrecision}; use styrolite::config::{Config, Wrappable}; fn main() -> Result<()> { - let config_path = env::args().nth(1).expect("config file path missing"); + let Some(config_path) = env::args().nth(1) else { + bail!("missing config file path (usage: styrolite )"); + }; let config_path = PathBuf::from(config_path); if !config_path.exists() { - panic!("config file did not exist"); + bail!("config file '{}' does not exist", config_path.display()); } env_logger::Builder::from_env(Env::default().default_filter_or("info")) .format_timestamp(Some(TimestampPrecision::Micros)) .init(); - let config: Config = serde_json::from_slice(&fs::read(&config_path)?)?; + let raw = fs::read(&config_path) + .with_context(|| format!("failed to read config file '{}'", config_path.display()))?; + let config: Config = serde_json::from_slice(&raw) + .with_context(|| format!("failed to parse config file '{}'", config_path.display()))?; match config { Config::Create(create) => create.wrap(), Config::Attach(attach) => attach.wrap(), diff --git a/src/caps.rs b/src/caps.rs index 83780a4..c63d1b1 100644 --- a/src/caps.rs +++ b/src/caps.rs @@ -325,7 +325,7 @@ pub struct CapResult { fn capget(result: &mut CapInternalResult) -> anyhow::Result<()> { unsafe { if syscall(libc::SYS_capget, &result.header, &result.data) < 0 { - Err(anyhow!("capget(2) failed")) + Err(anyhow!("capget(2) failed: {}", io::Error::last_os_error())) } else { Ok(()) } @@ -334,8 +334,8 @@ fn capget(result: &mut CapInternalResult) -> anyhow::Result<()> { fn capset(result: &CapInternalResult) -> anyhow::Result<()> { unsafe { - let err = syscall(libc::SYS_capset, &result.header, &result.data); - if err < 0 { + if syscall(libc::SYS_capset, &result.header, &result.data) < 0 { + let os_err = io::Error::last_os_error(); let effective = ((result.data[0].effective as u64) << 32) | result.data[1].effective as u64; let permitted = @@ -344,10 +344,8 @@ fn capset(result: &CapInternalResult) -> anyhow::Result<()> { ((result.data[0].inheritable as u64) << 32) | result.data[1].inheritable as u64; Err(anyhow!( - "capset(2) failed: {:x} {:x} {:x} (error = {err})", - effective, - permitted, - inheritable + "capset(2) failed (effective={effective:x} permitted={permitted:x} \ + inheritable={inheritable:x}): {os_err}" )) } else { Ok(()) diff --git a/src/mount.rs b/src/mount.rs index 1db8f38..eca9ebe 100644 --- a/src/mount.rs +++ b/src/mount.rs @@ -270,7 +270,15 @@ impl Mountable for MountSpec { let data_cstr = self .data .as_ref() - .map(|d| CString::new(d.as_str()).unwrap()); + .map(|d| { + CString::new(d.as_str()).map_err(|e| { + anyhow!( + "mount data '{d}' for {} contains an interior NUL byte: {e}", + self.target + ) + }) + }) + .transpose()?; let data_ptr = data_cstr .as_ref() .map(|c| c.as_ptr() as *const libc::c_void) @@ -330,11 +338,11 @@ impl Mountable for MountSpec { unsafe { if libc::syscall(libc::SYS_pivot_root, dot_p, dot_p) < 0 { - bail!("unable to pivot root"); + bail!("failed to pivot_root: {}", io::Error::last_os_error()); } if libc::umount2(dot_p, libc::MNT_DETACH) < 0 { - bail!("unable to unmount old root"); + bail!("failed to unmount old root: {}", io::Error::last_os_error()); } } diff --git a/src/runner.rs b/src/runner.rs index a99d17e..d47643b 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -430,7 +430,10 @@ impl Runner { return Ok(code); } - Err(anyhow!("failed to launch/monitor child process")) + Err(anyhow!( + "styrolite runner '{}' did not exit normally: {status}", + self.executable + )) } #[cfg(feature = "async")] @@ -443,7 +446,10 @@ impl Runner { return Ok(code); } - Err(anyhow!("failed to launch/monitor child process")) + Err(anyhow!( + "styrolite runner '{}' did not exit normally: {status}", + self.executable + )) } /// Replace the current process with the styrolite runner directly. @@ -459,7 +465,10 @@ impl Runner { // That means config_file won't be dropped, so a drop-based cleanup won't happen. // If TempFile is delete-on-drop, the file may be left behind. let err = command.exec(); // only returns on failure - Err(anyhow!(err)) + Err(anyhow!( + "failed to exec styrolite runner '{}': {err}", + self.executable + )) } #[cfg(not(unix))] diff --git a/src/wrap.rs b/src/wrap.rs index 2a102d7..ec92b84 100644 --- a/src/wrap.rs +++ b/src/wrap.rs @@ -53,7 +53,10 @@ fn set_process_limit(resource: RLimit, limit: Option) -> Result<()> { unsafe { if libc::setrlimit(resource, &rlimit) == -1 { - Err(anyhow!("failed to set resource limit")) + Err(anyhow!( + "failed to set resource limit {resource}: {}", + Error::last_os_error() + )) } else { Ok(()) } @@ -217,20 +220,22 @@ impl CreateRequest { } fn update_hostname(&self) -> Result<()> { - let wid = self - .identity() - .expect("unable to determine a workload identity"); + let wid = self.identity()?; let final_hostname = match &self.hostname { Some(hostname) => hostname.to_string(), None => format!("styrolite-{wid}"), }; - let final_hostname_cstr = - CString::new(final_hostname).expect("unable to parse hostname as valid C string"); + let final_hostname_cstr = CString::new(final_hostname.clone()).map_err(|e| { + anyhow!("hostname '{final_hostname}' contains an interior NUL byte: {e}") + })?; let final_hostname_ptr = final_hostname_cstr.as_ptr(); unsafe { if libc::sethostname(final_hostname_ptr, final_hostname_cstr.count_bytes()) < 0 { - Err(anyhow!("failed to set hostname")) + Err(anyhow!( + "failed to set hostname to '{final_hostname}': {}", + Error::last_os_error() + )) } else { Ok(()) } @@ -679,7 +684,7 @@ impl ExecutableSpec { let executable = self .executable .clone() - .expect("expected executable to be configured"); + .ok_or_else(|| anyhow!("no executable configured for the workload to run"))?; let program_cstring = CString::new(executable)?; let mut args_cstrings: Vec<_> = if let Some(args) = &self.arguments { @@ -732,7 +737,31 @@ impl ExecutableSpec { env_charptrs.as_ptr(), ) < 0 { - Err(anyhow!("execvpe failed")) + // execvpe only returns on failure. Capture errno immediately + // (before any other libc call can clobber it) and translate it + // into an actionable message. "execvpe failed" with no detail + // has repeatedly sent people looking in the wrong place. + let err = Error::last_os_error(); + let program = program_cstring.to_string_lossy(); + let hint = match err.raw_os_error() { + Some(libc::ENOENT) => format!( + " (is '{program}' installed and on PATH? if you meant to \ + pass it as an argument, check the order of the executable \ + and its arguments)" + ), + Some(libc::EACCES) => { + format!( + " (is '{program}' marked executable, and are all \ + leading path components accessible?)" + ) + } + Some(libc::ENOEXEC) => format!( + " (is '{program}' a valid executable for this architecture, \ + or is it a script missing a #! interpreter line?)" + ), + _ => String::new(), + }; + Err(anyhow!("failed to execute '{program}': {err}{hint}")) } else { Ok(()) }