From 03b3fcfb8b5f8cd1d7b913d02bf2530e86e2a223 Mon Sep 17 00:00:00 2001 From: Rahul Butani Date: Fri, 5 Jun 2026 21:17:07 -0700 Subject: [PATCH] dist: use bubblewrap for overlayFS mounts, run `sccache-dist` without root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the approach here is more or less the same as in #1628: use `CLONE_NEWUSER` (unprivileged user namespace) to be able to create a mount namespace + mount the overlayFS mounts without permissions unlike #1628, this PR leverages [`bubblewrap`'s overlay mount functionality](https://github.com/containers/bubblewrap/commit/f371022ad6af6309cebb4ccb9635b19ed4196ac0) (added in `v0.11.0`) to create the overlayFS mounts at the `sccache-dist` → `bwrap` compile command process boundary. This has a couple of upsides: - sidesteps the [safety issues](https://github.com/mozilla/sccache/blob/9fb6a2542c0d2e82846252be19bb377e5ca8885b/src/bin/sccache-dist/build.rs#L447-L458) in #1628 caused by forking the `sccache-dist` process - avoids the [issue](https://github.com/mozilla/sccache/issues/1688) with overlay fs mounts not being unmounted once the compile command finishes that ultimately led to #1628 being [reverted](https://github.com/mozilla/sccache/pull/1689) + though, I think this issue was actually caused by #1628 changing the default sandboxing codepath (i.e. `current_ns`) to call `CLONE_NEWNS` [in the `sccache-dist`'s `rouille` request handler thread](https://github.com/mozilla/sccache/blob/9fb6a2542c0d2e82846252be19bb377e5ca8885b/src/bin/sccache-dist/build.rs#L422-L429) instead of [in the thread spawned for a compile](https://github.com/mozilla/sccache/blob/c72eed63d78c8550b93133e9fa461a0c6ef7a30d/src/bin/sccache-dist/build.rs#L272-L276) (and not anything to do with the unprivileged user namespace logic) other notes: - previously deleting the build directory/the per-compile directories did not run into permission issues because `sccache-dist` was running as root closes #326, closes #1660 closes #1688 supersedes #2706 --- Cargo.lock | 91 +++++++++-------------------- Cargo.toml | 5 -- docs/DistributedQuickstart.md | 4 +- src/bin/sccache-dist/build.rs | 105 +++++++++++++++++----------------- 4 files changed, 81 insertions(+), 124 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ee4de02216..c83b42bba6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -23,7 +23,7 @@ version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b169f7a6d4742236a0a00c541b845991d0ac43e546831af1249753ab4c3aa3a0" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "cipher", "cpufeatures", ] @@ -198,7 +198,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bb531853791a215d7c62a30daf0dde835f381ab5de4589cfe7c649d2cbe92bd6" dependencies = [ "addr2line", - "cfg-if 1.0.0", + "cfg-if", "libc", "miniz_oxide", "object", @@ -271,7 +271,7 @@ dependencies = [ "arrayref", "arrayvec", "cc", - "cfg-if 1.0.0", + "cfg-if", "constant_time_eq", ] @@ -352,12 +352,6 @@ dependencies = [ "shlex", ] -[[package]] -name = "cfg-if" -version = "0.1.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" - [[package]] name = "cfg-if" version = "1.0.0" @@ -452,7 +446,7 @@ dependencies = [ "getrandom 0.2.11", "glob", "libc", - "nix 0.30.1", + "nix", "serde", "serde_json", "statrs", @@ -491,7 +485,7 @@ version = "4.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d9de586cc7e9752fc232f08e0733c2016122e16065c4adf0c8a8d9e370749ee" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "clap", "codspeed", "condtype", @@ -655,7 +649,7 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b540bd8bc810d3885c6ea91e2018302f68baba2129ab3e88f32389ee9370880d" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", ] [[package]] @@ -664,7 +658,7 @@ version = "0.8.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c3a430a770ebd84726f584a90ee7f020d28db52c6d02138900f22341f866d39c" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", ] [[package]] @@ -810,7 +804,7 @@ version = "0.8.34" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b45de904aa0b010bce2ab45264d0631681847fa7b6f2eaa7dab7619943bc4f59" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", ] [[package]] @@ -864,7 +858,7 @@ version = "0.2.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1ee447700ac8aa0b2f2bd7bc4462ad686ba06baa6727ac149a2d6277f0d240fd" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "libc", "redox_syscall", "windows-sys 0.52.0", @@ -1045,7 +1039,7 @@ version = "0.2.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fe9006bed769170c11f845cf00c7c1e9092aeb3f268e007c3e760ac68008070f" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "js-sys", "libc", "wasi 0.11.0+wasi-snapshot-preview1", @@ -1058,7 +1052,7 @@ version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26145e563e54f2cadc477553f1ec5ee650b00862f0a58bcd12cbdc5f0ea2d2f4" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "js-sys", "libc", "r-efi", @@ -1188,7 +1182,7 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "617aaa3557aef3810a6369d0a99fac8a080891b68bd9f9812a1eeda0c0730cbd" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "libc", "windows-link 0.2.0", ] @@ -1595,17 +1589,6 @@ version = "0.2.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4ec2a862134d2a7d32d7983ddcdd1c4923530833c9f2ea1a44fc5fa473989058" -[[package]] -name = "libmount" -version = "0.1.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23c4c2ad2d5cbd2f5a05620c3daf45930add53ec207fa99ce5eec971089dc35f" -dependencies = [ - "libc", - "nix 0.14.1", - "quick-error", -] - [[package]] name = "libredox" version = "0.1.10" @@ -1662,7 +1645,7 @@ version = "0.10.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d89e7ee0cfbedfc4da3340218492196241d89eefb6dab27de5df917a6d2e78cf" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "digest", ] @@ -1763,19 +1746,6 @@ dependencies = [ "tempfile", ] -[[package]] -name = "nix" -version = "0.14.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c722bee1037d430d0f8e687bbdbf222f27cc6e4e68d5caf630857bb2b6dbdce" -dependencies = [ - "bitflags 1.3.2", - "cc", - "cfg-if 0.1.10", - "libc", - "void", -] - [[package]] name = "nix" version = "0.30.1" @@ -1783,7 +1753,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74523f3a35e05aba87a1d978330aef40f67b0304ac79c1c00b294c9830543db6" dependencies = [ "bitflags 2.9.4", - "cfg-if 1.0.0", + "cfg-if", "cfg_aliases", "libc", ] @@ -1939,7 +1909,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "08838db121398ad17ab8531ce9de97b244589089e290a384c900cb9ff7434328" dependencies = [ "bitflags 2.9.4", - "cfg-if 1.0.0", + "cfg-if", "foreign-types", "libc", "once_cell", @@ -2018,7 +1988,7 @@ version = "0.9.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4c42a9226546d68acdd9c0a280d17ce19bfe27a46bf68784e4066115788d008e" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "libc", "redox_syscall", "smallvec", @@ -2420,7 +2390,7 @@ dependencies = [ "arc-swap", "backon", "bytes", - "cfg-if 1.0.0", + "cfg-if", "combine", "crc16", "futures-channel", @@ -2658,7 +2628,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "70ac5d832aa16abd7d1def883a8545280c20a60f523a370aa3a9617c2b8550ee" dependencies = [ "cc", - "cfg-if 1.0.0", + "cfg-if", "getrandom 0.2.11", "libc", "untrusted", @@ -2714,7 +2684,7 @@ version = "0.21.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0d625ed57d8f49af6cfa514c42e1a71fadcff60eb0b1c517ff82fe41aa025b41" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "ordered-multimap", "trim-in-place", ] @@ -2918,13 +2888,12 @@ dependencies = [ "jobserver", "jsonwebtoken", "libc", - "libmount", "linked-hash-map", "log", "memchr", "memmap2", "mime", - "nix 0.30.1", + "nix", "number_prefix", "object", "opendal", @@ -3145,7 +3114,7 @@ version = "0.10.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e3bf829a2d51ab4a5ddf1352d8470c140cadc8301b2ae1789db023f01cedd6ba" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "cpufeatures", "digest", ] @@ -3162,7 +3131,7 @@ version = "0.10.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "793db75ad2bcafc3ffa7c68b215fee268f537982cd901d132f89c6343f3a3dc8" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "cpufeatures", "digest", ] @@ -3417,7 +3386,7 @@ version = "3.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "85b77fafb263dd9d05cbeac119526425676db3784113aa9295c88498cbf8bff1" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "fastrand", "rustix", "windows-sys 0.52.0", @@ -3463,7 +3432,7 @@ version = "3.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "adcb7fd841cd518e279be3d5a3eb0636409487998a4aff22f3de87b81e88384f" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "proc-macro2", "quote", "syn", @@ -3491,7 +3460,7 @@ dependencies = [ "async-trait", "base64 0.22.1", "bytes", - "cfg-if 1.0.0", + "cfg-if", "const_format", "futures-util", "http", @@ -3971,12 +3940,6 @@ version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" -[[package]] -name = "void" -version = "1.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d" - [[package]] name = "vte" version = "0.11.1" @@ -4055,7 +4018,7 @@ version = "0.2.104" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c1da10c01ae9f1ae40cbfac0bac3b1e724b320abfcf52229f80b547c0d250e2d" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "once_cell", "rustversion", "wasm-bindgen-macro", @@ -4082,7 +4045,7 @@ version = "0.4.39" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac36a15a220124ac510204aec1c3e5db8a22ab06fd6706d881dc6149f8ed9a12" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "js-sys", "wasm-bindgen", "web-sys", diff --git a/Cargo.toml b/Cargo.toml index eb9a759985..3a7a8ee5fe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -151,10 +151,6 @@ thirtyfour = "0.36" [target.'cfg(unix)'.dependencies] daemonix = "0.1" -[target.'cfg(not(target_os = "freebsd"))'.dependencies.libmount] -optional = true -version = "0.1.15" - [target.'cfg(windows)'.dependencies.windows-sys] features = [ "Win32_Foundation", @@ -210,7 +206,6 @@ dist-client = [ dist-server = [ "jwt", "flate2", - "libmount", "nix", "openssl", "reqwest", diff --git a/docs/DistributedQuickstart.md b/docs/DistributedQuickstart.md index 089b1a6614..49dc82e21b 100644 --- a/docs/DistributedQuickstart.md +++ b/docs/DistributedQuickstart.md @@ -93,9 +93,9 @@ type = "jwt_token" token = "my server's token" ``` -Due to bubblewrap requirements currently the build server *must* be run as root. Start the build server by running: +Start the build server by running: ``` -sudo sccache-dist server --config server.conf +sccache-dist server --config server.conf ``` As with the scheduler, if the build server fails to start you may need to set `SCCACHE_LOG=trace` to get useful diagnostics. diff --git a/src/bin/sccache-dist/build.rs b/src/bin/sccache-dist/build.rs index bfd7a980e5..01fd91b2b1 100644 --- a/src/bin/sccache-dist/build.rs +++ b/src/bin/sccache-dist/build.rs @@ -15,7 +15,6 @@ use anyhow::{Context, Error, Result, anyhow, bail}; use flate2::read::GzDecoder; use fs_err as fs; -use libmount::Overlay; use sccache::dist::{ BuildResult, BuilderIncoming, CompileCommand, InputsReader, OutputData, ProcessOutput, TcCache, Toolchain, @@ -23,7 +22,7 @@ use sccache::dist::{ use sccache::lru_disk_cache::Error as LruError; use std::collections::{HashMap, hash_map}; use std::io; -use std::iter; +use std::os::unix::fs::PermissionsExt; use std::path::{self, Path, PathBuf}; use std::process::{ChildStdin, Command, Output, Stdio}; use std::sync::Mutex; @@ -87,6 +86,38 @@ fn join_suffix>(path: &Path, suffix: P) -> PathBuf { path.join(components) } +// The Linux kernel creates subdirectories (within the overlayFS work dir) with +// mode `0o000`: +// - https://github.com/torvalds/linux/blob/c10130c234c81f4a7a143edbf413080235f8d8ce/fs/overlayfs/super.c#L326 +// +// `fs::remove_dir_all` is unable to delete these, hence this function: +fn remove_dir_all_force>(path: P) -> io::Result<()> { + match fs::remove_dir_all(&path) { + Err(e) if e.kind() == io::ErrorKind::PermissionDenied => { + // attempt to change permissions (recursively) and try again: + for ent in walkdir::WalkDir::new(&path) { + let Ok(ent) = ent else { + continue; + }; + + // only attempt to change permissions on directories (we're only + // looking to delete files under `path`, not read them) + if !ent.file_type().is_dir() { + continue; + } + + // equivalent of `ug+rwx`: + let mut perms = ent.metadata()?.permissions(); + perms.set_mode(perms.mode() | 0o770); + fs::set_permissions(ent.path(), perms).unwrap(); + } + + fs::remove_dir_all(&path) + } + other => other, + } +} + #[derive(Debug)] struct OverlaySpec { build_dir: PathBuf, @@ -110,11 +141,6 @@ impl OverlayBuilder { pub fn new(bubblewrap: PathBuf, dir: PathBuf) -> Result { info!("Creating overlay builder"); - if !nix::unistd::getuid().is_root() || !nix::unistd::geteuid().is_root() { - // Not root, or a setuid binary - haven't put enough thought into supporting this, bail - bail!("not running as root") - } - let out = Command::new(&bubblewrap) .arg("--version") .check_stdout_trim() @@ -163,7 +189,7 @@ impl OverlayBuilder { fn cleanup(&self) -> Result<()> { if self.dir.exists() { - fs::remove_dir_all(&self.dir).context("Failed to clean up builder directory")? + remove_dir_all_force(&self.dir).context("Failed to clean up builder directory")? } Ok(()) } @@ -270,49 +296,15 @@ impl OverlayBuilder { std::thread::scope(|scope| { scope .spawn(|| { - // Now mounted filesystems will be automatically unmounted when this thread dies - // (and tmpfs filesystems will be completely destroyed) - nix::sched::unshare(nix::sched::CloneFlags::CLONE_NEWNS) - .context("Failed to enter a new Linux namespace")?; - // Make sure that all future mount changes are private to this namespace - // TODO: shouldn't need to add these annotations - let source: Option<&str> = None; - let fstype: Option<&str> = None; - let data: Option<&str> = None; - // Turn / into a 'slave', so it receives mounts from real root, but doesn't propagate back - nix::mount::mount( - source, - "/", - fstype, - nix::mount::MsFlags::MS_REC | nix::mount::MsFlags::MS_PRIVATE, - data, - ) - .context("Failed to turn / into a slave")?; - let work_dir = overlay.build_dir.join("work"); let upper_dir = overlay.build_dir.join("upper"); - let target_dir = overlay.build_dir.join("target"); fs::create_dir(&work_dir).context("Failed to create overlay work directory")?; fs::create_dir(&upper_dir) .context("Failed to create overlay upper directory")?; - fs::create_dir(&target_dir) - .context("Failed to create overlay target directory")?; - - let () = Overlay::writable( - iter::once(overlay.toolchain_dir.as_path()), - upper_dir, - work_dir, - &target_dir, - // This error is unfortunately not Send+Sync - ) - .mount() - .map_err(|e| anyhow!("Failed to mount overlay FS: {}", e.to_string()))?; trace!("copying in inputs"); - // Note that we don't unpack directly into the upperdir since there overlayfs has some - // special marker files that we don't want to create by accident (or malicious intent) tar::Archive::new(inputs_rdr) - .unpack(&target_dir) + .unpack(&upper_dir) .context("Failed to unpack inputs to overlay")?; let CompileCommand { @@ -324,7 +316,7 @@ impl OverlayBuilder { let cwd = Path::new(&cwd); trace!("creating output directories"); - fs::create_dir_all(join_suffix(&target_dir, cwd)) + fs::create_dir_all(join_suffix(&upper_dir, cwd)) .context("Failed to create cwd")?; for path in output_paths.iter() { // If it doesn't have a parent, nothing needs creating @@ -333,13 +325,12 @@ impl OverlayBuilder { } else { continue; }; - fs::create_dir_all(join_suffix(&target_dir, cwd.join(output_parent))) + fs::create_dir_all(join_suffix(&upper_dir, cwd.join(output_parent))) .context("Failed to create an output directory")?; } trace!("performing compile"); // Bubblewrap notes: - // - We're running as uid 0 (to do the mounts above), and so bubblewrap is run as uid 0 // - There's special handling in bubblewrap to compare uid and euid - of interest to us, // if uid == euid == 0, bubblewrap preserves capabilities (not good!) so we explicitly // drop all capabilities @@ -349,8 +340,8 @@ impl OverlayBuilder { // hurt. // - --unshare-all is not ideal as it happily continues if it fails to unshare either // the user or cgroups namespace, so we list everything explicitly - // - The order of bind vs proc + dev is important - the new root must be put in place - // first, otherwise proc and dev get hidden + // - The order of overlay mounts vs proc + dev is important - the new root must be put in + // place first, otherwise proc and dev get hidden let mut cmd = Command::new(bubblewrap); cmd.arg("--die-with-parent") .args(["--cap-drop", "ALL"]) @@ -362,9 +353,14 @@ impl OverlayBuilder { "--unshare-net", "--unshare-uts", ]) - .arg("--bind") - .arg(&target_dir) - .arg("/") + .args([ + "--overlay-src".as_ref(), + overlay.toolchain_dir.as_os_str(), + "--overlay".as_ref(), + /* RWSRC */ upper_dir.as_os_str(), + /* WORKDIR */ work_dir.as_os_str(), + /* DEST */ "/".as_ref(), + ]) .args(["--proc", "/proc"]) .args(["--dev", "/dev"]) .arg("--chdir") @@ -380,6 +376,7 @@ impl OverlayBuilder { cmd.arg("--"); cmd.arg(executable); cmd.args(arguments); + trace!("bubblewrap invocation: {cmd:?}"); let compile_output = cmd .output() .context("Failed to retrieve output from compile")?; @@ -388,7 +385,9 @@ impl OverlayBuilder { let mut outputs = vec![]; trace!("retrieving {:?}", output_paths); for path in output_paths { - let abspath = join_suffix(&target_dir, cwd.join(&path)); // Resolve in case it's relative since we copy it from the root level + // NOTE: this (resolving as relative to `upper_dir`) presumes that the + // output path is not part of the toolchain.. + let abspath = join_suffix(&upper_dir, cwd.join(&path)); // Resolve in case it's relative since we copy it from the root level match fs::File::open(abspath) { Ok(file) => { let output = OutputData::try_from_reader(file) @@ -428,7 +427,7 @@ impl OverlayBuilder { build_dir, toolchain_dir: _, } = overlay; - if let Err(e) = fs::remove_dir_all(&build_dir) { + if let Err(e) = remove_dir_all_force(&build_dir) { error!( "Failed to remove build directory {}: {}", build_dir.display(),