diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index c2a984e79..58a084bce 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -440,6 +440,79 @@ pub(crate) async fn get_container_manifest_and_config( Ok(ImgConfigManifest { manifest, config }) } +/// Directory where systemd-boot / BLS-compatible bootloaders expect Type 1 +/// boot entries. Its presence is used as a signal that a non-EFI system +/// nevertheless uses the BLS layout (see [`classify_bootloader`]). +const BLS_ENTRIES_DIR: &str = "/boot/loader/entries"; + +/// Directories where GRUB keeps its own configuration and modules. Their +/// presence means GRUB owns the boot flow even if BLS Type 1 entries also +/// exist, because GRUB can consume those entries itself via the `blscfg` +/// module — Fedora and RHEL enable exactly that with +/// `GRUB_ENABLE_BLSCFG=true`. `/boot/grub2` is the Fedora/RHEL path, +/// `/boot/grub` the Debian/Ubuntu one. +const GRUB_DIRS: [&str; 2] = ["/boot/grub2", "/boot/grub"]; + +/// Pure classifier for the bootloader kind, split from I/O for testability. +/// +/// - When `EFI_LOADER_INFO` is present, its content selects between systemd- +/// boot, GRUB Confidential Compute, and generic GRUB (existing behavior). +/// - When there are no EFI variables to inspect (`SystemNotUEFI` / +/// `MissingVar`), fall back to a filesystem probe: many non-EFI systems +/// still lay down the BLS Type 1 entry layout at `/boot/loader/entries/` +/// (Raspberry Pi with direct-kernel boot from Pi firmware, U-Boot with +/// the extlinux/BLS loader, coreboot with a linux payload, various +/// ARM/embedded boards). Treat those as BLS-compatible so `storage::new` +/// picks the ESP mount as `boot_dir` rather than `/sysroot/boot/`. Only +/// fall back to GRUB when neither an EFI system nor a BLS layout is +/// present. +/// +/// A BLS entries directory alone is not sufficient evidence, because GRUB +/// with `blscfg` reads the same directory. So GRUB's own directory wins +/// when both are present: a legacy-BIOS Fedora/RHEL install has +/// `/boot/grub2/` *and* `/boot/loader/entries/`, and is unambiguously +/// GRUB. Only a BLS layout with no GRUB directory implies a BLS-native +/// bootloader. +/// - Other EFI read errors propagate. +fn classify_bootloader( + efi_loader_info: Result, + bls_entries_dir_present: bool, + grub_dir_present: bool, +) -> Result { + match efi_loader_info { + Ok(loader) => { + let loader = loader.to_lowercase(); + if loader.contains("systemd-boot") { + Ok(Bootloader::Systemd) + } else if loader.contains("grub cc") { + Ok(Bootloader::GrubCC) + } else { + Ok(Bootloader::Grub) + } + } + Err(EfiError::SystemNotUEFI) | Err(EfiError::MissingVar) => { + if grub_dir_present { + tracing::debug!( + "No EFI vars and a GRUB directory is present; treating \ + bootloader as GRUB even if BLS entries also exist \ + (GRUB reads them via blscfg)" + ); + Ok(Bootloader::Grub) + } else if bls_entries_dir_present { + tracing::debug!( + "No EFI vars, no GRUB directory, and {BLS_ENTRIES_DIR} is \ + a directory; treating bootloader as BLS-compatible \ + (systemd-boot)" + ); + Ok(Bootloader::Systemd) + } else { + Ok(Bootloader::Grub) + } + } + Err(e) => anyhow::bail!("Failed to read EfiLoaderInfo: {e:?}"), + } +} + #[context("Getting bootloader")] pub(crate) fn get_bootloader() -> Result { static BOOTLOADER: OnceLock = OnceLock::new(); @@ -448,28 +521,29 @@ pub(crate) fn get_bootloader() -> Result { return Ok(*bootloader); } - let bootloader = match read_uefi_var(EFI_LOADER_INFO) { - Ok(loader) => { - if loader.to_lowercase().contains("systemd-boot") { - return Ok(Bootloader::Systemd); - } - - if loader.to_lowercase().contains("grub cc") { - return Ok(Bootloader::GrubCC); - } - - return Ok(Bootloader::Grub); - } - - Err(efi_error) => match efi_error { - EfiError::SystemNotUEFI | EfiError::MissingVar => Bootloader::Grub, - e => anyhow::bail!("Failed to read EfiLoaderInfo: {e:?}"), - }, - }; - - BOOTLOADER.get_or_init(|| bootloader); + let efi_result = read_uefi_var(EFI_LOADER_INFO); + // Non-EFI systems have a stable filesystem-based classification, so we + // can cache. EFI systems are left uncached to preserve the pre-existing + // behavior of re-reading `EFI_LOADER_INFO` on every call — some tests + // observe bootloader-info changes over the course of a run. + let non_efi = matches!( + &efi_result, + Err(EfiError::SystemNotUEFI) | Err(EfiError::MissingVar), + ); + + let bootloader = classify_bootloader( + efi_result, + // The FS probes are only consulted in the non-EFI classification + // branch; skip the `stat(2)`s on EFI systems. + non_efi && std::path::Path::new(BLS_ENTRIES_DIR).is_dir(), + non_efi && GRUB_DIRS.iter().any(|d| std::path::Path::new(d).is_dir()), + )?; + + if non_efi { + BOOTLOADER.get_or_init(|| bootloader); + } - return Ok(bootloader); + Ok(bootloader) } /// Retrieves the OCI manifest and config for a deployment from the composefs repository. @@ -1089,6 +1163,125 @@ mod tests { assert_eq!(v.digest.as_ref(), DIGEST); } + #[test] + fn classify_bootloader_cases() { + struct Case { + desc: &'static str, + efi: Result, + bls: bool, + grub_dir: bool, + expected: Bootloader, + } + let cases = [ + Case { + desc: "UEFI, EFI_LOADER_INFO advertises systemd-boot", + efi: Ok("systemd-boot 261.2".into()), + bls: false, + grub_dir: false, + expected: Bootloader::Systemd, + }, + Case { + desc: "UEFI, EFI_LOADER_INFO advertises GRUB CC", + efi: Ok("GRUB CC 2.12".into()), + bls: false, + grub_dir: false, + expected: Bootloader::GrubCC, + }, + Case { + desc: "UEFI, EFI_LOADER_INFO advertises unknown; default GRUB", + efi: Ok("something else 1.0".into()), + bls: false, + grub_dir: false, + expected: Bootloader::Grub, + }, + Case { + desc: "Non-EFI + BLS layout present: BLS (regression fix)", + efi: Err(EfiError::SystemNotUEFI), + bls: true, + grub_dir: false, + expected: Bootloader::Systemd, + }, + Case { + desc: "Non-EFI + no BLS layout: fall back to GRUB", + efi: Err(EfiError::SystemNotUEFI), + bls: false, + grub_dir: false, + expected: Bootloader::Grub, + }, + Case { + desc: "EFI mounted but EFI_LOADER_INFO missing, BLS present", + efi: Err(EfiError::MissingVar), + bls: true, + grub_dir: false, + expected: Bootloader::Systemd, + }, + Case { + desc: "EFI mounted but EFI_LOADER_INFO missing, no BLS: GRUB", + efi: Err(EfiError::MissingVar), + bls: false, + grub_dir: false, + expected: Bootloader::Grub, + }, + // A legacy-BIOS Fedora/RHEL install with GRUB_ENABLE_BLSCFG=true + // has both directories and is unambiguously GRUB. Without the + // GRUB probe this case returned Systemd, which is the + // misclassification raised in review on #2376. + Case { + desc: "Non-EFI + BLS layout + GRUB dir: GRUB wins (blscfg)", + efi: Err(EfiError::SystemNotUEFI), + bls: true, + grub_dir: true, + expected: Bootloader::Grub, + }, + Case { + desc: "Non-EFI + GRUB dir, no BLS: GRUB", + efi: Err(EfiError::SystemNotUEFI), + bls: false, + grub_dir: true, + expected: Bootloader::Grub, + }, + Case { + desc: "EFI_LOADER_INFO missing + BLS + GRUB dir: GRUB wins", + efi: Err(EfiError::MissingVar), + bls: true, + grub_dir: true, + expected: Bootloader::Grub, + }, + // The regression this PR fixes must survive the new probe: a + // BLS layout with no GRUB directory is still BLS-native. + Case { + desc: "Non-EFI + BLS, no GRUB dir: still BLS (Pi 5, U-Boot)", + efi: Err(EfiError::SystemNotUEFI), + bls: true, + grub_dir: false, + expected: Bootloader::Systemd, + }, + // UEFI classification must ignore both probes entirely. + Case { + desc: "UEFI systemd-boot with a stray GRUB dir: still systemd", + efi: Ok("systemd-boot 261.2".into()), + bls: true, + grub_dir: true, + expected: Bootloader::Systemd, + }, + ]; + for case in cases { + let got = classify_bootloader(case.efi, case.bls, case.grub_dir) + .unwrap_or_else(|e| panic!("{}: {e}", case.desc)); + assert_eq!(got, case.expected, "{}", case.desc); + } + } + + #[test] + fn classify_bootloader_propagates_other_efi_errors() { + let result = classify_bootloader( + Err(EfiError::InvalidData("test-only synthetic error")), + false, + false, + ); + assert!(result.is_err(), "InvalidData should propagate as an error"); + } + #[test] fn test_sorted_bls_boot_entries() -> Result<()> { let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?;