From 7963bb7fbf8c03b15919da3c17bd2f46eac63fd8 Mon Sep 17 00:00:00 2001 From: cdellacqua Date: Sat, 23 May 2026 10:16:14 +0200 Subject: [PATCH 1/2] bootloader: Run bootupctl via chroot instead of bwrap bwrap unconditionally clones a new user namespace during sandbox setup, and clone(CLONE_NEWUSER) returns EINVAL under qemu-user-mode emulation. That breaks cross-arch installs where bwrap is used to run bootupctl from the target image. Since bootc install already runs as root, the user namespace isn't needed: an unshared mount namespace + chroot is enough to give bootupctl a view of the target image while keeping bind mounts from leaking back to the host. Introduce ChrootCmd in bootc-internal-utils as a sibling to BwrapCmd and wire it into install_via_bootupd and the --filesystem capability probe. The child runs with a cleared environment so the install is not influenced by the buildroot's locale, TMPDIR, etc.; variables it needs are passed explicitly via ChrootCmd::setenv. Fixes https://github.com/bootc-dev/bootc/issues/2111 Assisted-by: Claude Code (Opus 4.7 1M) Signed-off-by: cdellacqua --- crates/lib/src/bootloader.rs | 38 +++--- crates/utils/src/chroot.rs | 249 +++++++++++++++++++++++++++++++++++ crates/utils/src/lib.rs | 2 + 3 files changed, 271 insertions(+), 18 deletions(-) create mode 100644 crates/utils/src/chroot.rs diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 0a40fc5c2..f32420436 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -2,7 +2,7 @@ use std::fs::create_dir_all; use std::process::Command; use anyhow::{Context, Result, anyhow, bail}; -use bootc_utils::{BwrapCmd, CommandRunExt}; +use bootc_utils::{ChrootCmd, CommandRunExt}; use camino::Utf8Path; use cap_std_ext::cap_std::fs::Dir; use cap_std_ext::dirext::CapStdExtDirExt; @@ -93,13 +93,13 @@ pub(crate) fn supports_bootupd(root: &Dir) -> Result { /// Check whether the target bootupd supports `--filesystem`. /// /// Runs `bootupctl backend install --help` and looks for `--filesystem` in the -/// output. When `deployment_path` is set the command runs inside a bwrap -/// container so we probe the binary from the target image. +/// output. When `deployment_path` is set the command runs inside a chroot +/// (via [`ChrootCmd`]) so we probe the binary from the target image. fn bootupd_supports_filesystem(rootfs: &Utf8Path, deployment_path: Option<&str>) -> Result { let help_args = ["bootupctl", "backend", "install", "--help"]; let output = if let Some(deploy) = deployment_path { let target_root = rootfs.join(deploy); - BwrapCmd::new(&target_root) + ChrootCmd::new(&target_root) .set_default_path() .run_get_string(help_args)? } else { @@ -124,7 +124,7 @@ fn bootupd_supports_filesystem(rootfs: &Utf8Path, deployment_path: Option<&str>) /// /// When the target bootupd supports `--filesystem` we pass it pointing at a /// block-backed mount so that bootupd can resolve the backing device(s) itself -/// via `lsblk`. In the bwrap path we bind-mount the physical root at +/// via `lsblk`. In the chroot path we bind-mount the physical root at /// `/sysroot` to give `lsblk` a real block-backed path. /// /// For older bootupd versions that lack `--filesystem` we fall back to the @@ -140,8 +140,8 @@ pub(crate) fn install_via_bootupd( // bootc defaults to only targeting the platform boot method. let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]); - // When not running inside the target container (through `--src-imgref`) we use - // will bwrap as a chroot to run bootupctl from the deployment. + // When not running inside the target container (through `--src-imgref`) we + // run bootupctl from the deployment via a chroot ([`ChrootCmd`]). // This makes sure we use binaries from the target image rather than the buildroot. // In that case, the target rootfs is replaced with `/` because this is just used by // bootupd to find the backing device. @@ -191,21 +191,23 @@ pub(crate) fn install_via_bootupd( bootupd_args.push(rootfs_mount); } - // Run inside a bwrap container. It takes care of mounting and creating - // the necessary API filesystems in the target deployment and acts as - // a nicer `chroot`. + // Run inside a chroot ([`ChrootCmd`]). It sets up a fresh mount + // namespace and the necessary API filesystems in the target + // deployment, without requiring a user namespace (which fails under + // qemu-user — see ). if let Some(deploy) = deployment_path { let target_root = rootfs.join(deploy); let boot_path = rootfs.join("boot"); let rootfs_path = rootfs.to_path_buf(); - tracing::debug!("Running bootupctl via bwrap in {}", target_root); + tracing::debug!("Running bootupctl via chroot in {}", target_root); - // Prepend "bootupctl" to the args for bwrap - let mut bwrap_args = vec!["bootupctl"]; - bwrap_args.extend(bootupd_args); + // Prepend "bootupctl" to the args (ChrootCmd's calling + // convention puts the program in args[0]). + let mut chroot_args = vec!["bootupctl"]; + chroot_args.extend(bootupd_args); - let mut cmd = BwrapCmd::new(&target_root) + let mut cmd = ChrootCmd::new(&target_root) // Bind mount /boot from the physical target root so bootupctl can find // the boot partition and install the bootloader there .bind(&boot_path, &"/boot"); @@ -216,9 +218,9 @@ pub(crate) fn install_via_bootupd( cmd = cmd.bind(&rootfs_path, &"/sysroot"); } - // The $PATH in the bwrap env is not complete enough for some images - // so we inject a reasonable default. - cmd.set_default_path().run(bwrap_args) + // ChrootCmd starts the child with a cleared environment, so we + // inject a default $PATH for it to find sub-tools. + cmd.set_default_path().run(chroot_args) } else { // Running directly without chroot Command::new("bootupctl") diff --git a/crates/utils/src/chroot.rs b/crates/utils/src/chroot.rs new file mode 100644 index 000000000..de497eaa9 --- /dev/null +++ b/crates/utils/src/chroot.rs @@ -0,0 +1,249 @@ +//! Builder for running commands inside a target os tree using a +//! mount namespace + chroot. Requires `CAP_SYS_ADMIN`. + +use std::borrow::Cow; +use std::ffi::{CString, OsStr}; +use std::fs::create_dir_all; +use std::os::unix::process::CommandExt; +use std::process::Command; + +use anyhow::{Context, Result}; +use cap_std_ext::camino::Utf8Path; +use rustix::mount::{MountFlags, MountPropagationFlags, mount, mount_bind_recursive, mount_change}; +use rustix::process::{chdir, chroot}; +use rustix::thread::{UnshareFlags, unshare_unsafe}; + +use crate::CommandRunExt; + +/// Builder for running commands inside a target directory using a +/// mount namespace + chroot. +#[derive(Debug)] +pub struct ChrootCmd<'a> { + /// The target directory to use as root for the chroot. + chroot_path: Cow<'a, Utf8Path>, + /// Bind mounts in format (host source, chroot-relative target). + bind_mounts: Vec<(&'a str, &'a str)>, + /// Environment variables to set on the spawned command. + env_vars: Vec<(&'a str, &'a str)>, +} + +impl<'a> ChrootCmd<'a> { + /// Create a new `ChrootCmd` builder with a root directory. + pub fn new(path: &'a Utf8Path) -> Self { + Self { + chroot_path: Cow::Borrowed(path), + bind_mounts: Vec::new(), + env_vars: Vec::new(), + } + } + + /// Add a bind mount from `source` (on the host) to `target` (a path + /// inside the chroot, e.g. `/boot`). + pub fn bind( + mut self, + source: &'a impl AsRef, + target: &'a impl AsRef, + ) -> Self { + self.bind_mounts + .push((source.as_ref().as_str(), target.as_ref().as_str())); + self + } + + /// Set an environment variable for the child. The chrooted + /// command runs with a cleared environment, isolating it from + /// the buildroot — callers must set every variable they want + /// the child to see. + pub fn setenv(mut self, key: &'a str, value: &'a str) -> Self { + self.env_vars.push((key, value)); + self + } + + /// Set `$PATH` to a reasonable default covering the standard + /// system binary directories. + pub fn set_default_path(self) -> Self { + self.setenv( + "PATH", + "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", + ) + } + + /// Build the underlying [`Command`] with the mount-namespace + /// setup and chroot installed as a `pre_exec` hook. + fn build_command>(self, args: impl IntoIterator) -> Result { + let mut args_iter = args.into_iter(); + let program = args_iter + .next() + .context("ChrootCmd requires the program as the first arg")?; + + // mount() requires its target directories to exist. + let proc_target = self.chroot_path.join("proc"); + let dev_target = self.chroot_path.join("dev"); + let sys_target = self.chroot_path.join("sys"); + let run_target = self.chroot_path.join("run"); + for p in [&proc_target, &dev_target, &sys_target, &run_target] { + create_dir_all(p).with_context(|| format!("Creating {p}"))?; + } + + // Convert paths to CStrings up front so the pre_exec closure + // below stays allocation-free. + let proc_target = CString::new(proc_target.as_str())?; + let dev_target = CString::new(dev_target.as_str())?; + let sys_target = CString::new(sys_target.as_str())?; + let run_target = CString::new(run_target.as_str())?; + + let user_binds: Vec<(CString, CString)> = self + .bind_mounts + .iter() + .map(|(src, tgt)| -> Result<_> { + let tgt_in_chroot = self.chroot_path.join(tgt.trim_start_matches('/')); + create_dir_all(&tgt_in_chroot) + .with_context(|| format!("Creating bind target {tgt_in_chroot}"))?; + Ok((CString::new(*src)?, CString::new(tgt_in_chroot.as_str())?)) + }) + .collect::>()?; + + let chroot_cstr = CString::new(self.chroot_path.as_str())?; + + let mut cmd = Command::new(program); + cmd.args(args_iter); + cmd.env_clear().envs(self.env_vars.iter().copied()); + + // SAFETY: All operations below are safe to invoke between + // fork and exec — only rustix-wrapped syscalls and iteration + // over CStrings allocated above. + #[allow(unsafe_code)] + unsafe { + cmd.pre_exec(move || { + unshare_unsafe(UnshareFlags::NEWNS)?; + + // Recursively mark every mount in our new namespace as + // PRIVATE. This both prevents the mounts we add below + // from leaking back to the host, and ensures that those + // mounts inherit PRIVATE propagation from their parent. + mount_change( + c"/", + MountPropagationFlags::PRIVATE | MountPropagationFlags::REC, + )?; + + // Bind-mount the chroot target onto itself so that `/` + // appears as a real mount point after chroot. Without + // this, tools that inspect mounts (e.g. `findmnt + // --mountpoint /`, which bootupd uses behind + // `--filesystem /`) fail because the chroot dir is a + // plain subdirectory of its parent mount and has no + // mountinfo entry of its own. + mount_bind_recursive(chroot_cstr.as_c_str(), chroot_cstr.as_c_str())?; + + // Setup API filesystems + // See https://systemd.io/API_FILE_SYSTEMS/ + mount( + c"proc", + proc_target.as_c_str(), + c"proc", + MountFlags::empty(), + None, + )?; + mount_bind_recursive(c"/dev", dev_target.as_c_str())?; + mount_bind_recursive(c"/sys", sys_target.as_c_str())?; + // /run carries the udev database, which lsblk/libblkid + // use to resolve partition GUIDs and other device + // properties. + mount_bind_recursive(c"/run", run_target.as_c_str())?; + + for (src, tgt) in &user_binds { + mount_bind_recursive(src.as_c_str(), tgt.as_c_str())?; + } + + chroot(chroot_cstr.as_c_str())?; + chdir(c"/")?; + + Ok(()) + }); + } + + Ok(cmd) + } + + /// Run the specified command inside the chroot, inheriting stdio. + /// `args` must include the program as its first element. + pub fn run>(self, args: impl IntoIterator) -> Result<()> { + self.build_command(args)? + .log_debug() + .run_inherited_with_cmd_context() + } + + /// Run the specified command inside the chroot and capture stdout + /// as a string. `args` must include the program as its first + /// element. + pub fn run_get_string>( + self, + args: impl IntoIterator, + ) -> Result { + self.build_command(args)?.log_debug().run_get_string() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use cap_std_ext::camino::Utf8PathBuf; + + fn tmp_root() -> (tempfile::TempDir, Utf8PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let path = Utf8PathBuf::from_path_buf(dir.path().to_path_buf()).unwrap(); + (dir, path) + } + + #[test] + fn builder_accumulates_binds_and_env() { + let (_keep, root) = tmp_root(); + let src = root.join("src"); + let cmd = ChrootCmd::new(&root) + .bind(&src, &"/boot") + .setenv("FOO", "bar") + .set_default_path(); + assert_eq!(cmd.bind_mounts.len(), 1); + assert_eq!(cmd.bind_mounts[0].1, "/boot"); + // setenv + set_default_path + assert_eq!(cmd.env_vars.len(), 2); + assert!(cmd.env_vars.iter().any(|(k, _)| *k == "PATH")); + assert!(cmd.env_vars.iter().any(|(k, v)| *k == "FOO" && *v == "bar")); + } + + #[test] + fn build_command_creates_api_mount_dirs() { + let (_keep, root) = tmp_root(); + // No user binds — just the API mount targets. + let cmd = ChrootCmd::new(&root).build_command(["/bin/true"]).unwrap(); + for sub in ["proc", "dev", "sys", "run"] { + assert!( + root.join(sub).is_dir(), + "API mount dir {sub} not created in {root}" + ); + } + assert_eq!(cmd.get_program(), "/bin/true"); + } + + #[test] + fn build_command_creates_user_bind_targets() { + let (_keep, root) = tmp_root(); + let (_keep2, src_root) = tmp_root(); + ChrootCmd::new(&root) + .bind(&src_root, &"/sysroot") + .build_command(["/bin/true"]) + .unwrap(); + assert!(root.join("sysroot").is_dir()); + } + + #[test] + fn build_command_rejects_empty_args() { + let (_keep, root) = tmp_root(); + let err = ChrootCmd::new(&root) + .build_command(std::iter::empty::<&str>()) + .unwrap_err(); + assert!( + err.to_string().contains("ChrootCmd requires the program"), + "unexpected error: {err}" + ); + } +} diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index 898ebf386..992215980 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -4,6 +4,8 @@ //! mod bwrap; pub use bwrap::*; +mod chroot; +pub use chroot::*; mod command; pub use command::*; mod iterators; From 1e728f4950b0beeb2559d31aa397698ccb285240 Mon Sep 17 00:00:00 2001 From: cdellacqua Date: Sat, 23 May 2026 10:31:14 +0200 Subject: [PATCH 2/2] utils: Drop unused BwrapCmd The only consumer of BwrapCmd (install_via_bootupd in bootloader.rs) switched to ChrootCmd in the previous commit, so BwrapCmd and the bubblewrap-based execution path have no remaining callers in the workspace. Assisted-by: Claude Code (Opus 4.7 1M) Signed-off-by: cdellacqua --- crates/utils/src/bwrap.rs | 124 -------------------------------------- crates/utils/src/lib.rs | 2 - 2 files changed, 126 deletions(-) delete mode 100644 crates/utils/src/bwrap.rs diff --git a/crates/utils/src/bwrap.rs b/crates/utils/src/bwrap.rs deleted file mode 100644 index 1f0d0a07c..000000000 --- a/crates/utils/src/bwrap.rs +++ /dev/null @@ -1,124 +0,0 @@ -/// Builder for running commands inside a target os tree using bubblewrap (bwrap). -use std::borrow::Cow; -use std::ffi::OsStr; -use std::os::fd::AsRawFd; -use std::process::Command; - -use anyhow::Result; -use cap_std_ext::camino::{Utf8Path, Utf8PathBuf}; -use cap_std_ext::cap_std::fs::Dir; - -use crate::CommandRunExt; - -/// Builder for running commands inside a target directory using bwrap. -#[derive(Debug)] -pub struct BwrapCmd<'a> { - /// The target directory to use as root for the container - chroot_path: Cow<'a, Utf8Path>, - /// Bind mounts in format (source, target) - bind_mounts: Vec<(&'a str, &'a str)>, - /// Environment variables to set - env_vars: Vec<(&'a str, &'a str)>, -} - -impl<'a> BwrapCmd<'a> { - /// Create a new BwrapCmd builder with a root directory as a File Descriptor. - #[allow(dead_code)] - pub fn new_with_dir(path: &'a Dir) -> Self { - let fd_path: String = format!("/proc/self/fd/{}", path.as_raw_fd()); - Self { - chroot_path: Cow::Owned(Utf8PathBuf::from(&fd_path)), - bind_mounts: Vec::new(), - env_vars: Vec::new(), - } - } - - /// Create a new BwrapCmd builder with a root directory - pub fn new(path: &'a Utf8Path) -> Self { - Self { - chroot_path: Cow::Borrowed(path), - bind_mounts: Vec::new(), - env_vars: Vec::new(), - } - } - - /// Add a bind mount from source to target inside the container. - pub fn bind( - mut self, - source: &'a impl AsRef, - target: &'a impl AsRef, - ) -> Self { - self.bind_mounts - .push((source.as_ref().as_str(), target.as_ref().as_str())); - self - } - - /// Set an environment variable for the command. - pub fn setenv(mut self, key: &'a str, value: &'a str) -> Self { - self.env_vars.push((key, value)); - self - } - - /// Set $PATH to a reasonable default for finding system binaries. - /// - /// The bwrap environment may not have a complete $PATH, causing - /// tools like bootupctl or sfdisk to not be found. This sets a - /// default that covers the standard binary directories. - pub fn set_default_path(self) -> Self { - self.setenv( - "PATH", - "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin", - ) - } - - /// Build the bwrap `Command` with all bind mounts, env vars, and args. - fn build_command>(&self, args: impl IntoIterator) -> Command { - let mut cmd = Command::new("bwrap"); - - // Bind the root filesystem - cmd.args(["--bind", self.chroot_path.as_str(), "/"]); - - // Setup API filesystems - // See https://systemd.io/API_FILE_SYSTEMS/ - cmd.args(["--proc", "/proc"]); - cmd.args(["--dev-bind", "/dev", "/dev"]); - cmd.args(["--bind", "/sys", "/sys"]); - - // Bind /run primarily for the udev database so that - // lsblk/libblkid inside the sandbox can read - // partition type GUIDs and other device properties. - cmd.args(["--tmpfs", "/run"]); - cmd.args(["--bind", "/run", "/run"]); - - // Add bind mounts - for (source, target) in &self.bind_mounts { - cmd.args(["--bind", source, target]); - } - - // Add environment variables - for (key, value) in &self.env_vars { - cmd.args(["--setenv", key, value]); - } - - // Command to run - cmd.arg("--"); - cmd.args(args); - - cmd - } - - /// Run the specified command inside the container. - pub fn run>(self, args: impl IntoIterator) -> Result<()> { - self.build_command(args) - .log_debug() - .run_inherited_with_cmd_context() - } - - /// Run the specified command inside the container and capture stdout as a string. - pub fn run_get_string>( - self, - args: impl IntoIterator, - ) -> Result { - self.build_command(args).log_debug().run_get_string() - } -} diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index 992215980..edd15c9e8 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -2,8 +2,6 @@ //! things here that only depend on the standard library and //! "core" crates. //! -mod bwrap; -pub use bwrap::*; mod chroot; pub use chroot::*; mod command;