Skip to content

Commit c0ff1c9

Browse files
cdellacquacgwalters
authored andcommitted
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 bootc-dev#2111 Assisted-by: Claude Code (Opus 4.7 1M) Signed-off-by: cdellacqua <carlo.dellacqua97@gmail.com>
1 parent e4dea2b commit c0ff1c9

3 files changed

Lines changed: 271 additions & 18 deletions

File tree

crates/lib/src/bootloader.rs

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::fs::create_dir_all;
22
use std::process::Command;
33

44
use anyhow::{Context, Result, anyhow, bail};
5-
use bootc_utils::{BwrapCmd, CommandRunExt};
5+
use bootc_utils::{ChrootCmd, CommandRunExt};
66
use camino::Utf8Path;
77
use cap_std_ext::cap_std::fs::Dir;
88
use cap_std_ext::dirext::CapStdExtDirExt;
@@ -93,13 +93,13 @@ pub(crate) fn supports_bootupd(root: &Dir) -> Result<bool> {
9393
/// Check whether the target bootupd supports `--filesystem`.
9494
///
9595
/// Runs `bootupctl backend install --help` and looks for `--filesystem` in the
96-
/// output. When `deployment_path` is set the command runs inside a bwrap
97-
/// container so we probe the binary from the target image.
96+
/// output. When `deployment_path` is set the command runs inside a chroot
97+
/// (via [`ChrootCmd`]) so we probe the binary from the target image.
9898
fn bootupd_supports_filesystem(rootfs: &Utf8Path, deployment_path: Option<&str>) -> Result<bool> {
9999
let help_args = ["bootupctl", "backend", "install", "--help"];
100100
let output = if let Some(deploy) = deployment_path {
101101
let target_root = rootfs.join(deploy);
102-
BwrapCmd::new(&target_root)
102+
ChrootCmd::new(&target_root)
103103
.set_default_path()
104104
.run_get_string(help_args)?
105105
} else {
@@ -124,7 +124,7 @@ fn bootupd_supports_filesystem(rootfs: &Utf8Path, deployment_path: Option<&str>)
124124
///
125125
/// When the target bootupd supports `--filesystem` we pass it pointing at a
126126
/// block-backed mount so that bootupd can resolve the backing device(s) itself
127-
/// via `lsblk`. In the bwrap path we bind-mount the physical root at
127+
/// via `lsblk`. In the chroot path we bind-mount the physical root at
128128
/// `/sysroot` to give `lsblk` a real block-backed path.
129129
///
130130
/// For older bootupd versions that lack `--filesystem` we fall back to the
@@ -140,8 +140,8 @@ pub(crate) fn install_via_bootupd(
140140
// bootc defaults to only targeting the platform boot method.
141141
let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]);
142142

143-
// When not running inside the target container (through `--src-imgref`) we use
144-
// will bwrap as a chroot to run bootupctl from the deployment.
143+
// When not running inside the target container (through `--src-imgref`) we
144+
// run bootupctl from the deployment via a chroot ([`ChrootCmd`]).
145145
// This makes sure we use binaries from the target image rather than the buildroot.
146146
// In that case, the target rootfs is replaced with `/` because this is just used by
147147
// bootupd to find the backing device.
@@ -191,21 +191,23 @@ pub(crate) fn install_via_bootupd(
191191
bootupd_args.push(rootfs_mount);
192192
}
193193

194-
// Run inside a bwrap container. It takes care of mounting and creating
195-
// the necessary API filesystems in the target deployment and acts as
196-
// a nicer `chroot`.
194+
// Run inside a chroot ([`ChrootCmd`]). It sets up a fresh mount
195+
// namespace and the necessary API filesystems in the target
196+
// deployment, without requiring a user namespace (which fails under
197+
// qemu-user — see <https://github.com/bootc-dev/bootc/issues/2111>).
197198
if let Some(deploy) = deployment_path {
198199
let target_root = rootfs.join(deploy);
199200
let boot_path = rootfs.join("boot");
200201
let rootfs_path = rootfs.to_path_buf();
201202

202-
tracing::debug!("Running bootupctl via bwrap in {}", target_root);
203+
tracing::debug!("Running bootupctl via chroot in {}", target_root);
203204

204-
// Prepend "bootupctl" to the args for bwrap
205-
let mut bwrap_args = vec!["bootupctl"];
206-
bwrap_args.extend(bootupd_args);
205+
// Prepend "bootupctl" to the args (ChrootCmd's calling
206+
// convention puts the program in args[0]).
207+
let mut chroot_args = vec!["bootupctl"];
208+
chroot_args.extend(bootupd_args);
207209

208-
let mut cmd = BwrapCmd::new(&target_root)
210+
let mut cmd = ChrootCmd::new(&target_root)
209211
// Bind mount /boot from the physical target root so bootupctl can find
210212
// the boot partition and install the bootloader there
211213
.bind(&boot_path, &"/boot");
@@ -216,9 +218,9 @@ pub(crate) fn install_via_bootupd(
216218
cmd = cmd.bind(&rootfs_path, &"/sysroot");
217219
}
218220

219-
// The $PATH in the bwrap env is not complete enough for some images
220-
// so we inject a reasonable default.
221-
cmd.set_default_path().run(bwrap_args)
221+
// ChrootCmd starts the child with a cleared environment, so we
222+
// inject a default $PATH for it to find sub-tools.
223+
cmd.set_default_path().run(chroot_args)
222224
} else {
223225
// Running directly without chroot
224226
Command::new("bootupctl")

crates/utils/src/chroot.rs

Lines changed: 249 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,249 @@
1+
//! Builder for running commands inside a target os tree using a
2+
//! mount namespace + chroot. Requires `CAP_SYS_ADMIN`.
3+
4+
use std::borrow::Cow;
5+
use std::ffi::{CString, OsStr};
6+
use std::fs::create_dir_all;
7+
use std::os::unix::process::CommandExt;
8+
use std::process::Command;
9+
10+
use anyhow::{Context, Result};
11+
use cap_std_ext::camino::Utf8Path;
12+
use rustix::mount::{MountFlags, MountPropagationFlags, mount, mount_bind_recursive, mount_change};
13+
use rustix::process::{chdir, chroot};
14+
use rustix::thread::{UnshareFlags, unshare_unsafe};
15+
16+
use crate::CommandRunExt;
17+
18+
/// Builder for running commands inside a target directory using a
19+
/// mount namespace + chroot.
20+
#[derive(Debug)]
21+
pub struct ChrootCmd<'a> {
22+
/// The target directory to use as root for the chroot.
23+
chroot_path: Cow<'a, Utf8Path>,
24+
/// Bind mounts in format (host source, chroot-relative target).
25+
bind_mounts: Vec<(&'a str, &'a str)>,
26+
/// Environment variables to set on the spawned command.
27+
env_vars: Vec<(&'a str, &'a str)>,
28+
}
29+
30+
impl<'a> ChrootCmd<'a> {
31+
/// Create a new `ChrootCmd` builder with a root directory.
32+
pub fn new(path: &'a Utf8Path) -> Self {
33+
Self {
34+
chroot_path: Cow::Borrowed(path),
35+
bind_mounts: Vec::new(),
36+
env_vars: Vec::new(),
37+
}
38+
}
39+
40+
/// Add a bind mount from `source` (on the host) to `target` (a path
41+
/// inside the chroot, e.g. `/boot`).
42+
pub fn bind(
43+
mut self,
44+
source: &'a impl AsRef<Utf8Path>,
45+
target: &'a impl AsRef<Utf8Path>,
46+
) -> Self {
47+
self.bind_mounts
48+
.push((source.as_ref().as_str(), target.as_ref().as_str()));
49+
self
50+
}
51+
52+
/// Set an environment variable for the child. The chrooted
53+
/// command runs with a cleared environment, isolating it from
54+
/// the buildroot — callers must set every variable they want
55+
/// the child to see.
56+
pub fn setenv(mut self, key: &'a str, value: &'a str) -> Self {
57+
self.env_vars.push((key, value));
58+
self
59+
}
60+
61+
/// Set `$PATH` to a reasonable default covering the standard
62+
/// system binary directories.
63+
pub fn set_default_path(self) -> Self {
64+
self.setenv(
65+
"PATH",
66+
"/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin",
67+
)
68+
}
69+
70+
/// Build the underlying [`Command`] with the mount-namespace
71+
/// setup and chroot installed as a `pre_exec` hook.
72+
fn build_command<S: AsRef<OsStr>>(self, args: impl IntoIterator<Item = S>) -> Result<Command> {
73+
let mut args_iter = args.into_iter();
74+
let program = args_iter
75+
.next()
76+
.context("ChrootCmd requires the program as the first arg")?;
77+
78+
// mount() requires its target directories to exist.
79+
let proc_target = self.chroot_path.join("proc");
80+
let dev_target = self.chroot_path.join("dev");
81+
let sys_target = self.chroot_path.join("sys");
82+
let run_target = self.chroot_path.join("run");
83+
for p in [&proc_target, &dev_target, &sys_target, &run_target] {
84+
create_dir_all(p).with_context(|| format!("Creating {p}"))?;
85+
}
86+
87+
// Convert paths to CStrings up front so the pre_exec closure
88+
// below stays allocation-free.
89+
let proc_target = CString::new(proc_target.as_str())?;
90+
let dev_target = CString::new(dev_target.as_str())?;
91+
let sys_target = CString::new(sys_target.as_str())?;
92+
let run_target = CString::new(run_target.as_str())?;
93+
94+
let user_binds: Vec<(CString, CString)> = self
95+
.bind_mounts
96+
.iter()
97+
.map(|(src, tgt)| -> Result<_> {
98+
let tgt_in_chroot = self.chroot_path.join(tgt.trim_start_matches('/'));
99+
create_dir_all(&tgt_in_chroot)
100+
.with_context(|| format!("Creating bind target {tgt_in_chroot}"))?;
101+
Ok((CString::new(*src)?, CString::new(tgt_in_chroot.as_str())?))
102+
})
103+
.collect::<Result<_>>()?;
104+
105+
let chroot_cstr = CString::new(self.chroot_path.as_str())?;
106+
107+
let mut cmd = Command::new(program);
108+
cmd.args(args_iter);
109+
cmd.env_clear().envs(self.env_vars.iter().copied());
110+
111+
// SAFETY: All operations below are safe to invoke between
112+
// fork and exec — only rustix-wrapped syscalls and iteration
113+
// over CStrings allocated above.
114+
#[allow(unsafe_code)]
115+
unsafe {
116+
cmd.pre_exec(move || {
117+
unshare_unsafe(UnshareFlags::NEWNS)?;
118+
119+
// Recursively mark every mount in our new namespace as
120+
// PRIVATE. This both prevents the mounts we add below
121+
// from leaking back to the host, and ensures that those
122+
// mounts inherit PRIVATE propagation from their parent.
123+
mount_change(
124+
c"/",
125+
MountPropagationFlags::PRIVATE | MountPropagationFlags::REC,
126+
)?;
127+
128+
// Bind-mount the chroot target onto itself so that `/`
129+
// appears as a real mount point after chroot. Without
130+
// this, tools that inspect mounts (e.g. `findmnt
131+
// --mountpoint /`, which bootupd uses behind
132+
// `--filesystem /`) fail because the chroot dir is a
133+
// plain subdirectory of its parent mount and has no
134+
// mountinfo entry of its own.
135+
mount_bind_recursive(chroot_cstr.as_c_str(), chroot_cstr.as_c_str())?;
136+
137+
// Setup API filesystems
138+
// See https://systemd.io/API_FILE_SYSTEMS/
139+
mount(
140+
c"proc",
141+
proc_target.as_c_str(),
142+
c"proc",
143+
MountFlags::empty(),
144+
None,
145+
)?;
146+
mount_bind_recursive(c"/dev", dev_target.as_c_str())?;
147+
mount_bind_recursive(c"/sys", sys_target.as_c_str())?;
148+
// /run carries the udev database, which lsblk/libblkid
149+
// use to resolve partition GUIDs and other device
150+
// properties.
151+
mount_bind_recursive(c"/run", run_target.as_c_str())?;
152+
153+
for (src, tgt) in &user_binds {
154+
mount_bind_recursive(src.as_c_str(), tgt.as_c_str())?;
155+
}
156+
157+
chroot(chroot_cstr.as_c_str())?;
158+
chdir(c"/")?;
159+
160+
Ok(())
161+
});
162+
}
163+
164+
Ok(cmd)
165+
}
166+
167+
/// Run the specified command inside the chroot, inheriting stdio.
168+
/// `args` must include the program as its first element.
169+
pub fn run<S: AsRef<OsStr>>(self, args: impl IntoIterator<Item = S>) -> Result<()> {
170+
self.build_command(args)?
171+
.log_debug()
172+
.run_inherited_with_cmd_context()
173+
}
174+
175+
/// Run the specified command inside the chroot and capture stdout
176+
/// as a string. `args` must include the program as its first
177+
/// element.
178+
pub fn run_get_string<S: AsRef<OsStr>>(
179+
self,
180+
args: impl IntoIterator<Item = S>,
181+
) -> Result<String> {
182+
self.build_command(args)?.log_debug().run_get_string()
183+
}
184+
}
185+
186+
#[cfg(test)]
187+
mod tests {
188+
use super::*;
189+
use cap_std_ext::camino::Utf8PathBuf;
190+
191+
fn tmp_root() -> (tempfile::TempDir, Utf8PathBuf) {
192+
let dir = tempfile::tempdir().unwrap();
193+
let path = Utf8PathBuf::from_path_buf(dir.path().to_path_buf()).unwrap();
194+
(dir, path)
195+
}
196+
197+
#[test]
198+
fn builder_accumulates_binds_and_env() {
199+
let (_keep, root) = tmp_root();
200+
let src = root.join("src");
201+
let cmd = ChrootCmd::new(&root)
202+
.bind(&src, &"/boot")
203+
.setenv("FOO", "bar")
204+
.set_default_path();
205+
assert_eq!(cmd.bind_mounts.len(), 1);
206+
assert_eq!(cmd.bind_mounts[0].1, "/boot");
207+
// setenv + set_default_path
208+
assert_eq!(cmd.env_vars.len(), 2);
209+
assert!(cmd.env_vars.iter().any(|(k, _)| *k == "PATH"));
210+
assert!(cmd.env_vars.iter().any(|(k, v)| *k == "FOO" && *v == "bar"));
211+
}
212+
213+
#[test]
214+
fn build_command_creates_api_mount_dirs() {
215+
let (_keep, root) = tmp_root();
216+
// No user binds — just the API mount targets.
217+
let cmd = ChrootCmd::new(&root).build_command(["/bin/true"]).unwrap();
218+
for sub in ["proc", "dev", "sys", "run"] {
219+
assert!(
220+
root.join(sub).is_dir(),
221+
"API mount dir {sub} not created in {root}"
222+
);
223+
}
224+
assert_eq!(cmd.get_program(), "/bin/true");
225+
}
226+
227+
#[test]
228+
fn build_command_creates_user_bind_targets() {
229+
let (_keep, root) = tmp_root();
230+
let (_keep2, src_root) = tmp_root();
231+
ChrootCmd::new(&root)
232+
.bind(&src_root, &"/sysroot")
233+
.build_command(["/bin/true"])
234+
.unwrap();
235+
assert!(root.join("sysroot").is_dir());
236+
}
237+
238+
#[test]
239+
fn build_command_rejects_empty_args() {
240+
let (_keep, root) = tmp_root();
241+
let err = ChrootCmd::new(&root)
242+
.build_command(std::iter::empty::<&str>())
243+
.unwrap_err();
244+
assert!(
245+
err.to_string().contains("ChrootCmd requires the program"),
246+
"unexpected error: {err}"
247+
);
248+
}
249+
}

crates/utils/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
//!
55
mod bwrap;
66
pub use bwrap::*;
7+
mod chroot;
8+
pub use chroot::*;
79
mod command;
810
pub use command::*;
911
mod iterators;

0 commit comments

Comments
 (0)