Skip to content

Commit 9b1b696

Browse files
committed
efi: avoid reading global state for get_product_name()
Inspired by coreos/bootupd#685 (comment)
1 parent eea4124 commit 9b1b696

File tree

3 files changed

+152
-7
lines changed

3 files changed

+152
-7
lines changed

Cargo.lock

Lines changed: 111 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ path = "src/main.rs"
2121
[dependencies]
2222
anyhow = "1.0"
2323
bincode = "1.3.2"
24+
cap-std-ext = "4.0.0"
2425
chrono = { version = "0.4.38", features = ["serde"] }
2526
clap = { version = "3.2", default-features = false, features = ["cargo", "derive", "std", "suggestions"] }
2627
env_logger = "0.10"

src/efi.rs

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ use std::path::{Path, PathBuf};
1010
use std::process::Command;
1111

1212
use anyhow::{bail, Context, Result};
13+
use cap_std::fs::Dir;
14+
use cap_std_ext::cap_std;
1315
use fn_error_context::context;
1416
use openat_ext::OpenatDirExt;
1517
use os_release::OsRelease;
@@ -139,7 +141,8 @@ impl Efi {
139141
log::debug!("Not booted via EFI, skipping firmware update");
140142
return Ok(());
141143
}
142-
let product_name = get_product_name()?;
144+
let sysroot = Dir::open_ambient_dir(&Path::new("/"), cap_std::ambient_authority())?;
145+
let product_name = get_product_name(&sysroot)?;
143146
log::debug!("Get product name: {product_name}");
144147
assert!(product_name.len() > 0);
145148
// clear all the boot entries that match the target name
@@ -149,10 +152,10 @@ impl Efi {
149152
}
150153

151154
#[context("Get product name")]
152-
fn get_product_name() -> Result<String> {
153-
let file_path = Path::new("/etc/system-release");
154-
if file_path.exists() {
155-
let content = std::fs::read_to_string(file_path)?;
155+
fn get_product_name(sysroot: &Dir) -> Result<String> {
156+
let release_path = "etc/system-release";
157+
if sysroot.exists(release_path) {
158+
let content = sysroot.read_to_string(release_path)?;
156159
let re = regex::Regex::new(r" *release.*").unwrap();
157160
return Ok(re.replace_all(&content, "").to_string());
158161
}
@@ -597,6 +600,8 @@ fn find_file_recursive<P: AsRef<Path>>(dir: P, target_file: &str) -> Result<Vec<
597600

598601
#[cfg(test)]
599602
mod tests {
603+
use cap_std_ext::dirext::CapStdExtDirExt;
604+
600605
use super::*;
601606

602607
#[test]
@@ -670,10 +675,38 @@ Boot0003* test";
670675
);
671676
Ok(())
672677
}
678+
#[cfg(test)]
679+
fn fixture() -> Result<cap_std_ext::cap_tempfile::TempDir> {
680+
let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?;
681+
tempdir.create_dir("etc")?;
682+
Ok(tempdir)
683+
}
673684
#[test]
674685
fn test_get_product_name() -> Result<()> {
675-
let name = get_product_name()?;
676-
assert!(name.len() > 0);
686+
let tmpd = fixture()?;
687+
{
688+
tmpd.atomic_write("etc/system-release", "Fedora release 40 (Forty)")?;
689+
let name = get_product_name(&tmpd)?;
690+
assert_eq!("Fedora", name);
691+
}
692+
{
693+
tmpd.atomic_write("etc/system-release", "CentOS Stream release 9")?;
694+
let name = get_product_name(&tmpd)?;
695+
assert_eq!("CentOS Stream", name);
696+
}
697+
{
698+
tmpd.atomic_write(
699+
"etc/system-release",
700+
"Red Hat Enterprise Linux CoreOS release 4",
701+
)?;
702+
let name = get_product_name(&tmpd)?;
703+
assert_eq!("Red Hat Enterprise Linux CoreOS", name);
704+
}
705+
{
706+
tmpd.remove_file("etc/system-release")?;
707+
let name = get_product_name(&tmpd)?;
708+
assert!(name.len() > 0);
709+
}
677710
Ok(())
678711
}
679712
}

0 commit comments

Comments
 (0)