From 4e10d5714302bba893dfd8a1da17946c26cbbbb2 Mon Sep 17 00:00:00 2001 From: Alex Zenla Date: Sat, 3 Oct 2026 20:19:21 -0700 Subject: [PATCH] fix(boot): tidy the devicetree, reboot and profile handling after review Only warn about a devicetree under Secure Boot when the entry has one, skip the fixup if its protocol can't be opened, and reject a devicetree whose size is smaller than its header. Announce a reboot before the delay, advertise the multi-profile UKI feature, and treat a Type 1 entry with a profile after the first as an extra profile that is not picked as the fallback default. A failure to read Sprout's own partition no longer drops the entries of the XBOOTLDR partition, and the README lists every initrd slot and describes the reboot behaviour more exactly. --- README.md | 14 ++++++++--- crates/boot/src/actions/chainload.rs | 29 ++++++++++++++++------ crates/boot/src/entries.rs | 9 ++++++- crates/boot/src/generators/bls.rs | 15 +++++++++-- crates/boot/src/main.rs | 10 ++++++-- crates/config/src/generators/bls.rs | 3 ++- crates/eficore/src/bootloader_interface.rs | 1 + crates/eficore/src/devicetree.rs | 11 +++++--- 8 files changed, 72 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index abff499..be215bf 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ We recommend running Sprout without Secure Boot for development, and with Secure - [x] [Bootloader specification (BLS)](https://uapi-group.org/specifications/specs/boot_loader_specification/) support: Type #1 entries, Type #2 unified kernel images, and the extended boot loader partition - [x] [UKI support](https://github.com/edera-dev/sprout/issues/6): beta, including images with multiple profiles -- [x] Boot counting and automatic boot assessment +- [x] Boot counting, so `systemd-bless-boot` can assess a boot - [x] `loader.conf` support - [x] Chainload support - [x] Linux boot support via EFI stub @@ -209,9 +209,16 @@ bls.entry.actions = ["boot-bls"] chainload.path = "$entry-root\\$chainload" chainload.options = ["$options"] chainload.devicetree = "$entry-root\\$devicetree" +# an entry can have up to eight initrds. unused ones are skipped. chainload.linux-initrd-chain = [ "$entry-root\\$initrd-0", "$entry-root\\$initrd-1", + "$entry-root\\$initrd-2", + "$entry-root\\$initrd-3", + "$entry-root\\$initrd-4", + "$entry-root\\$initrd-5", + "$entry-root\\$initrd-6", + "$entry-root\\$initrd-7", ] ``` @@ -224,8 +231,9 @@ An entry file named like `fedora+3.conf` or `fedora+3.efi` has three tries. Each file is renamed, such as to `fedora+2-1.conf`, and the new path is given to the system in `LoaderBootCountPath` so `systemd-bless-boot` can remove the counter once the boot works. Entries that have no tries left are sorted last and are not picked as the default entry, but they can still be -booted by hand. If an entry with tries left fails to start, the machine resets, so the next boot can use -the next try or another entry. This is set by `reboot-on-error` in `loader.conf`. +booted by hand. By default, if an entry fails to start after a try was used up and it had tries left, the machine +resets, so the next boot can use the next try or another entry. This is the `reboot-on-error` setting +in `loader.conf`. Without boot counting, a failure to start returns to the firmware. #### loader.conf diff --git a/crates/boot/src/actions/chainload.rs b/crates/boot/src/actions/chainload.rs index 5af8c2c..ca551f4 100644 --- a/crates/boot/src/actions/chainload.rs +++ b/crates/boot/src/actions/chainload.rs @@ -13,6 +13,7 @@ use eficore::loader::source::ImageSource; use eficore::loader::{ImageLoadRequest, ImageLoader}; use eficore::media_loader::MediaLoaderHandle; use eficore::media_loader::constants::linux::LINUX_EFI_INITRD_MEDIA_GUID; +use eficore::path::ResolvedPath; use eficore::secure::SecureBoot; use log::warn; use uefi::proto::loaded_image::LoadedImage; @@ -31,6 +32,22 @@ fn read_optional_file( path: &str, what: &str, ) -> Result>> { + let Some(resolved) = resolve_optional_file(context, path, what)? else { + return Ok(None); + }; + let content = resolved + .read_file() + .with_context(|| format!("unable to read {}", what))?; + Ok(Some(content)) +} + +/// Resolve the `what` at the stamped `path` relative to the sprout image. +/// Provides [None] if the path refers to the root of a filesystem rather than a file. +fn resolve_optional_file( + context: &Rc, + path: &str, + what: &str, +) -> Result> { let resolved = eficore::path::resolve_path(Some(context.root().loaded_image_path()?), path) .with_context(|| format!("unable to resolve {} path", what))?; @@ -42,11 +59,7 @@ fn read_optional_file( if subpath.trim_matches('\\').is_empty() { return Ok(None); } - - let content = resolved - .read_file() - .with_context(|| format!("unable to read {}", what))?; - Ok(Some(content)) + Ok(Some(resolved)) } /// Unloads the image with the contained handle when dropped, unless the handle is taken. @@ -167,10 +180,12 @@ pub fn chainload(context: Rc, configuration: &ChainloadConfigurat .devicetree .as_ref() .map(|path| context.stamp(path)), - ) { + ) && let Some(resolved) = resolve_optional_file(&context, &path, "devicetree")? + { if SecureBoot::enabled().unwrap_or(true) { warn!("ignoring the devicetree, as Secure Boot is enabled"); - } else if let Some(content) = read_optional_file(&context, &path, "devicetree")? { + } else { + let content = resolved.read_file().context("unable to read devicetree")?; devicetree = Some(DeviceTree::install(&content).context("unable to install the devicetree")?); } diff --git a/crates/boot/src/entries.rs b/crates/boot/src/entries.rs index b0d4923..b1ca4c6 100644 --- a/crates/boot/src/entries.rs +++ b/crates/boot/src/entries.rs @@ -20,6 +20,7 @@ pub struct BootableEntry { boot_counter: Option, id_suffix: Option, id_profile: Option, + extra_profile: bool, } impl BootableEntry { @@ -41,6 +42,7 @@ impl BootableEntry { boot_counter: None, id_suffix: None, id_profile: None, + extra_profile: false, } } @@ -78,7 +80,12 @@ impl BootableEntry { /// Fetch whether the entry is a profile of a unified kernel image after the first, which /// is never picked as the default entry unless it was asked for. pub fn is_extra_profile(&self) -> bool { - self.id_profile.is_some() + self.id_profile.is_some() || self.extra_profile + } + + /// Mark this entry as booting a profile of a unified kernel image after the first. + pub fn mark_extra_profile(&mut self) { + self.extra_profile = true; } /// Set the profile of a unified kernel image that is part of the id of this entry. diff --git a/crates/boot/src/generators/bls.rs b/crates/boot/src/generators/bls.rs index 56c493e..64d85a5 100644 --- a/crates/boot/src/generators/bls.rs +++ b/crates/boot/src/generators/bls.rs @@ -266,6 +266,9 @@ fn generate_type1( let mut boot = bootable_entry(context, bls, &name, &entry, &initrds, entry_root); boot.set_id_suffix(&extension); + if entry.profile_number() > 0 { + boot.mark_extra_profile(); + } // Record where the boot counter lives so a try can be consumed when this entry boots. if let Some(counter) = boot_counter { @@ -458,13 +461,21 @@ pub fn generate(context: Rc, bls: &BlsConfiguration) -> Result entries, + Err(error) if bls.xbootldr => { + warn!("unable to generate bls entries: {:#}", error); + Vec::new() + } + Err(error) => return Err(error), + }; // Add the entries of the Extended Boot Loader Partition, which are sorted together with the // others. A problem with it should not prevent booting from the entries that are found. diff --git a/crates/boot/src/main.rs b/crates/boot/src/main.rs index e1780ff..586bb09 100644 --- a/crates/boot/src/main.rs +++ b/crates/boot/src/main.rs @@ -706,12 +706,18 @@ fn efi_main() -> Status { for (index, stack) in error.chain().enumerate() { error!("[{}]: {}", index, stack); } - // Sleep to allow the user to read the error. + // Sleep to allow the user to read the error. A reboot is announced first, so it is + // known why the machine is about to reset. + if reboot_on_error { + error!( + "rebooting in {} seconds after a failure to start the boot entry", + DELAY_ON_ERROR.as_secs() + ); + } uefi::boot::stall(DELAY_ON_ERROR); // Reboot when asked to, so that the next boot can use the next try or entry. if reboot_on_error { - error!("rebooting after a failure to start the boot entry"); uefi::runtime::reset(ResetType::COLD, Status::SUCCESS, None); } return Status::ABORTED; diff --git a/crates/config/src/generators/bls.rs b/crates/config/src/generators/bls.rs index be5d665..a4cbffa 100644 --- a/crates/config/src/generators/bls.rs +++ b/crates/config/src/generators/bls.rs @@ -29,7 +29,8 @@ pub struct BlsConfiguration { /// Whether to also read the Extended Boot Loader Partition, which is the partition with the /// XBOOTLDR type on the same disk as the partition Sprout was loaded from. Its entries are /// sorted with the others, and their paths are on that partition, so an action has to use - /// the `$entry-root` value to find them, such as `$entry-root\\$chainload`. + /// the `$entry-root` value to find them, such as `$entry-root\\$chainload`. Its unified kernel + /// images are always in `\\EFI\\Linux`. #[serde(default)] pub xbootldr: bool, /// Whether generated entries keep the name of the BLS entry file as-is. diff --git a/crates/eficore/src/bootloader_interface.rs b/crates/eficore/src/bootloader_interface.rs index 02f9630..feafc22 100644 --- a/crates/eficore/src/bootloader_interface.rs +++ b/crates/eficore/src/bootloader_interface.rs @@ -68,6 +68,7 @@ impl BootloaderInterface { | LoaderFeatures::EntryPreferred | LoaderFeatures::SortKey | LoaderFeatures::DeviceTree + | LoaderFeatures::MultiProfileUki } /// Tell the system that Sprout was initialized at the current time. diff --git a/crates/eficore/src/devicetree.rs b/crates/eficore/src/devicetree.rs index 38e84d2..d3a2b54 100644 --- a/crates/eficore/src/devicetree.rs +++ b/crates/eficore/src/devicetree.rs @@ -67,8 +67,8 @@ impl DeviceTree { if magic != FDT_MAGIC { bail!("the file is not a flattened devicetree"); } - if total > dtb.len() { - bail!("the devicetree is truncated"); + if total < FDT_HEADER_SIZE || total > dtb.len() { + bail!("the devicetree has an invalid size"); } Ok(()) } @@ -127,8 +127,11 @@ impl DeviceTree { let Some(handle) = find_handle(&DT_FIXUP_GUID)? else { return Ok(()); }; - let mut protocol = uefi::boot::open_protocol_exclusive::(handle) - .context("unable to open the devicetree fixup protocol")?; + // Without the protocol the devicetree is used as it is, as in systemd-boot. + let Ok(mut protocol) = crate::handle::open_shared::(handle) else { + warn!("unable to open the devicetree fixup protocol, skipping the fixup"); + return Ok(()); + }; let fixup = protocol.fixup; let this: *mut DtFixupProtocol = &mut *protocol; let flags = DT_APPLY_FIXUPS | DT_RESERVE_MEMORY;