Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 86 additions & 44 deletions crates/lib/src/bootc_composefs/boot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1196,11 +1196,21 @@ pub(crate) fn setup_composefs_uki_boot(
/// `<tmpdir>/tmp` to provide a writable scratch area for tools invoked with
/// `--root`.
///
/// Drop order matters: the ESP and tmpfs guards are declared before `composefs`
/// so they are unmounted (and flushed) before the composefs root is detached.
/// Drop order matters: the tmpfs guard is declared before `composefs` so it
/// is unmounted (and flushed) before the composefs root is detached.
pub(crate) struct MountedImageRoot {
// Unmounted before `composefs` on drop; ESP before tmp (inner before outer).
_esp: bootc_mount::tempmount::MountGuard,
// The ESP's device path. The ESP is intentionally *not* mounted here for
// our whole lifetime; it's mounted on demand via `with_esp()` instead.
// That's because `install_via_bootupd` runs bootupd's install tooling
// inside a `ChrootCmd`, which unshares the mount namespace and
// recursively self-binds the chroot directory (our `root_path()`) onto
// itself. If the ESP were already mounted at `<root_path>/boot` when
// that happens, the self-bind would duplicate it into an
// orphaned/shadowed mountinfo entry that's still visible to naive
// `findmnt`-based scans (like bootupd's), causing it to pick the wrong
// filesystem UUID. See the commit introducing this comment for details.
esp_device: Utf8PathBuf,
// Unmounted before `composefs` on drop.
_tmp: bootc_mount::tempmount::MountGuard,
composefs: TempMount,
pub(crate) esp_subdir: &'static str,
Expand Down Expand Up @@ -1236,10 +1246,6 @@ impl MountedImageRoot {
// unconditionally mount the ESP at /boot for now.
let esp_subdir = "boot";

let esp_path = composefs.dir.path().join(esp_subdir);
let esp =
mount_esp_at(&esp_part.path(), esp_path).context("Mounting ESP into composefs root")?;

// Mount a tmpfs over /tmp so that tools invoked with --root have a
// writable scratch area without touching the read-only EROFS root.
let tmp_path = composefs.dir.path().join("tmp");
Expand All @@ -1253,7 +1259,7 @@ impl MountedImageRoot {
.context("Mounting tmpfs into composefs root")?;

Ok(Self {
_esp: esp,
esp_device: esp_part.path().into(),
_tmp: tmp,
composefs,
esp_subdir,
Expand All @@ -1271,12 +1277,29 @@ impl MountedImageRoot {
}

/// Open the mounted ESP as a capability-safe directory.
///
/// This only succeeds while the ESP is actually mounted, i.e. from
/// within (or after) a call to [`Self::with_esp`].
pub(crate) fn open_esp_dir(&self) -> Result<Dir> {
self.composefs
.fd
.open_dir(self.esp_subdir)
.with_context(|| format!("Opening ESP at /{}", self.esp_subdir))
}

/// Mount the ESP at `<tmpdir>/<esp_subdir>` for the duration of `f`, then
/// unmount it again. Nothing is mounted there outside of calls to this
/// method, so callers that need to invoke bootloader-install tooling
/// that itself creates a private mount namespace (e.g. via `ChrootCmd`)
/// can safely do so without risking this mount being shadowed/duplicated
/// by that tooling's own mount-namespace setup.
pub(crate) fn with_esp<T>(&self, f: impl FnOnce(&Dir) -> Result<T>) -> Result<T> {
let esp_path = self.root_path().join(self.esp_subdir);
let _guard = mount_esp_at(self.esp_device.as_str(), esp_path)
.context("Mounting ESP into composefs root")?;
let dir = self.open_esp_dir()?;
f(&dir)
}
}

pub struct SecurebootKeys {
Expand Down Expand Up @@ -1381,56 +1404,75 @@ pub(crate) async fn setup_composefs_boot(
postfetch.detected_bootloader,
Bootloader::Grub | Bootloader::GrubCC
) {
let chroot_target = Utf8Path::from_path(mounted_root.root_path())
.ok_or_else(|| anyhow!("composefs tmpdir path is not valid UTF-8"))?;
// Like the ostree backend, bind the physical root's real /boot (an
// ordinary ext4/xfs/... directory, not yet populated with kernels at
// this point) into the chroot. This gives bootupd both a correct
// filesystem to inspect for `--write-uuid` (rather than the ESP,
// which is otherwise mounted at the composefs root's own /boot) and
// an empty `boot/efi` directory for its EFI component to discover
// and mount the real ESP into, exactly as it would on ostree.
let bind_boot_path = root_setup.physical_root_path.join("boot");
crate::bootloader::install_via_bootupd(
&root_setup.device_info,
&root_setup.physical_root_path,
&state.config_opts,
None,
Some(chroot_target),
Some(bind_boot_path.as_path()),
)?;

// FIXME: Remove this hack once we have support in bootupd
if matches!(postfetch.detected_bootloader, Bootloader::GrubCC) {
// bootupctl wrote this under the physical root's real /boot (via
// the bind mount above), not under the composefs root.
root_setup
.physical_root
.remove_dir_all("boot/grub2")
.remove_all_optional("boot/grub2")
.context("removing grub2")?;

let (os_id, ..) = parse_os_release(mounted_root.dir())?
.ok_or_else(|| anyhow::anyhow!("Failed to parse os-release"))?;

let dir = format!("EFI/{os_id}");

// Files are in EFI/<os-name>/
let efis_dir = mounted_root
.open_esp_dir()
.context("opening esp")?
.open_dir(&dir)
.with_context(|| format!("Opening {dir}"))?;

efis_dir
.remove_file_optional("bootuuid.cfg")
.context("Removing bootuuid.cfg")?;
efis_dir
.remove_file_optional("grub.cfg")
.context("Removing grub.cfg")?;

let final_name = match std::env::consts::ARCH {
"x86_64" => "grubx64.efi",
"aarch64" => "grubaa64-cc.efi",
arch => anyhow::bail!("GrubCC not supported for: {arch}"),
};
// install_via_bootupd above has already returned, so it's safe
// to mount the ESP here for the duration of this cleanup.
mounted_root.with_esp(|esp_dir| {
let (os_id, ..) = parse_os_release(mounted_root.dir())?
.ok_or_else(|| anyhow::anyhow!("Failed to parse os-release"))?;

let dir = format!("EFI/{os_id}");

// Files are in EFI/<os-name>/
let efis_dir = esp_dir
.open_dir(&dir)
.with_context(|| format!("Opening {dir}"))?;

efis_dir
.remove_file_optional("bootuuid.cfg")
.context("Removing bootuuid.cfg")?;
efis_dir
.remove_file_optional("grub.cfg")
.context("Removing grub.cfg")?;

let final_name = match std::env::consts::ARCH {
"x86_64" => "grubx64.efi",
"aarch64" => "grubaa64-cc.efi",
arch => anyhow::bail!("GrubCC not supported for: {arch}"),
};

mounted_root
.dir()
.copy("usr/lib/grub-cc/grub-cc.efi", &efis_dir, final_name)
.context("Copying grub-cc binary")?;

mounted_root
.dir()
.copy("usr/lib/grub-cc/grub-cc.efi", &efis_dir, final_name)
.context("Copying grub-cc binary")?;
Ok(())
})?;
}
} else {
crate::bootloader::install_systemd_boot(
&mounted_root,
&state.config_opts,
get_secureboot_keys(mounted_root.dir(), BOOTC_AUTOENROLL_PATH)?,
)?;
mounted_root.with_esp(|_esp_dir| {
crate::bootloader::install_systemd_boot(
&mounted_root,
&state.config_opts,
get_secureboot_keys(mounted_root.dir(), BOOTC_AUTOENROLL_PATH)?,
)
})?;
}

let Some(entry) = entries.iter().next() else {
Expand Down
63 changes: 45 additions & 18 deletions crates/lib/src/bootloader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,12 @@ pub(crate) fn supports_bootupd(root: &Dir) -> Result<bool> {
/// 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 chroot
/// output. When `chroot_target` 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<bool> {
fn bootupd_supports_filesystem(chroot_target: Option<&Utf8Path>) -> Result<bool> {
let help_args = ["bootupctl", "backend", "install", "--help"];
let output = if let Some(deploy) = deployment_path {
let target_root = rootfs.join(deploy);
ChrootCmd::new(&target_root)
let output = if let Some(target_root) = chroot_target {
ChrootCmd::new(target_root)
.set_default_path()
.run_get_string(help_args)?
} else {
Expand All @@ -129,17 +128,29 @@ 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 chroot path we bind-mount the physical root at
/// `/sysroot` to give `lsblk` a real block-backed path.
/// via `lsblk`. When `chroot_target` is set, bootupctl is executed inside the
/// given chroot root via [`ChrootCmd`], with the physical root bind-mounted at
/// `/sysroot` so `lsblk` can resolve a real block-backed path.
///
/// For older bootupd versions that lack `--filesystem` we fall back to the
/// legacy `--device <device_path> <rootfs>` invocation.
///
/// If `bind_boot_path` is set, the given host path is bind-mounted onto
/// `/boot` inside the chroot. Both the ostree and composefs backends use
/// this to expose the physical root's real `/boot` inside their respective
/// chroots, since neither chroot target (an ostree deployment, or a mounted
/// composefs image) has a `/boot` backed by the real root filesystem on its
/// own. This matters because bootupd derives the UUID it writes for
/// `--write-uuid` from whatever filesystem is mounted at `<chroot>/boot`,
/// and looks for an empty `boot/efi` directory there to discover and mount
/// the real ESP into.
#[context("Installing bootloader")]
pub(crate) fn install_via_bootupd(
device: &bootc_blockdev::Device,
rootfs: &Utf8Path,
configopts: &crate::install::InstallConfigOpts,
deployment_path: Option<&str>,
chroot_target: Option<&Utf8Path>,
bind_boot_path: Option<&Utf8Path>,
) -> Result<()> {
let verbose = std::env::var_os("BOOTC_BOOTLOADER_DEBUG").map(|_| "-vvvv");
// bootc defaults to only targeting the platform boot method.
Expand All @@ -150,7 +161,7 @@ pub(crate) fn install_via_bootupd(
// 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.
let rootfs_mount = if deployment_path.is_none() {
let rootfs_mount = if chroot_target.is_none() {
rootfs.as_str()
} else {
"/"
Expand Down Expand Up @@ -179,7 +190,7 @@ pub(crate) fn install_via_bootupd(
// parent via require_single_root(). (Older bootupd doesn't support
// multiple backing devices anyway.)
// Computed before building bootupd_args so the String lives long enough.
let root_device_path = if bootupd_supports_filesystem(rootfs, deployment_path)
let root_device_path = if bootupd_supports_filesystem(chroot_target)
.context("Probing bootupd --filesystem support")?
{
None
Expand All @@ -192,17 +203,24 @@ pub(crate) fn install_via_bootupd(
bootupd_args.push(rootfs_mount);
} else {
tracing::debug!("bootupd supports --filesystem");
bootupd_args.extend(["--filesystem", rootfs_mount]);
// Inside a chroot the physical root is bind-mounted at /sysroot (see
// below) so bootupd's own device resolution (via lsblk) sees a real
// block-backed path. This matters for composefs, where the chroot's
// own "/" is a virtual composefs mount with no backing block device.
let filesystem_path = if chroot_target.is_some() {
"/sysroot"
} else {
rootfs_mount
};
bootupd_args.extend(["--filesystem", filesystem_path]);
bootupd_args.push(rootfs_mount);
}

// 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 <https://github.com/bootc-dev/bootc/issues/2111>).
if let Some(deploy) = deployment_path {
let target_root = rootfs.join(deploy);
let boot_path = rootfs.join("boot");
if let Some(target_root) = chroot_target {
let rootfs_path = rootfs.to_path_buf();

tracing::debug!("Running bootupctl via chroot in {}", target_root);
Expand All @@ -212,10 +230,19 @@ pub(crate) fn install_via_bootupd(
let mut chroot_args = vec!["bootupctl"];
chroot_args.extend(bootupd_args);

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");
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. This is a
// non-recursive bind: the physical root's /boot may itself have the
// ESP mounted at boot/efi (see clean_boot_directories()), and we
// don't want that mount to be dragged along, since bootupd's own EFI
// component expects to find an empty boot/efi directory to mount the
// real ESP onto itself (see MountedImageRoot::with_esp()). A stray
// nested ESP mount there has also been observed to confuse grub-probe
// into embedding the wrong root device in the BIOS boot prefix.
if let Some(boot_path) = &bind_boot_path {
cmd = cmd.bind_flat(boot_path, &"/boot");
}

// Only bind mount the physical root at /sysroot when using --filesystem;
// bootupd needs it to resolve backing block devices via lsblk.
Expand Down
14 changes: 9 additions & 5 deletions crates/lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1868,14 +1868,18 @@ async fn install_with_sysroot(
} else {
match postfetch.detected_bootloader {
Bootloader::Grub => {
let root_path = rootfs
.target_root_path
.clone()
.unwrap_or(rootfs.physical_root_path.clone());
let chroot_target = root_path.join(deployment_path.as_str());
let bind_boot_path = root_path.join("boot");
crate::bootloader::install_via_bootupd(
&rootfs.device_info,
&rootfs
.target_root_path
.clone()
.unwrap_or(rootfs.physical_root_path.clone()),
&root_path,
&state.config_opts,
Some(&deployment_path.as_str()),
Some(chroot_target.as_path()),
Some(bind_boot_path.as_path()),
)?;
}
Bootloader::Systemd | Bootloader::GrubCC => {
Expand Down
Loading
Loading