mirror of
https://github.com/edera-dev/sprout.git
synced 2026-11-04 17:42:20 +00:00
fix(bls): write boot counters systemd can read and match systemd on small details
Do not pad the boot counter digits or cap the tries done counter, as systemd-bless-boot reads a counter with a leading zero as octal and systemd-boot never writes one. A profile only selects a profile when it is a number above zero, an unterminated bracket in a pattern matches nothing, and files that start with auto- are ignored in any case and for unified kernel images too. The saved entry is compared ignoring case, and entry settings are only expanded for sources that are looked at.
This commit is contained in:
1 parent
8bb5f1132d
commit
14bcfc2653
4 files changed
+78
-66
No files matched your search
+54
-53
@@ -203,7 +203,13 @@ impl BlsEntry {
|
|||||||
/// Fetches the load options to pass to the image that is chainloaded.
|
/// Fetches the load options to pass to the image that is chainloaded.
|
||||||
/// A unified kernel image with a profile gets `@<profile>` first, which selects the profile.
|
/// A unified kernel image with a profile gets `@<profile>` first, which selects the profile.
|
||||||
pub fn chainload_options(&self) -> Option<String> {
|
pub fn chainload_options(&self) -> Option<String> {
|
||||||
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), Some(options)) => Some(format!("@{} {}", profile, options)),
|
||||||
(true, Some(profile), None) => Some(format!("@{}", profile)),
|
(true, Some(profile), None) => Some(format!("@{}", profile)),
|
||||||
(_, _, options) => options,
|
(_, _, options) => options,
|
||||||
@@ -273,20 +279,14 @@ pub struct BootCounter {
|
|||||||
pub tries_left: u32,
|
pub tries_left: u32,
|
||||||
/// The number of boot attempts already made.
|
/// The number of boot attempts already made.
|
||||||
pub tries_done: u32,
|
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 {
|
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 {
|
pub fn new(tries_left: u32, tries_done: u32) -> Self {
|
||||||
Self {
|
Self {
|
||||||
tries_left,
|
tries_left,
|
||||||
tries_done,
|
tries_done,
|
||||||
left_width: decimal_width(tries_left),
|
|
||||||
done_width: decimal_width(tries_done),
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -302,43 +302,25 @@ impl BootCounter {
|
|||||||
return None;
|
return None;
|
||||||
}
|
}
|
||||||
let (left, done) = suffix.split_once('-').unwrap_or((suffix, "0"));
|
let (left, done) = suffix.split_once('-').unwrap_or((suffix, "0"));
|
||||||
Some((
|
Some((id, Self::new(parse_digits(left)?, parse_digits(done)?)))
|
||||||
id,
|
|
||||||
Self {
|
|
||||||
tries_left: parse_digits(left)?,
|
|
||||||
tries_done: parse_digits(done)?,
|
|
||||||
left_width: left.len(),
|
|
||||||
done_width: done.len(),
|
|
||||||
},
|
|
||||||
))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Produces the counter after one more boot attempt. The digit widths are kept, so the
|
/// Produces the counter after one more boot attempt.
|
||||||
/// tries done counter stops at the largest number that fits in its width.
|
|
||||||
pub fn decremented(&self) -> Self {
|
pub fn decremented(&self) -> Self {
|
||||||
let done_limit = 10u32
|
Self::new(
|
||||||
.checked_pow(self.done_width as u32)
|
self.tries_left.saturating_sub(1),
|
||||||
.map_or(u32::MAX, |limit| limit - 1);
|
self.tries_done.saturating_add(1),
|
||||||
Self {
|
)
|
||||||
tries_left: self.tries_left.saturating_sub(1),
|
|
||||||
tries_done: self.tries_done.saturating_add(1).min(done_limit),
|
|
||||||
..*self
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Renders the entry file name stem for the entry `id` with this counter.
|
/// 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 {
|
pub fn render(&self, id: &str) -> String {
|
||||||
if self.tries_done == 0 {
|
if self.tries_done == 0 {
|
||||||
format!("{}+{:0left$}", id, self.tries_left, left = self.left_width)
|
format!("{}+{}", id, self.tries_left)
|
||||||
} else {
|
} else {
|
||||||
format!(
|
format!("{}+{}-{}", id, self.tries_left, self.tries_done)
|
||||||
"{}+{:0left$}-{:0done$}",
|
|
||||||
id,
|
|
||||||
self.tries_left,
|
|
||||||
self.tries_done,
|
|
||||||
left = self.left_width,
|
|
||||||
done = self.done_width
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -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)`,
|
/// 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.
|
/// 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
|
/// 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))
|
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
|
/// 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.
|
/// slashes, with a leading slash and no repeated or trailing separators.
|
||||||
pub fn boot_path(path: &str) -> String {
|
pub fn boot_path(path: &str) -> String {
|
||||||
@@ -834,6 +816,15 @@ mod tests {
|
|||||||
assert_eq!(sections[".uname"], b"6.5");
|
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]
|
#[test]
|
||||||
fn strip_extension_ignores_case_and_keeps_the_original_extension() {
|
fn strip_extension_ignores_case_and_keeps_the_original_extension() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
@@ -1115,6 +1106,20 @@ mod tests {
|
|||||||
assert!(entry.matches_architecture("aa64"));
|
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]
|
#[test]
|
||||||
fn uki_profile_prefixes_the_load_options() {
|
fn uki_profile_prefixes_the_load_options() {
|
||||||
let entry: BlsEntry = "uki /a.efi\nprofile 2\noptions quiet\n".parse().unwrap();
|
let entry: BlsEntry = "uki /a.efi\nprofile 2\noptions quiet\n".parse().unwrap();
|
||||||
@@ -1185,18 +1190,14 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn boot_counter_keeps_the_width_of_the_left_counter() {
|
fn boot_counter_renders_plain_decimals_that_systemd_can_parse() {
|
||||||
let (id, counter) = BootCounter::parse("foo+10");
|
// Leading zeros would be read as octal by systemd-bless-boot, so they are never written.
|
||||||
assert_eq!(counter.unwrap().decremented().render(id), "foo+09-1");
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn boot_counter_pads_and_caps_the_done_counter() {
|
|
||||||
for (stem, expected) in [
|
for (stem, expected) in [
|
||||||
("foo+5-08", "foo+4-09"),
|
("foo+10", "foo+9-1"),
|
||||||
("foo+5-99", "foo+4-99"),
|
("foo+5-08", "foo+4-9"),
|
||||||
("foo+5-9", "foo+4-9"),
|
("foo+5-9", "foo+4-10"),
|
||||||
("foo+0-3", "foo+0-4"),
|
("foo+0-3", "foo+0-4"),
|
||||||
|
("foo+3", "foo+2-1"),
|
||||||
] {
|
] {
|
||||||
let (id, counter) = BootCounter::parse(stem);
|
let (id, counter) = BootCounter::parse(stem);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ use alloc::{
|
|||||||
};
|
};
|
||||||
use anyhow::{Context, Result};
|
use anyhow::{Context, Result};
|
||||||
use core::{cmp::Ordering, str::FromStr};
|
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 edera_sprout_config::generators::bls::BlsConfiguration;
|
||||||
use log::{info, warn};
|
use log::{info, warn};
|
||||||
use uefi::{
|
use uefi::{
|
||||||
@@ -187,7 +187,7 @@ fn generate_type1(
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
// Names that start with auto- are reserved for entries the boot loader makes itself.
|
// 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;
|
continue;
|
||||||
}
|
}
|
||||||
let extension = extension.to_string();
|
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.
|
// The id is the file name without the extension and the boot counter.
|
||||||
let file_name = image.file_name;
|
let file_name = image.file_name;
|
||||||
|
if is_reserved_entry_name(&file_name) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
let Some((stem, extension)) = strip_extension(&file_name, ".efi") else {
|
let Some((stem, extension)) = strip_extension(&file_name, ".efi") else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
|
|||||||
+11
-6
@@ -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.
|
// 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.
|
// 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.
|
// 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",
|
"configuration",
|
||||||
config.options.default_entry.clone(),
|
config.options.default_entry.clone(),
|
||||||
@@ -486,29 +486,31 @@ fn run() -> Result<()> {
|
|||||||
),
|
),
|
||||||
(
|
(
|
||||||
"bootloader interface preferred",
|
"bootloader interface preferred",
|
||||||
entry_pattern(bootloader_interface_preferred_entry),
|
bootloader_interface_preferred_entry.clone(),
|
||||||
true,
|
true,
|
||||||
true,
|
true,
|
||||||
),
|
),
|
||||||
(
|
(
|
||||||
"loader.conf preferred",
|
"loader.conf preferred",
|
||||||
entry_pattern(loader_conf.preferred.clone()),
|
loader_conf.preferred.clone(),
|
||||||
true,
|
true,
|
||||||
true,
|
true,
|
||||||
),
|
),
|
||||||
(
|
(
|
||||||
"bootloader interface default",
|
"bootloader interface default",
|
||||||
entry_pattern(bootloader_interface_default_entry.clone()),
|
bootloader_interface_default_entry.clone(),
|
||||||
true,
|
true,
|
||||||
false,
|
false,
|
||||||
),
|
),
|
||||||
(
|
(
|
||||||
"loader.conf default",
|
"loader.conf default",
|
||||||
entry_pattern(loader_conf.default.clone()),
|
loader_conf.default.clone(),
|
||||||
true,
|
true,
|
||||||
false,
|
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 {
|
let Some(pattern) = pattern else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
@@ -618,7 +620,10 @@ fn run() -> Result<()> {
|
|||||||
if forced_entry.is_none() {
|
if forced_entry.is_none() {
|
||||||
let id = entry.id();
|
let id = entry.id();
|
||||||
if use_saved_entry {
|
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(
|
advisory(
|
||||||
BootloaderInterface::set_last_booted_entry(&id)
|
BootloaderInterface::set_last_booted_entry(&id)
|
||||||
.context("unable to save last booted entry in bootloader interface"),
|
.context("unable to save last booted entry in bootloader interface"),
|
||||||
|
|||||||
@@ -216,9 +216,11 @@ enum FnmatchToken {
|
|||||||
Star,
|
Star,
|
||||||
/// `[...]`, which matches a byte in any of the inclusive ranges, already lowercased.
|
/// `[...]`, which matches a byte in any of the inclusive ranges, already lowercased.
|
||||||
Class(Vec<(u8, u8)>),
|
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<FnmatchToken> {
|
fn fnmatch_tokens(pattern: &[u8]) -> Vec<FnmatchToken> {
|
||||||
let mut tokens = Vec::new();
|
let mut tokens = Vec::new();
|
||||||
let mut index = 0;
|
let mut index = 0;
|
||||||
@@ -250,7 +252,7 @@ fn fnmatch_tokens(pattern: &[u8]) -> Vec<FnmatchToken> {
|
|||||||
tokens.push(FnmatchToken::Class(ranges));
|
tokens.push(FnmatchToken::Class(ranges));
|
||||||
index += length + 1;
|
index += length + 1;
|
||||||
}
|
}
|
||||||
_ => tokens.push(FnmatchToken::Literal(b'[')),
|
_ => tokens.push(FnmatchToken::Never),
|
||||||
},
|
},
|
||||||
other => tokens.push(FnmatchToken::Literal(other.to_ascii_lowercase())),
|
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;
|
p += 1;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
None => false,
|
Some(FnmatchToken::Never) | None => false,
|
||||||
};
|
};
|
||||||
if matched {
|
if matched {
|
||||||
p += 1;
|
p += 1;
|
||||||
@@ -711,9 +713,10 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn fnmatch_unterminated_bracket_is_a_literal() {
|
fn fnmatch_unterminated_bracket_matches_nothing() {
|
||||||
assert!(fnmatch_ignore_case("a[b", "a[b"));
|
assert!(!fnmatch_ignore_case("a[b", "a[b"));
|
||||||
assert!(!fnmatch_ignore_case("a[b", "ab"));
|
assert!(!fnmatch_ignore_case("a[b", "ab"));
|
||||||
|
assert!(!fnmatch_ignore_case("a[", "a["));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
Reference in new issue
Block a user