diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index 801a019bd..18c1b2c98 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::{ @@ -214,6 +212,17 @@ fi ) } +fn open_target_root(root_setup: &RootSetup) -> Result { + if let Some(target_root) = root_setup.target_root_path.as_ref() { + Dir::open_ambient_dir(target_root, ambient_authority()).context("Opening target root path") + } else { + root_setup + .physical_root + .try_clone() + .context("Cloning target root handle") + } +} + pub fn get_esp_partition(device: &str) -> Result<(String, Option)> { let device_info = bootc_blockdev::partitions_of(Utf8Path::new(device))?; let esp = crate::bootloader::esp_in(&device_info)?; @@ -521,11 +530,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 = open_target_root(root_setup)?; + let esp_part = if root_setup.require_esp_mount { + crate::bootloader::require_boot_efi_mount(&esp_root)? + } else { + match crate::bootloader::require_boot_efi_mount(&esp_root) { + Ok(p) => p, + 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_part, cmdline_options, fs, postfetch.detected_bootloader.clone(), @@ -1063,11 +1086,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 = open_target_root(root_setup)?; + + let esp_part = if root_setup.require_esp_mount { + crate::bootloader::require_boot_efi_mount(&esp_root)? + } else { + match crate::bootloader::require_boot_efi_mount(&esp_root) { + Ok(p) => p, + 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_part, postfetch.detected_bootloader.clone(), state.composefs_options.insecure, state.composefs_options.uki_addon.as_ref(), @@ -1231,18 +1269,23 @@ 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 = open_target_root(root_setup)?; + 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, @@ -1406,4 +1449,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 7fce2888a..882c95390 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -11,7 +11,7 @@ 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) @@ -31,40 +31,45 @@ 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())) +fn normalize_esp_source(source: String) -> String { + if let Some(uuid) = source.strip_prefix("UUID=") { + format!("/dev/disk/by-uuid/{uuid}") + } else if let Some(label) = source.strip_prefix("LABEL=") { + format!("/dev/disk/by-label/{label}") + } else { + 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 +pub(crate) fn require_boot_efi_mount(root: &Dir) -> Result { + let efi_path = Utf8Path::new("boot").join(EFI_DIR); + let esp_fd = root .open_dir_optional(&efi_path) .context("Opening /boot/efi")? - else { - return Ok(()); - }; + .ok_or_else(|| anyhow::anyhow!("/boot/efi does not exist"))?; - let Some(false) = esp_fd.is_mountpoint(".")? else { - return Ok(()); - }; + if !esp_fd.is_mountpoint(".")?.unwrap_or(false) { + anyhow::bail!("/boot/efi is not a mountpoint. This is required for installation."); + } - 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")? - } 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"); + let fs = bootc_mount::inspect_filesystem_of_dir(&esp_fd)?; + if fs.fstype != "vfat" && fs.fstype != "fat" { + anyhow::bail!( + "/boot/efi is mounted but is not vfat/fat (found {}). This is required for installation.", + fs.fstype + ); } - Ok(()) + + let source = normalize_esp_source(fs.source); + + let source_path = Utf8Path::new(&source); + if !source_path.try_exists()? { + anyhow::bail!( + "/boot/efi source device does not exist: {source}. This is required for installation." + ); + } + + Ok(source) } /// Determine if the invoking environment contains bootupd, and if there are bootupd-based @@ -83,10 +88,19 @@ 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<()> { + if require_mount { + // We require /boot/efi to be mounted; finding the device is just a sanity check that it is. + // The device argument is currently used by bootupctl as a fallback if it can't find the ESP. + // But we want to fail fast if our own check fails. + let _esp_device = require_boot_efi_mount(root)?; + } + 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"]); @@ -111,17 +125,16 @@ pub(crate) fn install_via_bootupd( #[context("Installing bootloader")] pub(crate) fn install_systemd_boot( - device: &PartitionTable, + root: &Dir, + _device: &PartitionTable, _rootfs: &Utf8Path, _configopts: &crate::install::InstallConfigOpts, _deployment_path: Option<&str>, autoenroll: Option, ) -> 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 = require_boot_efi_mount(root)?; - 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"))?; @@ -237,3 +250,26 @@ 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 normalize_esp_source_uuid() { + let normalized = normalize_esp_source("UUID=abcd-1234".to_string()); + assert_eq!(normalized, "/dev/disk/by-uuid/abcd-1234"); + } + + #[test] + fn normalize_esp_source_label() { + let normalized = normalize_esp_source("LABEL=EFI".to_string()); + assert_eq!(normalized, "/dev/disk/by-label/EFI"); + } + + #[test] + fn normalize_esp_source_passthrough() { + let normalized = normalize_esp_source("/dev/sda1".to_string()); + assert_eq!(normalized, "/dev/sda1"); + } +} diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 600371e63..45764981e 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -1160,6 +1160,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> { @@ -1606,6 +1607,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)?; @@ -1614,12 +1622,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 => { @@ -2007,14 +2014,13 @@ 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)?; + if ARCH_USES_EFI && strict_esp { + // Require an explicit /boot/efi mount to avoid cleaning the wrong ESP. + crate::bootloader::require_boot_efi_mount(rootfs)?; } // This should not remove /boot/efi note. @@ -2228,7 +2234,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)?, } @@ -2390,6 +2396,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..e8c9f9a94 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: true, }) }