From 34ea3cbada4fd60945c3ecc762305a38a91992ab Mon Sep 17 00:00:00 2001 From: Dustin Kirkland Date: Sun, 9 Aug 2026 15:44:54 -0500 Subject: [PATCH] composefs/status: Detect BLS layout on non-EFI systems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get_bootloader()` unconditionally returns `Bootloader::Grub` when there are no EFI variables to inspect (`SystemNotUEFI` / `MissingVar`). That's wrong for many non-EFI setups that use the Boot Loader Specification Type 1 entry layout at `/boot/loader/entries/` without any EFI vars to advertise it — Raspberry Pi 4/5 with direct- kernel boot from Pi firmware, U-Boot with the extlinux/BLS loader, coreboot chaining to a bare kernel, and various ARM/embedded boards. When bootc misclassifies these as `Bootloader::Grub` → `BootloaderKind::GRUBClassic`, `storage::new` sets `boot_dir = physical_root.open_dir("boot")` = `/sysroot/boot/`. On systems where `/boot` is a separate partition (the ESP mounted at `/boot` via the `systemd.mount-extra=UUID=:/boot:auto:ro` cmdline that `bootc install to-filesystem` itself writes), `/sysroot/boot/` is empty. Every subsequent code path that reads BLS entries via `boot_dir.read_dir("loader/entries")` then `ENOENT`s — including the idempotent `prepend_custom_prefix()` backwards-compat migration called from `storage::new` itself, which is why `bootc status`, `bootc upgrade`, and `bootc switch` all fail at storage init. This bug was masked before #2356 by an EBUSY on the pre-mounted ESP. With that fixed, execution now reaches `prepend_custom_prefix`, which is where the wrong `boot_dir` gets used. Fix: when there are no EFI vars, stat `/boot/loader/entries`. If it is a directory, treat the bootloader as BLS-compatible; otherwise fall back to `Bootloader::Grub` as before. The probe is a single `stat(2)` and the else-branch preserves prior behaviour on real grub-classic systems (where `/boot/grub2/` exists but `/boot/loader/entries/` does not). Split the inner match into a pure `classify_bootloader(efi_result, bls_present) -> Result` helper per REVIEW_RUST.md "separate parsing from I/O" guidance, and add a table-driven unit test covering both prior branches and both new branches. Preserve the pre-existing "don't cache on EFI" behavior of `get_bootloader()`: the old code had an early-return in the `Ok(loader)` branch that bypassed the `OnceLock` cache, and the grub-cc TMT plans observed bootloader-info changes over a run (discovered via `is_composefs` → `bootc status --json` in `tap.nu` after v1 of this PR unified the caching path). The new code caches only when the classification came from the FS probe (non-EFI, filesystem-stable state). Verified on aarch64 with `bootc` built from this branch: before the fix, `bootc status` errored at "Prepending custom prefix to EFI and BLS entries: Getting sorted Type1 boot entries: No such file or directory (os error 2)"; after, it returns a healthy `BootcHost` report with `bootType: Bls`, and `bootc switch --transport=registry` proceeds normally. Assisted-by: Claude (Opus 4) Signed-off-by: Dustin Kirkland Closes: #2375 --- crates/lib/src/bootc_composefs/status.rs | 159 ++++++++++++++++++++--- 1 file changed, 138 insertions(+), 21 deletions(-) diff --git a/crates/lib/src/bootc_composefs/status.rs b/crates/lib/src/bootc_composefs/status.rs index c2a984e794..1245df10ce 100644 --- a/crates/lib/src/bootc_composefs/status.rs +++ b/crates/lib/src/bootc_composefs/status.rs @@ -440,6 +440,55 @@ 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"; + +/// 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. +/// - Other EFI read errors propagate. +fn classify_bootloader( + efi_loader_info: Result, + bls_entries_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 bls_entries_dir_present { + tracing::debug!( + "No EFI vars but {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 +497,28 @@ 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 probe is only consulted in the non-EFI classification + // branch; skip the `stat(2)` on EFI systems. + non_efi && std::path::Path::new(BLS_ENTRIES_DIR).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 +1138,74 @@ mod tests { assert_eq!(v.digest.as_ref(), DIGEST); } + #[test] + fn classify_bootloader_cases() { + struct Case { + desc: &'static str, + efi: Result, + bls: bool, + expected: Bootloader, + } + let cases = [ + Case { + desc: "UEFI, EFI_LOADER_INFO advertises systemd-boot", + efi: Ok("systemd-boot 261.2".into()), + bls: false, + expected: Bootloader::Systemd, + }, + Case { + desc: "UEFI, EFI_LOADER_INFO advertises GRUB CC", + efi: Ok("GRUB CC 2.12".into()), + bls: false, + expected: Bootloader::GrubCC, + }, + Case { + desc: "UEFI, EFI_LOADER_INFO advertises unknown; default GRUB", + efi: Ok("something else 1.0".into()), + bls: false, + expected: Bootloader::Grub, + }, + Case { + desc: "Non-EFI + BLS layout present: BLS (regression fix)", + efi: Err(EfiError::SystemNotUEFI), + bls: true, + expected: Bootloader::Systemd, + }, + Case { + desc: "Non-EFI + no BLS layout: fall back to GRUB", + efi: Err(EfiError::SystemNotUEFI), + bls: false, + expected: Bootloader::Grub, + }, + Case { + desc: "EFI mounted but EFI_LOADER_INFO missing, BLS present", + efi: Err(EfiError::MissingVar), + bls: true, + expected: Bootloader::Systemd, + }, + Case { + desc: "EFI mounted but EFI_LOADER_INFO missing, no BLS: GRUB", + efi: Err(EfiError::MissingVar), + bls: false, + expected: Bootloader::Grub, + }, + ]; + for case in cases { + let got = classify_bootloader(case.efi, case.bls) + .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, + ); + 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())?;