diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index f8e12b7d2..265fd58c4 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -101,9 +101,12 @@ use serde::{Deserialize, Serialize}; use crate::bootc_composefs::state::{get_booted_bls, write_composefs_state}; use crate::bootc_composefs::status::build_composefs_karg; use crate::bootc_kargs::compute_new_kargs; -use crate::composefs_consts::{TYPE1_BOOT_DIR_PREFIX, TYPE1_ENT_PATH, TYPE1_ENT_PATH_STAGED}; +use crate::composefs_consts::{ + BLS_ENTRY_FILE_PREFIX, TYPE1_BOOT_DIR_PREFIX, TYPE1_ENT_PATH, TYPE1_ENT_PATH_STAGED, +}; use crate::parsers::bls_config::{BLSConfig, BLSConfigType, EFIKey}; use crate::spec::BootloaderKind; +use crate::store::find_booted_from_esp; use crate::task::Task; use crate::{ bootc_composefs::repo::open_composefs_repo, @@ -421,7 +424,7 @@ const ESP_MOUNT_DATA: &std::ffi::CStr = c"fmask=0177,dmask=0077"; /// is already mounted in the current mount namespace; callers should use /// [`mount_esp_readonly`] or [`mount_esp_writable`] instead of this primitive /// so that pre-existing mounts are handled. -fn mount_esp(device: &str) -> Result { +pub fn mount_esp(device: &str) -> Result { TempMount::mount_dev(device, "vfat", ESP_MOUNT_FLAGS, Some(ESP_MOUNT_DATA)) } @@ -518,7 +521,7 @@ pub fn type1_entry_conf_file_name( priority: &str, ) -> String { let os_id_safe = os_id.replace('-', "_"); - format!("bootc_{os_id_safe}-{version}-{priority}.conf") + format!("{BLS_ENTRY_FILE_PREFIX}{os_id_safe}-{version}-{priority}.conf") } /// Generate sort key for the primary (new/upgraded) boot entry. @@ -812,6 +815,9 @@ pub(crate) fn setup_composefs_bls_boot( } // Locate ESP partition device by walking up to the root disk(s) + // + // NOTE: Not using [`find_booted_from_esp`] as we aren't booted + // into a bootc system let esp_part = root_setup.device_info.find_first_colocated_esp()?; ( @@ -847,11 +853,11 @@ pub(crate) fn setup_composefs_bls_boot( ), )?; - // Locate ESP partition device by walking up to the root disk(s) - let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?; - let esp_dev = root_dev.find_first_colocated_esp()?; - - (esp_dev.path(), cmdline, bootloader) + ( + find_booted_from_esp(&storage.physical_root)?, + cmdline, + bootloader, + ) } }; @@ -1781,6 +1787,9 @@ pub(crate) fn setup_composefs_uki_boot( state.require_no_kargs_for_uki()?; // Locate ESP partition device by walking up to the root disk(s) + // + // NOTE: Not using [`find_booted_from_esp`] as we aren't booted + // into a bootc system let esp_part = root_setup.device_info.find_first_colocated_esp()?; ( @@ -1794,12 +1803,8 @@ pub(crate) fn setup_composefs_uki_boot( BootSetupType::Upgrade((storage, booted_cfs, host)) => { let bootloader = host.require_composefs_booted()?.bootloader.clone(); - // Locate ESP partition device by walking up to the root disk(s) - let root_dev = bootc_blockdev::list_dev_by_dir(&storage.physical_root)?; - let esp_dev = root_dev.find_first_colocated_esp()?; - ( - esp_dev.path(), + find_booted_from_esp(&storage.physical_root)?, bootloader, booted_cfs.cmdline.allow_missing_fsverity, // TODO: We never (re)install UKI addons on upgrade, only on initial diff --git a/crates/lib/src/composefs_consts.rs b/crates/lib/src/composefs_consts.rs index 03ccc5310..c9022acf5 100644 --- a/crates/lib/src/composefs_consts.rs +++ b/crates/lib/src/composefs_consts.rs @@ -42,6 +42,9 @@ pub(crate) const BOOTC_FINALIZE_STAGED_SERVICE: &str = "bootc-finalize-staged.se /// The prefix for the directories containing kernel + initrd pub(crate) const TYPE1_BOOT_DIR_PREFIX: &str = "bootc_composefs-"; +/// The prefix for BLS entry config files +pub(crate) const BLS_ENTRY_FILE_PREFIX: &str = "bootc_"; + /// The prefix for names of UKI and UKI Addons pub(crate) const UKI_NAME_PREFIX: &str = TYPE1_BOOT_DIR_PREFIX; diff --git a/crates/lib/src/store/mod.rs b/crates/lib/src/store/mod.rs index dbda82819..e517bc963 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -117,13 +117,14 @@ use composefs::repository::{RepositoryConfig, RepositoryOpenError}; use composefs_ctl::composefs; use crate::bootc_composefs::backwards_compat::bcompat_boot::prepend_custom_prefix; -use crate::bootc_composefs::boot::{EFI_LINUX, mount_esp_readonly, mount_esp_writable}; +use crate::bootc_composefs::boot::{EFI_LINUX, mount_esp, mount_esp_readonly, mount_esp_writable}; use crate::bootc_composefs::status::{ComposefsCmdline, composefs_booted, get_bootloader}; +use crate::composefs_consts::{BLS_ENTRY_FILE_PREFIX, TYPE1_ENT_PATH}; use crate::install::BOOT; use crate::lsm; use crate::podstorage::CStorage; use crate::spec::{BootloaderKind, ImageStatus}; -use crate::utils::{deployment_fd, open_dir_remount_rw}; +use crate::utils::{EfiError, deployment_fd, open_dir_remount_rw, read_uefi_var}; /// See pub type ComposefsRepository = composefs::repository::Repository; @@ -490,6 +491,90 @@ fn get_boot_dir_for_grub(physical_root: &Dir) -> Result<(Dir, Utf8PathBuf)> { )) } +const LOADER_DEVICE_PART_UUID: &str = "LoaderDevicePartUUID-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f"; + +/// Find the ESP from which the system was booted +/// +/// This is a better way to find the correct ESP than just searching +/// trough all ESPs and returning the first one as we may end up with +/// the wrong ESP in dual booted systems +#[context("Finding ESP used for booting")] +pub(crate) fn find_booted_from_esp(physical_root: &Dir) -> Result { + let root_dev = bootc_blockdev::list_dev_by_dir(&physical_root)?; + let all_roots = root_dev.find_all_roots()?; + + let device_uuid = match read_uefi_var(LOADER_DEVICE_PART_UUID) { + Ok(u) => u, + // Older version of Grub (in c10s) do not include this efivar + // We can't rely on ESP being mounted on /boot, /boot/efi as older Grub + // doesn't fully support BLS spec + Err(EfiError::MissingVar) => { + tracing::debug!("Missing {LOADER_DEVICE_PART_UUID}, searching through all ESPs..."); + + let all_esps = root_dev + .find_colocated_esps()? + .ok_or_else(|| anyhow::anyhow!("No ESP found"))?; + + if all_esps.len() == 1 { + // If only one ESP, there's no ambiguity + return Ok(all_esps.first().unwrap().path()); + } + + for esp in all_esps { + let tmp_mount = mount_esp(&esp.path()).context("Mounting ESP")?; + + // Check if we have bootc owned entries in loader/entries + let entries = tmp_mount + .fd + .open_dir_optional(TYPE1_ENT_PATH) + .context("Opening entries dir")?; + + let Some(entries_dir) = entries else { + tracing::debug!("No entries found in ESP {}", esp.path()); + continue; + }; + + for entry in entries_dir + .entries_utf8() + .context("Reading directory entries")? + { + let entry = entry?; + + // We can be fairly sure we put our entries in this ESP + if entry.file_name()?.starts_with(BLS_ENTRY_FILE_PREFIX) { + return Ok(esp.path()); + } + } + + tracing::debug!("ESP {} not owned by us", esp.path()); + } + + anyhow::bail!("Failed to find bootc owned ESP"); + } + Err(e) => { + anyhow::bail!("Reading {LOADER_DEVICE_PART_UUID}: {e:?}") + } + }; + + tracing::debug!("Found {device_uuid} in {LOADER_DEVICE_PART_UUID}"); + + for root in all_roots { + let Some(children) = root.children else { + continue; + }; + + for child in children { + if child.partuuid.as_ref().is_some_and(|partuuid| { + *partuuid.to_ascii_lowercase() == device_uuid.to_ascii_lowercase() + }) { + return Ok(child.path()); + } + } + } + + anyhow::bail!("ESP with uuid {device_uuid} not found") +} + impl BootedStorage { /// Create a new booted storage accessor for the given environment. /// @@ -506,13 +591,15 @@ impl BootedStorage { } let composefs = Arc::new(composefs); - // Locate ESP by walking up to the root disk(s). Both mount - // variants transparently reuse an already-mounted ESP when - // present (e.g. auto-mounted at /boot ro via + // Locate ESP by walking up to the root disk(s), and reading + // [`LOADER_DEVICE_PART_UUID`] and matching the UUID with the + // respective ESP device UUID + // + // Both mount variants transparently reuse an already-mounted + // ESP when present (e.g. auto-mounted at /boot ro via // `systemd.mount-extra` in the deployment cmdline). - let root_dev = bootc_blockdev::list_dev_by_dir(&physical_root)?; - let esp_dev = root_dev.find_first_colocated_esp()?; - let esp_path = esp_dev.path(); + let esp_path = find_booted_from_esp(&physical_root)?; + let esp_mount = match esp_access { EspAccess::ReadOnly => mount_esp_readonly(&esp_path)?, EspAccess::ReadWrite => mount_esp_writable(&esp_path)?, diff --git a/crates/lib/src/utils.rs b/crates/lib/src/utils.rs index fd0d613b7..7941606a1 100644 --- a/crates/lib/src/utils.rs +++ b/crates/lib/src/utils.rs @@ -1,5 +1,5 @@ use std::future::Future; -use std::io::Write; +use std::io::{Read, Write}; use std::os::fd::BorrowedFd; use std::path::{Component, Path, PathBuf}; use std::process::Command; @@ -249,32 +249,51 @@ pub fn read_uefi_var(var_name: &str) -> Result { Err(e) => Err(e)?, }; - match efivarfs.read(var_name) { - Ok(loader_bytes) => { - if loader_bytes.len() % 2 != 0 { - return Err(EfiError::InvalidData( - "EFI var length is not valid UTF-16 LE", - )); - } + match efivarfs.open(var_name) { + Ok(f) => parse_efi_var(f), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Err(EfiError::MissingVar), + Err(e) => Err(e)?, + } +} - // EFI vars are UTF-16 LE - let loader_u16_bytes: Vec = loader_bytes - .chunks_exact(2) - .map(|x| u16::from_le_bytes([x[0], x[1]])) - .collect(); +/// Parse the raw contents of an efivarfs EFI variable into its string value. +/// +/// Ref: +/// +/// When a content of an UEFI variable in /sys/firmware/efi/efivars is displayed, +/// for example using “hexdump”, pay attention that the first 4 bytes of the output +/// represent the UEFI variable attributes, in little-endian format. +/// +/// Practically the output of each efivar is composed of: +/// +/// 4_bytes_of_attributes + efivar_data +fn parse_efi_var(mut reader: impl Read) -> Result { + let mut bytes = Vec::new(); + reader.read_to_end(&mut bytes)?; - let loader = String::from_utf16(&loader_u16_bytes) - .map_err(|_| EfiError::InvalidData("EFI var is not UTF-16"))?; + let Some((_, data)) = bytes.split_at_checked(4) else { + return Err(EfiError::InvalidData("EFI variable is too short")); + }; - return Ok(loader); - } + if data.len() % 2 != 0 { + return Err(EfiError::InvalidData( + "EFI var length is not valid UTF-16 LE", + )); + } - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - return Err(EfiError::MissingVar); - } + // EFI vars are UTF-16 LE + let data_u16_bytes: Vec = data + .chunks_exact(2) + .map(|x| u16::from_le_bytes([x[0], x[1]])) + .collect(); - Err(e) => Err(e)?, - } + let var_string = String::from_utf16(&data_u16_bytes) + .map_err(|_| EfiError::InvalidData("EFI var is not UTF-16"))?; + + // EFI string variables are NUL-terminated; strip the trailing + // NUL(s) and any surrounding whitespace so the value compares + // cleanly against UTF-8 strings + Ok(var_string.trim_matches(|c| c == '\0').trim().to_string()) } /// Computes a relative path from `from` to `to`. @@ -320,6 +339,54 @@ mod tests { ); } + /// Build raw efivarfs contents: 4 attribute bytes followed by the + /// UTF-16 LE encoding of `s`. + fn efi_var_bytes(s: &str) -> Vec { + let mut bytes = vec![0x07, 0x00, 0x00, 0x00]; + for unit in s.encode_utf16() { + bytes.extend_from_slice(&unit.to_le_bytes()); + } + bytes + } + + #[test] + fn test_parse_efi_var() { + // A plain value, NUL-terminated as the firmware writes it. + let bytes = efi_var_bytes("abcd-1234\0"); + assert_eq!(parse_efi_var(&bytes[..]).unwrap(), "abcd-1234"); + + // Surrounding whitespace and trailing NULs are stripped. + let bytes = efi_var_bytes(" value \0\0"); + assert_eq!(parse_efi_var(&bytes[..]).unwrap(), "value"); + + // Just the attributes with no data is empty, not an error. + let bytes = efi_var_bytes(""); + assert_eq!(parse_efi_var(&bytes[..]).unwrap(), ""); + } + + #[test] + fn test_parse_efi_var_errors() { + // Fewer than the 4 attribute bytes. + assert!(matches!( + parse_efi_var(&[0x07, 0x00][..]), + Err(EfiError::InvalidData(_)) + )); + + // Odd number of data bytes cannot be UTF-16 LE. + let bytes = [0x07, 0x00, 0x00, 0x00, 0x41]; + assert!(matches!( + parse_efi_var(&bytes[..]), + Err(EfiError::InvalidData(_)) + )); + + // Unpaired UTF-16 surrogate is not valid UTF-16. + let bytes = [0x07, 0x00, 0x00, 0x00, 0x00, 0xD8]; + assert!(matches!( + parse_efi_var(&bytes[..]), + Err(EfiError::InvalidData(_)) + )); + } + #[test] fn test_find_mount_option() { const V1: &str = "rw,relatime,compress=foo,subvol=blah,fast";