diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 801a019bd..479ed4d19 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -106,10 +106,8 @@ use crate::{ }; use crate::{ bootc_composefs::state::{get_booted_bls, write_composefs_state}, - bootloader::esp_in, -}; -use crate::{ - bootc_composefs::status::get_container_manifest_and_config, bootc_kargs::compute_new_kargs, + bootc_composefs::status::get_container_manifest_and_config, + bootc_kargs::compute_new_kargs, }; use crate::{bootc_composefs::status::get_sorted_grub_uki_boot_entries, install::PostFetchState}; use crate::{ @@ -521,11 +519,25 @@ pub(crate) fn setup_composefs_bls_boot( cmdline_options.extend(&Cmdline::from(&composefs_cmdline)); // Locate ESP partition device - let esp_part = esp_in(&root_setup.device_info)?; + let esp_root = root_setup.open_target_root()?; + let esp_device = if root_setup.require_esp_mount { + crate::bootloader::find_esp_mount(&esp_root)?.device + } else { + match crate::bootloader::find_esp_mount(&esp_root) { + Ok(esp) => esp.device, + Err(e) => { + tracing::debug!( + "ESP mount check failed in permissive mode: {e}; falling back to partition table scan" + ); + let esp = crate::bootloader::esp_in(&root_setup.device_info)?; + esp.node.clone() + } + } + }; ( root_setup.physical_root_path.clone(), - esp_part.node.clone(), + esp_device, cmdline_options, fs, postfetch.detected_bootloader.clone(), @@ -1063,11 +1075,26 @@ pub(crate) fn setup_composefs_uki_boot( BootSetupType::Setup((root_setup, state, postfetch, ..)) => { state.require_no_kargs_for_uki()?; - let esp_part = esp_in(&root_setup.device_info)?; + let esp_root = root_setup.open_target_root()?; + + let esp_device = if root_setup.require_esp_mount { + crate::bootloader::find_esp_mount(&esp_root)?.device + } else { + match crate::bootloader::find_esp_mount(&esp_root) { + Ok(esp) => esp.device, + Err(e) => { + tracing::debug!( + "ESP mount check failed in permissive mode: {e}; falling back to partition table scan" + ); + let esp = crate::bootloader::esp_in(&root_setup.device_info)?; + esp.node.clone() + } + } + }; ( root_setup.physical_root_path.clone(), - esp_part.node.clone(), + esp_device, postfetch.detected_bootloader.clone(), state.composefs_options.insecure, state.composefs_options.uki_addon.as_ref(), @@ -1231,23 +1258,29 @@ pub(crate) async fn setup_composefs_boot( .or(root_setup.rootfs_uuid.as_deref()) .ok_or_else(|| anyhow!("No uuid for boot/root"))?; + let target_root = root_setup.open_target_root()?; + if cfg!(target_arch = "s390x") { // TODO: Integrate s390x support into install_via_bootupd crate::bootloader::install_via_zipl(&root_setup.device_info, boot_uuid)?; } else if postfetch.detected_bootloader == Bootloader::Grub { crate::bootloader::install_via_bootupd( &root_setup.device_info, + &target_root, &root_setup.physical_root_path, &state.config_opts, None, + root_setup.require_esp_mount, )?; } else { crate::bootloader::install_systemd_boot( + &target_root, &root_setup.device_info, &root_setup.physical_root_path, &state.config_opts, None, get_secureboot_keys(&mounted_fs, BOOTC_AUTOENROLL_PATH)?, + root_setup.require_esp_mount, )?; } @@ -1406,4 +1439,11 @@ mod tests { "RHEL should sort before Fedora in descending order" ); } + + #[test] + fn test_efi_uuid_source_formatting() { + let source = get_efi_uuid_source(); + assert!(source.contains("${config_directory}/")); + assert!(source.contains(EFI_UUID_FILE)); + } } diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 1e1157ef8..2188a5203 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -1,9 +1,10 @@ +use std::borrow::Cow; use std::fs::create_dir_all; use std::process::Command; use anyhow::{Context, Result, anyhow, bail}; use bootc_utils::{BwrapCmd, CommandRunExt}; -use camino::Utf8Path; +use camino::{Utf8Path, Utf8PathBuf}; use cap_std_ext::cap_std::fs::Dir; use cap_std_ext::dirext::CapStdExtDirExt; use fn_error_context::context; @@ -11,11 +12,12 @@ use fn_error_context::context; use bootc_blockdev::{Partition, PartitionTable}; use bootc_mount as mount; -use crate::bootc_composefs::boot::{SecurebootKeys, get_sysroot_parent_dev, mount_esp}; +use crate::bootc_composefs::boot::{SecurebootKeys, mount_esp}; use crate::{discoverable_partition_specification, utils}; /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel) pub(crate) const EFI_DIR: &str = "efi"; + /// The EFI system partition GUID /// Path to the bootupd update payload #[allow(dead_code)] @@ -24,6 +26,14 @@ const BOOTUPD_UPDATES: &str = "usr/lib/bootupd/updates"; // from: https://github.com/systemd/systemd/blob/26b2085d54ebbfca8637362eafcb4a8e3faf832f/man/systemd-boot.xml#L392 const SYSTEMD_KEY_DIR: &str = "loader/keys"; +/// Represents a found EFI System Partition (ESP) mount +pub(crate) struct Esp { + /// The backing device path + pub(crate) device: String, + /// The mountpoint path relative to the root + pub(crate) path: Utf8PathBuf, +} + #[allow(dead_code)] pub(crate) fn esp_in(device: &PartitionTable) -> Result<&Partition> { device @@ -31,40 +41,99 @@ pub(crate) fn esp_in(device: &PartitionTable) -> Result<&Partition> { .ok_or(anyhow::anyhow!("ESP not found in partition table")) } -/// Get esp partition node based on the root dir -pub(crate) fn get_esp_partition_node(root: &Dir) -> Result> { - let device = get_sysroot_parent_dev(&root)?; - let base_partitions = bootc_blockdev::partitions_of(Utf8Path::new(&device))?; - let esp = base_partitions.find_partition_of_esp()?; - Ok(esp.map(|v| v.node.clone())) +/// Convert a source specification like UUID=... or LABEL=... into a +/// canonical /dev/disk/by-... path. If the source is already a +/// path, it is returned unchanged. +fn normalize_esp_source(source: &str) -> Cow<'_, str> { + if let Some(uuid) = source.strip_prefix("UUID=") { + Cow::Owned(format!("/dev/disk/by-uuid/{uuid}")) + } else if let Some(label) = source.strip_prefix("LABEL=") { + Cow::Owned(format!("/dev/disk/by-label/{label}")) + } else { + Cow::Borrowed(source) + } } -/// Mount ESP part at /boot/efi -pub(crate) fn mount_esp_part(root: &Dir, root_path: &Utf8Path, is_ostree: bool) -> Result<()> { - let efi_path = Utf8Path::new("boot").join(crate::bootloader::EFI_DIR); - let Some(esp_fd) = root - .open_dir_optional(&efi_path) - .context("Opening /boot/efi")? - else { - return Ok(()); +/// Helper to check if a path is a valid ESP mount +fn inspect_esp_at(root: &Dir, path: impl AsRef) -> Result> { + let path = path.as_ref(); + let esp_fd = match root.open_dir_optional(path)? { + Some(fd) => fd, + None => return Ok(None), }; - let Some(false) = esp_fd.is_mountpoint(".")? else { - return Ok(()); - }; + if !esp_fd.is_mountpoint(".")?.unwrap_or(false) { + return Ok(None); + } + + let fs = bootc_mount::inspect_filesystem_of_dir(&esp_fd)?; + if fs.fstype != "vfat" && fs.fstype != "fat" { + return Ok(None); + } - tracing::debug!("Not a mountpoint: /boot/efi"); - // On ostree env with enabled composefs, should be /target/sysroot - let physical_root = if is_ostree { - &root.open_dir("sysroot").context("Opening /sysroot")? + let source = normalize_esp_source(&fs.source); + + let source_path = Utf8Path::new(source.as_ref()); + if !source_path.try_exists()? { + return Ok(None); + } + + Ok(Some(source.into_owned())) +} + +/// Find the ESP mountpoint by searching common locations (/efi, /boot/efi, /boot) +pub(crate) fn find_esp_mount(root: &Dir) -> Result { + // Possible paths for ESP, in order of preference. + // We check /efi first as per DPS, then /boot/efi (distro standard), + // and finally /boot itself (if it is the ESP). + if let Some(device) = inspect_esp_at(root, EFI_DIR)? { + tracing::debug!("Found ESP at {EFI_DIR} (device: {device})"); + return Ok(Esp { + device, + path: Utf8PathBuf::from(EFI_DIR), + }); + } + + let boot_efi = Utf8Path::new("boot").join(EFI_DIR); + if let Some(device) = inspect_esp_at(root, &boot_efi)? { + tracing::debug!("Found ESP at {boot_efi} (device: {device})"); + return Ok(Esp { + device, + path: boot_efi, + }); + } + + if let Some(device) = inspect_esp_at(root, "boot")? { + tracing::debug!("Found ESP at boot (device: {device})"); + return Ok(Esp { + device, + path: Utf8PathBuf::from("boot"), + }); + } + + anyhow::bail!("No ESP found at /efi, /boot/efi or /boot. This is required for installation.") +} + +/// Helper to find the ESP device, either via mount or partition table scan +pub(crate) fn get_esp_device<'a>( + root: &Dir, + device_info: &'a PartitionTable, + require_mount: bool, +) -> Result> { + if require_mount { + Ok(Cow::Owned(find_esp_mount(root)?.device)) } else { - root - }; - if let Some(esp_part) = get_esp_partition_node(physical_root)? { - bootc_mount::mount(&esp_part, &root_path.join(&efi_path))?; - tracing::debug!("Mounted {esp_part} at /boot/efi"); + match find_esp_mount(root) { + Ok(esp) => Ok(Cow::Owned(esp.device)), + Err(e) => { + tracing::debug!( + "ESP mount check failed in permissive mode: {e}; falling back to partition table scan" + ); + let esp = esp_in(device_info)?; + Ok(Cow::Borrowed(&esp.node)) + } + } } - Ok(()) } /// Determine if the invoking environment contains bootupd, and if there are bootupd-based @@ -83,10 +152,18 @@ pub(crate) fn supports_bootupd(root: &Dir) -> Result { #[context("Installing bootloader")] pub(crate) fn install_via_bootupd( device: &PartitionTable, + root: &Dir, rootfs: &Utf8Path, configopts: &crate::install::InstallConfigOpts, deployment_path: Option<&str>, + require_mount: bool, ) -> Result<()> { + let esp = if require_mount { + Some(find_esp_mount(root)?) + } else { + find_esp_mount(root).ok() + }; + let verbose = std::env::var_os("BOOTC_BOOTLOADER_DEBUG").map(|_| "-vvvv"); // bootc defaults to only targeting the platform boot method. let bootupd_opts = (!configopts.generic_image).then_some(["--update-firmware", "--auto"]); @@ -126,6 +203,8 @@ pub(crate) fn install_via_bootupd( if let Some(deploy) = deployment_path { let target_root = rootfs.join(deploy); let boot_path = rootfs.join("boot"); + let efi_path = rootfs.join(EFI_DIR); + let efi_target = format!("/{EFI_DIR}"); tracing::debug!("Running bootupctl via bwrap in {}", target_root); @@ -140,6 +219,15 @@ pub(crate) fn install_via_bootupd( // Bind the target block device inside the bwrap container so bootupctl can access it .bind_device(device.path().as_str()); + // If we found an ESP, ensure it's bound. + // If it's at /boot or /boot/efi, it's already bound via /boot. + // If it's at /efi, we need to bind it explicitly. + if let Some(esp) = esp { + if esp.path == EFI_DIR { + cmd = cmd.bind(&efi_path, &efi_target); + } + } + // Also bind all partitions of the tafet block device for partition in &device.partitions { cmd = cmd.bind_device(&partition.node); @@ -165,17 +253,17 @@ pub(crate) fn install_via_bootupd( #[context("Installing bootloader")] pub(crate) fn install_systemd_boot( + root: &Dir, device: &PartitionTable, _rootfs: &Utf8Path, _configopts: &crate::install::InstallConfigOpts, _deployment_path: Option<&str>, autoenroll: Option, + require_mount: bool, ) -> Result<()> { - let esp_part = device - .find_partition_of_type(discoverable_partition_specification::ESP) - .ok_or_else(|| anyhow::anyhow!("ESP partition not found"))?; + let esp_device = get_esp_device(root, device, require_mount)?; - let esp_mount = mount_esp(&esp_part.node).context("Mounting ESP")?; + let esp_mount = mount_esp(&esp_device).context("Mounting ESP")?; let esp_path = Utf8Path::from_path(esp_mount.dir.path()) .ok_or_else(|| anyhow::anyhow!("Failed to convert ESP mount path to UTF-8"))?; @@ -291,3 +379,30 @@ pub(crate) fn install_via_zipl(device: &PartitionTable, boot_uuid: &str) -> Resu .log_debug() .run_inherited_with_cmd_context() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_normalize_esp_source_uuid() { + let normalized = normalize_esp_source("UUID=abcd-1234"); + assert_eq!(normalized, "/dev/disk/by-uuid/abcd-1234"); + assert!(matches!(normalized, Cow::Owned(_))); + } + + #[test] + fn test_normalize_esp_source_label() { + let normalized = normalize_esp_source("LABEL=EFI"); + assert_eq!(normalized, "/dev/disk/by-label/EFI"); + assert!(matches!(normalized, Cow::Owned(_))); + } + + #[test] + fn test_normalize_esp_source_passthrough() { + let path = "/dev/sda1"; + let normalized = normalize_esp_source(path); + assert_eq!(normalized, path); + assert!(matches!(normalized, Cow::Borrowed(_))); + } +} diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 75c151e0d..3e06fb728 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -1298,6 +1298,7 @@ pub(crate) struct RootSetup { skip_finalize: bool, boot: Option, pub(crate) kargs: CmdlineOwned, + pub(crate) require_esp_mount: bool, } fn require_boot_uuid(spec: &MountSpec) -> Result<&str> { @@ -1306,6 +1307,17 @@ fn require_boot_uuid(spec: &MountSpec) -> Result<&str> { } impl RootSetup { + pub(crate) fn open_target_root(&self) -> Result { + if let Some(target_root) = self.target_root_path.as_ref() { + Dir::open_ambient_dir(target_root, cap_std::ambient_authority()) + .context("Opening target root path") + } else { + self.physical_root + .try_clone() + .context("Cloning target root handle") + } + } + /// Get the UUID= mount specifier for the /boot filesystem; if there isn't one, the root UUID will /// be returned. pub(crate) fn get_boot_uuid(&self) -> Result> { @@ -1753,6 +1765,13 @@ async fn install_with_sysroot( .context("Opening deployment dir")?; let postfetch = PostFetchState::new(state, &deployment_dir)?; + let target_root_path = rootfs + .target_root_path + .clone() + .unwrap_or(rootfs.physical_root_path.clone()); + let target_root = Dir::open_ambient_dir(&target_root_path, cap_std::ambient_authority()) + .with_context(|| format!("Opening target root directory {target_root_path}"))?; + if cfg!(target_arch = "s390x") { // TODO: Integrate s390x support into install_via_bootupd crate::bootloader::install_via_zipl(&rootfs.device_info, boot_uuid)?; @@ -1761,12 +1780,11 @@ async fn install_with_sysroot( Bootloader::Grub => { crate::bootloader::install_via_bootupd( &rootfs.device_info, - &rootfs - .target_root_path - .clone() - .unwrap_or(rootfs.physical_root_path.clone()), + &target_root, + &target_root_path, &state.config_opts, Some(&deployment_path.as_str()), + rootfs.require_esp_mount, )?; } Bootloader::Systemd => { @@ -2154,26 +2172,35 @@ fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> { } #[context("Removing boot directory content")] -fn clean_boot_directories(rootfs: &Dir, rootfs_path: &Utf8Path, is_ostree: bool) -> Result<()> { +fn clean_boot_directories(rootfs: &Dir, is_ostree: bool, strict_esp: bool) -> Result<()> { let bootdir = crate::utils::open_dir_remount_rw(rootfs, BOOT.into()).context("Opening /boot")?; - if ARCH_USES_EFI { - // On booted FCOS, esp is not mounted by default - // Mount ESP part at /boot/efi before clean - crate::bootloader::mount_esp_part(&rootfs, &rootfs_path, is_ostree)?; - } + let esp = if ARCH_USES_EFI { + if strict_esp { + Some(crate::bootloader::find_esp_mount(rootfs)?) + } else { + crate::bootloader::find_esp_mount(rootfs).ok() + } + } else { + None + }; - // This should not remove /boot/efi note. + // This should not remove the ESP if it is a mount point. remove_all_except_loader_dirs(&bootdir, is_ostree).context("Emptying /boot")?; + // If we have an ESP that is NOT /boot, we empty it. + // If it is /boot, we already did a selective empty above. // TODO: we should also support not wiping the ESP. - if ARCH_USES_EFI { - if let Some(efidir) = bootdir - .open_dir_optional(crate::bootloader::EFI_DIR) - .context("Opening /boot/efi")? - { - remove_all_in_dir_no_xdev(&efidir, false).context("Emptying EFI system partition")?; + if let Some(esp) = esp { + if esp.path != BOOT { + if let Some(efidir) = rootfs + .open_dir_optional(&esp.path) + .with_context(|| format!("Opening ESP at {}", esp.path))? + { + remove_all_in_dir_no_xdev(&efidir, false) + .context("Emptying EFI system partition")?; + } } } @@ -2375,7 +2402,7 @@ pub(crate) async fn install_to_filesystem( .await??; } Some(ReplaceMode::Alongside) => { - clean_boot_directories(&target_rootfs_fd, &target_root_path, is_already_ostree)? + clean_boot_directories(&target_rootfs_fd, is_already_ostree, !targeting_host_root)? } None => require_empty_rootdir(&rootfs_fd)?, } @@ -2537,6 +2564,7 @@ pub(crate) async fn install_to_filesystem( boot, kargs, skip_finalize, + require_esp_mount: !targeting_host_root, }; install_to_filesystem_impl(&state, &mut rootfs, cleanup).await?; diff --git a/crates/lib/src/install/baseline.rs b/crates/lib/src/install/baseline.rs index d05604ed5..0d7280e1d 100644 --- a/crates/lib/src/install/baseline.rs +++ b/crates/lib/src/install/baseline.rs @@ -496,5 +496,6 @@ pub(crate) fn install_create_rootfs( boot, kargs, skip_finalize: false, + require_esp_mount: false, }) }