From 581102abe0488ef20df830f271b4c882face108d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 10 Nov 2023 10:35:24 -0500 Subject: [PATCH] Improve EFI, add support for root=boot and writing UUID file In FCOS we never tried to support root=boot, but for bootupd to do alongside installs in the general case we have to. There were two things to fix here: - Tweak the EFI "trampoline" to check for both $prefix/grub.cfg and $prefix/boot/grub.cfg - Add support for writing the boot UUID into both places (This avoids higher level tools like bootupd needing to know about how to find the EFI vendor dir) Then the bigger issue here is that we need to support invoking `efibootmgr` to re-synchronize the firmware. This is particularly important in cases like bootc "alongside" installs where we're taking over the target OS. --- src/bios.rs | 1 + src/bootupd.rs | 42 ++++++++++---- src/cli/bootupd.rs | 21 ++++++- src/component.rs | 1 + src/efi.rs | 105 ++++++++++++++++++++++++++++++++-- src/filesystem.rs | 44 ++++++++++++++ src/grub2/grub-static-efi.cfg | 9 ++- src/grubconfigs.rs | 34 ++++++++++- src/main.rs | 1 + 9 files changed, 235 insertions(+), 23 deletions(-) create mode 100644 src/filesystem.rs diff --git a/src/bios.rs b/src/bios.rs index e2a25312..9f2546e9 100644 --- a/src/bios.rs +++ b/src/bios.rs @@ -93,6 +93,7 @@ impl Component for Bios { src_root: &openat::Dir, dest_root: &str, device: &str, + _update_firmware: bool, ) -> Result { let meta = if let Some(meta) = get_component_update(src_root, self)? { meta diff --git a/src/bootupd.rs b/src/bootupd.rs index 560f38bf..c323d68e 100644 --- a/src/bootupd.rs +++ b/src/bootupd.rs @@ -26,11 +26,28 @@ pub(crate) enum ClientRequest { Status, } +pub(crate) enum ConfigMode { + None, + Static, + WithUUID, +} + +impl ConfigMode { + pub(crate) fn enabled_with_uuid(&self) -> Option { + match self { + ConfigMode::None => None, + ConfigMode::Static => Some(false), + ConfigMode::WithUUID => Some(true), + } + } +} + pub(crate) fn install( source_root: &str, dest_root: &str, device: Option<&str>, - with_static_configs: bool, + configs: ConfigMode, + update_firmware: bool, target_components: Option<&[String]>, auto_components: bool, ) -> Result<()> { @@ -78,7 +95,7 @@ pub(crate) fn install( } let meta = component - .install(&source_root, dest_root, device) + .install(&source_root, dest_root, device, update_firmware) .with_context(|| format!("installing component {}", component.name()))?; log::info!("Installed {} {}", component.name(), meta.meta.version); state.installed.insert(component.name().into(), meta); @@ -89,14 +106,17 @@ pub(crate) fn install( } let sysroot = &openat::Dir::open(dest_root)?; - if with_static_configs { - #[cfg(any( - target_arch = "x86_64", - target_arch = "aarch64", - target_arch = "powerpc64" - ))] - crate::grubconfigs::install(sysroot, installed_efi)?; - // On other architectures, assume that there's nothing to do. + match configs.enabled_with_uuid() { + Some(uuid) => { + #[cfg(any( + target_arch = "x86_64", + target_arch = "aarch64", + target_arch = "powerpc64" + ))] + crate::grubconfigs::install(sysroot, installed_efi, uuid)?; + // On other architectures, assume that there's nothing to do. + } + None => {} } // Unmount the ESP, etc. @@ -126,7 +146,7 @@ pub(crate) fn get_components_impl(auto: bool) -> Components { #[cfg(target_arch = "x86_64")] { if auto { - let is_efi_booted = Path::new("/sys/firmware/efi").exists(); + let is_efi_booted = crate::efi::is_efi_booted().unwrap(); log::info!( "System boot method: {}", if is_efi_booted { "EFI" } else { "BIOS" } diff --git a/src/cli/bootupd.rs b/src/cli/bootupd.rs index b2cfd598..d2233761 100644 --- a/src/cli/bootupd.rs +++ b/src/cli/bootupd.rs @@ -1,4 +1,4 @@ -use crate::bootupd; +use crate::bootupd::{self, ConfigMode}; use anyhow::{Context, Result}; use clap::Parser; use log::LevelFilter; @@ -56,6 +56,15 @@ pub struct InstallOpts { #[clap(long)] with_static_configs: bool, + /// Implies `--with-static-configs`. When present, this also writes a + /// file with the UUID of the target filesystems. + #[clap(long)] + write_uuid: bool, + + /// On EFI systems, invoke `efibootmgr` to update the firmware. + #[clap(long)] + update_firmware: bool, + #[clap(long = "component", conflicts_with = "auto")] /// Only install these components components: Option>, @@ -97,11 +106,19 @@ impl DCommand { /// Runner for `install` verb. pub(crate) fn run_install(opts: InstallOpts) -> Result<()> { + let configmode = if opts.write_uuid { + ConfigMode::WithUUID + } else if opts.with_static_configs { + ConfigMode::Static + } else { + ConfigMode::None + }; bootupd::install( &opts.src_root, &opts.dest_root, opts.device.as_deref(), - opts.with_static_configs, + configmode, + opts.update_firmware, opts.components.as_deref(), opts.auto, ) diff --git a/src/component.rs b/src/component.rs index 7ac0c7f0..dc1c6068 100644 --- a/src/component.rs +++ b/src/component.rs @@ -50,6 +50,7 @@ pub(crate) trait Component { src_root: &openat::Dir, dest_root: &str, device: &str, + update_firmware: bool, ) -> Result; /// Implementation of `bootupd generate-update-metadata` for a given component. diff --git a/src/efi.rs b/src/efi.rs index 331b607b..4d18bdce 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -10,6 +10,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use anyhow::{bail, Context, Result}; +use fn_error_context::context; use openat_ext::OpenatDirExt; use widestring::U16CString; @@ -22,6 +23,10 @@ use crate::{component::*, packagesystem}; /// Well-known paths to the ESP that may have been mounted external to us. pub(crate) const ESP_MOUNTS: &[&str] = &["boot/efi", "efi", "boot"]; +/// The binary to change EFI boot ordering +const EFIBOOTMGR: &str = "efibootmgr"; +const SHIM: &str = "shimx64.efi"; + /// The ESP partition label on Fedora CoreOS derivatives pub(crate) const COREOS_ESP_PART_LABEL: &str = "EFI-SYSTEM"; pub(crate) const ANACONDA_ESP_PART_LABEL: &str = "EFI\\x20System\\x20Partition"; @@ -30,6 +35,13 @@ pub(crate) const ANACONDA_ESP_PART_LABEL: &str = "EFI\\x20System\\x20Partition"; const LOADER_INFO_VAR_STR: &str = "LoaderInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f"; const STUB_INFO_VAR_STR: &str = "StubInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f"; +/// Return `true` if the system is booted via EFI +pub(crate) fn is_efi_booted() -> Result { + Path::new("/sys/firmware/efi") + .try_exists() + .map_err(Into::into) +} + #[derive(Default)] pub(crate) struct Efi { mountpoint: RefCell>, @@ -113,6 +125,17 @@ impl Efi { } Ok(()) } + + #[context("Updating EFI firmware variables")] + fn update_firmware(&self, device: &str, espdir: &openat::Dir) -> Result<()> { + let efidir = &espdir.sub_dir("EFI").context("Opening EFI")?; + let vendordir = super::grubconfigs::find_efi_vendordir(efidir)?; + let vendordir = vendordir + .to_str() + .ok_or_else(|| anyhow::anyhow!("Invalid non-UTF-8 vendordir"))?; + clear_efi_current()?; + set_efi_current(device, espdir, vendordir) + } } /// Convert a nul-terminated UTF-16 byte array to a String. @@ -259,7 +282,8 @@ impl Component for Efi { &self, src_root: &openat::Dir, dest_root: &str, - _: &str, + device: &str, + update_firmware: bool, ) -> Result { let meta = if let Some(meta) = get_component_update(src_root, self)? { meta @@ -270,11 +294,11 @@ impl Component for Efi { let srcdir_name = component_updatedirname(self); let ft = crate::filetree::FileTree::new_from_dir(&src_root.sub_dir(&srcdir_name)?)?; let destdir = &self.ensure_mounted_esp(Path::new(dest_root))?; - { - let destd = openat::Dir::open(destdir) - .with_context(|| format!("opening dest dir {}", destdir.display()))?; - validate_esp(&destd)?; - } + + let destd = &openat::Dir::open(destdir) + .with_context(|| format!("opening dest dir {}", destdir.display()))?; + validate_esp(destd)?; + // TODO - add some sort of API that allows directly setting the working // directory to a file descriptor. let r = std::process::Command::new("cp") @@ -286,6 +310,9 @@ impl Component for Efi { if !r.success() { anyhow::bail!("Failed to copy"); } + if update_firmware { + self.update_firmware(device, destd)? + } Ok(InstalledContent { meta, filetree: Some(ft), @@ -399,3 +426,69 @@ fn validate_esp(dir: &openat::Dir) -> Result<()> { }; Ok(()) } + +#[context("Clearing current EFI boot entry")] +pub(crate) fn clear_efi_current() -> Result<()> { + const BOOTCURRENT: &str = "BootCurrent"; + if !crate::efi::is_efi_booted()? { + log::debug!("System is not booted via EFI"); + return Ok(()); + } + let output = Command::new(EFIBOOTMGR).output()?; + if !output.status.success() { + anyhow::bail!("Failed to invoke {EFIBOOTMGR}") + } + let output = String::from_utf8(output.stdout)?; + let current = output + .lines() + .filter_map(|l| l.split_once(':')) + .filter_map(|(k, v)| (k == BOOTCURRENT).then_some(v.trim())) + .next() + .ok_or_else(|| anyhow::anyhow!("Failed to find BootCurrent"))?; + let output = Command::new(EFIBOOTMGR) + .args(["-b", current, "-B"]) + .output()?; + if !output.status.success() { + std::io::copy( + &mut std::io::Cursor::new(output.stderr), + &mut std::io::stderr().lock(), + )?; + anyhow::bail!("Failed to invoke {EFIBOOTMGR}"); + } + anyhow::Ok(()) +} + +#[context("Adding new EFI boot entry")] +pub(crate) fn set_efi_current(device: &str, espdir: &openat::Dir, vendordir: &str) -> Result<()> { + let fsinfo = crate::filesystem::inspect_filesystem(espdir, ".")?; + let source = fsinfo.source; + let devname = source + .rsplit_once('/') + .ok_or_else(|| anyhow::anyhow!("Failed to parse {source}"))? + .1; + let partition_path = format!("/sys/class/block/{devname}/partition"); + let partition_number = std::fs::read_to_string(&partition_path) + .with_context(|| format!("Failed to read {partition_path}"))?; + let shim = format!("{vendordir}/{SHIM}"); + if espdir.exists(&shim)? { + anyhow::bail!("Failed to find {SHIM}"); + } + let loader = format!("\\EFI\\{}\\shimx64.efi", vendordir); + let st = Command::new(EFIBOOTMGR) + .args([ + "--create", + "--disk", + device, + "--part", + partition_number.as_str(), + "--loader", + loader.as_str(), + "--label", + vendordir, + ]) + .status()?; + if !st.success() { + anyhow::bail!("Failed to invoke {EFIBOOTMGR}") + } + anyhow::Ok(()) +} diff --git a/src/filesystem.rs b/src/filesystem.rs new file mode 100644 index 00000000..f09ceb34 --- /dev/null +++ b/src/filesystem.rs @@ -0,0 +1,44 @@ +use std::os::fd::AsRawFd; +use std::os::unix::process::CommandExt; +use std::process::Command; + +use anyhow::{Context, Result}; +use fn_error_context::context; +use serde::Deserialize; + +#[derive(Deserialize, Debug)] +#[serde(rename_all = "kebab-case")] +#[allow(dead_code)] +pub(crate) struct Filesystem { + pub(crate) source: String, + pub(crate) fstype: String, + pub(crate) options: String, + pub(crate) uuid: Option, +} + +#[derive(Deserialize, Debug)] +pub(crate) struct Findmnt { + pub(crate) filesystems: Vec, +} + +#[context("Inspecting filesystem {path:?}")] +pub(crate) fn inspect_filesystem(root: &openat::Dir, path: &str) -> Result { + let rootfd = root.as_raw_fd(); + // SAFETY: This is unsafe just for the pre_exec, when we port to cap-std we can use cap-std-ext + let o = unsafe { + Command::new("findmnt") + .args(["-J", "-v", "--output-all", path]) + .pre_exec(move || nix::unistd::fchdir(rootfd).map_err(Into::into)) + .output()? + }; + let st = o.status; + if !st.success() { + anyhow::bail!("findmnt failed: {st:?}"); + } + let o: Findmnt = serde_json::from_reader(std::io::Cursor::new(&o.stdout)) + .context("Parsing findmnt output")?; + o.filesystems + .into_iter() + .next() + .ok_or_else(|| anyhow::anyhow!("findmnt returned no data")) +} diff --git a/src/grub2/grub-static-efi.cfg b/src/grub2/grub-static-efi.cfg index bf38ca90..3d552c30 100644 --- a/src/grub2/grub-static-efi.cfg +++ b/src/grub2/grub-static-efi.cfg @@ -13,7 +13,12 @@ else search --label boot --set prefix --no-floppy fi fi -set prefix=($prefix)/grub2 -configfile $prefix/grub.cfg +if [ -d ($prefix)/grub2 ]; then + set prefix=($prefix)/grub2 + configfile $prefix/grub.cfg +else + set prefix=($prefix)/boot/grub2 + configfile $prefix/grub.cfg +fi boot diff --git a/src/grubconfigs.rs b/src/grubconfigs.rs index dcdf986c..8b4b0c90 100644 --- a/src/grubconfigs.rs +++ b/src/grubconfigs.rs @@ -29,8 +29,14 @@ pub(crate) fn find_efi_vendordir(efidir: &openat::Dir) -> Result { /// Install the static GRUB config files. #[context("Installing static GRUB configs")] -pub(crate) fn install(target_root: &openat::Dir, efi: bool) -> Result<()> { +pub(crate) fn install(target_root: &openat::Dir, efi: bool, write_uuid: bool) -> Result<()> { let bootdir = &target_root.sub_dir("boot").context("Opening /boot")?; + let boot_is_mount = { + let root_dev = target_root.self_metadata()?.stat().st_dev; + let boot_dev = bootdir.self_metadata()?.stat().st_dev; + log::debug!("root_dev={root_dev} boot_dev={boot_dev}"); + root_dev != boot_dev + }; let mut config = std::fs::read_to_string(Path::new(CONFIGDIR).join("grub-static-pre.cfg"))?; @@ -71,6 +77,22 @@ pub(crate) fn install(target_root: &openat::Dir, efi: bool) -> Result<()> { .context("Copying grub-static.cfg")?; println!("Installed: grub.cfg"); + let uuid_path = if write_uuid { + let target_fs = if boot_is_mount { bootdir } else { target_root }; + let bootfs_meta = crate::filesystem::inspect_filesystem(target_fs, ".")?; + let bootfs_uuid = bootfs_meta + .uuid + .ok_or_else(|| anyhow::anyhow!("Failed to find UUID for boot"))?; + let grub2_uuid_contents = format!("set BOOT_UUID=\"{bootfs_uuid}\"\n"); + let uuid_path = format!("{GRUB2DIR}/bootuuid.cfg"); + bootdir + .write_file_contents(&uuid_path, 0o644, grub2_uuid_contents) + .context("Writing bootuuid.cfg")?; + Some(uuid_path) + } else { + None + }; + let efidir = efi .then(|| { target_root @@ -87,6 +109,14 @@ pub(crate) fn install(target_root: &openat::Dir, efi: bool) -> Result<()> { .copy_file(&Path::new(CONFIGDIR).join("grub-static-efi.cfg"), target) .context("Copying static EFI")?; println!("Installed: {target:?}"); + if let Some(uuid_path) = uuid_path { + // SAFETY: we always have a filename + let filename = Path::new(&uuid_path).file_name().unwrap(); + let target = &vendordir.join(filename); + bootdir + .copy_file_at(uuid_path, efidir, target) + .context("Writing bootuuid.cfg to efi dir")?; + } } Ok(()) @@ -106,7 +136,7 @@ mod tests { std::fs::create_dir_all(tdp.join("boot/grub2"))?; std::fs::create_dir_all(tdp.join("boot/efi/EFI/BOOT"))?; std::fs::create_dir_all(tdp.join("boot/efi/EFI/fedora"))?; - install(&td, true).unwrap(); + install(&td, true, false).unwrap(); assert!(td.exists("boot/grub2/grub.cfg")?); assert!(td.exists("boot/efi/EFI/fedora/grub.cfg")?); diff --git a/src/main.rs b/src/main.rs index 61b73ff8..133e9bbf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,6 +23,7 @@ mod coreos; mod daemon; #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] mod efi; +mod filesystem; mod filetree; #[cfg(any( target_arch = "x86_64",