diff --git a/crates/bls/src/lib.rs b/crates/bls/src/lib.rs index cac52e2..94148f0 100644 --- a/crates/bls/src/lib.rs +++ b/crates/bls/src/lib.rs @@ -203,7 +203,13 @@ impl BlsEntry { /// Fetches the load options to pass to the image that is chainloaded. /// A unified kernel image with a profile gets `@` first, which selects the profile. pub fn chainload_options(&self) -> Option { - match (self.is_uki_only(), self.profile.as_deref(), self.options()) { + // Only a profile that is a number above zero selects a profile, as in systemd-boot. + let profile = self + .profile + .as_deref() + .and_then(parse_digits) + .filter(|profile| *profile > 0); + match (self.is_uki_only(), profile, self.options()) { (true, Some(profile), Some(options)) => Some(format!("@{} {}", profile, options)), (true, Some(profile), None) => Some(format!("@{}", profile)), (_, _, options) => options, @@ -273,20 +279,14 @@ pub struct BootCounter { pub tries_left: u32, /// The number of boot attempts already made. pub tries_done: u32, - /// The number of digits the tries left counter is written with, including leading zeros. - left_width: usize, - /// The number of digits the tries done counter is written with, including leading zeros. - done_width: usize, } impl BootCounter { - /// Creates a counter whose digit widths are those of the values written plainly. + /// Creates a counter with the given number of tries left and tries done. pub fn new(tries_left: u32, tries_done: u32) -> Self { Self { tries_left, tries_done, - left_width: decimal_width(tries_left), - done_width: decimal_width(tries_done), } } @@ -302,43 +302,25 @@ impl BootCounter { return None; } let (left, done) = suffix.split_once('-').unwrap_or((suffix, "0")); - Some(( - id, - Self { - tries_left: parse_digits(left)?, - tries_done: parse_digits(done)?, - left_width: left.len(), - done_width: done.len(), - }, - )) + Some((id, Self::new(parse_digits(left)?, parse_digits(done)?))) } - /// Produces the counter after one more boot attempt. The digit widths are kept, so the - /// tries done counter stops at the largest number that fits in its width. + /// Produces the counter after one more boot attempt. pub fn decremented(&self) -> Self { - let done_limit = 10u32 - .checked_pow(self.done_width as u32) - .map_or(u32::MAX, |limit| limit - 1); - Self { - tries_left: self.tries_left.saturating_sub(1), - tries_done: self.tries_done.saturating_add(1).min(done_limit), - ..*self - } + Self::new( + self.tries_left.saturating_sub(1), + self.tries_done.saturating_add(1), + ) } /// Renders the entry file name stem for the entry `id` with this counter. + /// The counters are plain decimals, without leading zeros, as systemd-boot writes them and + /// as systemd-bless-boot can read them. pub fn render(&self, id: &str) -> String { if self.tries_done == 0 { - format!("{}+{:0left$}", id, self.tries_left, left = self.left_width) + format!("{}+{}", id, self.tries_left) } else { - format!( - "{}+{:0left$}-{:0done$}", - id, - self.tries_left, - self.tries_done, - left = self.left_width, - done = self.done_width - ) + format!("{}+{}-{}", id, self.tries_left, self.tries_done) } } @@ -348,13 +330,6 @@ impl BootCounter { } } -/// The number of digits needed to write `value`. -fn decimal_width(value: u32) -> usize { - value - .checked_ilog10() - .map_or(1, |digits| digits as usize + 1) -} - /// Decides which entries are the default. Each item of `entries` is `(is_default, is_bad)`, /// and the result says whether each entry is the default afterwards. /// Bad entries, which have no boot counter tries left, are never the default while a good entry @@ -388,6 +363,13 @@ pub fn strip_extension<'a>(name: &'a str, extension: &str) -> Option<(&'a str, & Some(name.split_at(split)) } +/// Whether the entry file `name` is reserved for the entries that the boot loader makes itself, +/// which are named with `auto-`. Such files are not entries. +pub fn is_reserved_entry_name(name: &str) -> bool { + name.get(..5) + .is_some_and(|prefix| prefix.eq_ignore_ascii_case("auto-")) +} + /// Converts an EFI `path` to the form used by the bootloader interface: separated by forward /// slashes, with a leading slash and no repeated or trailing separators. pub fn boot_path(path: &str) -> String { @@ -834,6 +816,15 @@ mod tests { assert_eq!(sections[".uname"], b"6.5"); } + #[test] + fn auto_names_are_reserved_ignoring_case() { + assert!(is_reserved_entry_name("auto-windows.conf")); + assert!(is_reserved_entry_name("AUTO-x.efi")); + assert!(!is_reserved_entry_name("autox.conf")); + assert!(!is_reserved_entry_name("fedora-auto-1.conf")); + assert!(!is_reserved_entry_name("aut")); + } + #[test] fn strip_extension_ignores_case_and_keeps_the_original_extension() { assert_eq!( @@ -1115,6 +1106,20 @@ mod tests { assert!(entry.matches_architecture("aa64")); } + #[test] + fn uki_profile_zero_and_invalid_profiles_add_no_prefix() { + for profile in ["0", "x", "-1", "2x", ""] { + let entry: BlsEntry = format!("uki /a.efi\nprofile {profile}\noptions quiet\n") + .parse() + .unwrap(); + assert_eq!( + entry.chainload_options().as_deref(), + Some("quiet"), + "{profile}" + ); + } + } + #[test] fn uki_profile_prefixes_the_load_options() { let entry: BlsEntry = "uki /a.efi\nprofile 2\noptions quiet\n".parse().unwrap(); @@ -1185,18 +1190,14 @@ mod tests { } #[test] - fn boot_counter_keeps_the_width_of_the_left_counter() { - let (id, counter) = BootCounter::parse("foo+10"); - assert_eq!(counter.unwrap().decremented().render(id), "foo+09-1"); - } - - #[test] - fn boot_counter_pads_and_caps_the_done_counter() { + fn boot_counter_renders_plain_decimals_that_systemd_can_parse() { + // Leading zeros would be read as octal by systemd-bless-boot, so they are never written. for (stem, expected) in [ - ("foo+5-08", "foo+4-09"), - ("foo+5-99", "foo+4-99"), - ("foo+5-9", "foo+4-9"), + ("foo+10", "foo+9-1"), + ("foo+5-08", "foo+4-9"), + ("foo+5-9", "foo+4-10"), ("foo+0-3", "foo+0-4"), + ("foo+3", "foo+2-1"), ] { let (id, counter) = BootCounter::parse(stem); assert_eq!( diff --git a/crates/boot/src/generators/bls.rs b/crates/boot/src/generators/bls.rs index 635344b..1dad3b5 100644 --- a/crates/boot/src/generators/bls.rs +++ b/crates/boot/src/generators/bls.rs @@ -9,7 +9,7 @@ use alloc::{ }; use anyhow::{Context, Result}; use core::{cmp::Ordering, str::FromStr}; -use edera_sprout_bls::{BlsEntry, BootCounter, sort_bls, strip_extension}; +use edera_sprout_bls::{BlsEntry, BootCounter, is_reserved_entry_name, sort_bls, strip_extension}; use edera_sprout_config::generators::bls::BlsConfiguration; use log::{info, warn}; use uefi::{ @@ -187,7 +187,7 @@ fn generate_type1( continue; } // Names that start with auto- are reserved for entries the boot loader makes itself. - if file_name.starts_with("auto-") { + if is_reserved_entry_name(&file_name) { continue; } let extension = extension.to_string(); @@ -315,6 +315,9 @@ fn generate_type2( // The id is the file name without the extension and the boot counter. let file_name = image.file_name; + if is_reserved_entry_name(&file_name) { + continue; + } let Some((stem, extension)) = strip_extension(&file_name, ".efi") else { continue; }; diff --git a/crates/boot/src/main.rs b/crates/boot/src/main.rs index de90d14..26973cf 100644 --- a/crates/boot/src/main.rs +++ b/crates/boot/src/main.rs @@ -477,7 +477,7 @@ fn run() -> Result<()> { // no entry, such as when the entry was removed, is ignored so that the next source is used. // The preferred entry sources never use an entry with no boot counter tries left. // Each source has whether it matches by id, and whether it skips bad entries. - for (source, pattern, by_id, skip_bad) in [ + for (source, value, by_id, skip_bad) in [ ( "configuration", config.options.default_entry.clone(), @@ -486,29 +486,31 @@ fn run() -> Result<()> { ), ( "bootloader interface preferred", - entry_pattern(bootloader_interface_preferred_entry), + bootloader_interface_preferred_entry.clone(), true, true, ), ( "loader.conf preferred", - entry_pattern(loader_conf.preferred.clone()), + loader_conf.preferred.clone(), true, true, ), ( "bootloader interface default", - entry_pattern(bootloader_interface_default_entry.clone()), + bootloader_interface_default_entry.clone(), true, false, ), ( "loader.conf default", - entry_pattern(loader_conf.default.clone()), + loader_conf.default.clone(), true, false, ), ] { + // The configuration is a pattern as it is. The other sources can be `@saved`. + let pattern = if by_id { entry_pattern(value) } else { value }; let Some(pattern) = pattern else { continue; }; @@ -618,7 +620,10 @@ fn run() -> Result<()> { if forced_entry.is_none() { let id = entry.id(); if use_saved_entry { - if last_booted_entry.as_deref() != Some(id.as_str()) { + if !last_booted_entry + .as_deref() + .is_some_and(|saved| saved.eq_ignore_ascii_case(&id)) + { advisory( BootloaderInterface::set_last_booted_entry(&id) .context("unable to save last booted entry in bootloader interface"), diff --git a/crates/parsing/src/lib.rs b/crates/parsing/src/lib.rs index 667aa55..8922a00 100644 --- a/crates/parsing/src/lib.rs +++ b/crates/parsing/src/lib.rs @@ -216,9 +216,11 @@ enum FnmatchToken { Star, /// `[...]`, which matches a byte in any of the inclusive ranges, already lowercased. Class(Vec<(u8, u8)>), + /// A pattern that can't match anything, such as one with an unterminated `[`. + Never, } -/// Splits `pattern` into tokens. An unterminated `[` matches itself. +/// Splits `pattern` into tokens. An unterminated `[` never matches, as in systemd-boot. fn fnmatch_tokens(pattern: &[u8]) -> Vec { let mut tokens = Vec::new(); let mut index = 0; @@ -250,7 +252,7 @@ fn fnmatch_tokens(pattern: &[u8]) -> Vec { tokens.push(FnmatchToken::Class(ranges)); index += length + 1; } - _ => tokens.push(FnmatchToken::Literal(b'[')), + _ => tokens.push(FnmatchToken::Never), }, other => tokens.push(FnmatchToken::Literal(other.to_ascii_lowercase())), } @@ -282,7 +284,7 @@ pub fn fnmatch_ignore_case(pattern: &str, text: &str) -> bool { p += 1; continue; } - None => false, + Some(FnmatchToken::Never) | None => false, }; if matched { p += 1; @@ -711,9 +713,10 @@ mod tests { } #[test] - fn fnmatch_unterminated_bracket_is_a_literal() { - assert!(fnmatch_ignore_case("a[b", "a[b")); + fn fnmatch_unterminated_bracket_matches_nothing() { + assert!(!fnmatch_ignore_case("a[b", "a[b")); assert!(!fnmatch_ignore_case("a[b", "ab")); + assert!(!fnmatch_ignore_case("a[", "a[")); } #[test]