From 23f0e9c0b8cc8b7be7d98c53427b4653777c6605 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 8 Oct 2026 10:58:21 +0530 Subject: [PATCH 1/4] utils: Strip efivarfs attribute header and NUL when reading EFI vars read_uefi_var() decoded the entire contents of the efivarfs file as UTF-16LE. efivarfs prepends a 4-byte little-endian u32 of the variable's attributes, and EFI string variables are NUL-terminated. As a result the returned string was wrapped in invisible characters and incorrect string matches with UTF-8 encoded Strings Skip the 4-byte attribute header before decoding and trim trailing NUL and surrounding whitespace from the result. Signed-off-by: Pragyan Poudyal --- crates/lib/src/utils.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/crates/lib/src/utils.rs b/crates/lib/src/utils.rs index fd0d613b7..58f4147e4 100644 --- a/crates/lib/src/utils.rs +++ b/crates/lib/src/utils.rs @@ -251,22 +251,36 @@ pub fn read_uefi_var(var_name: &str) -> Result { match efivarfs.read(var_name) { Ok(loader_bytes) => { - if loader_bytes.len() % 2 != 0 { + // Ref: https://www.kernel.org/doc/html/latest/filesystems/efivarfs.html + // + // 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 + let data = &loader_bytes[4..]; + + if data.len() % 2 != 0 { return Err(EfiError::InvalidData( "EFI var length is not valid UTF-16 LE", )); } // EFI vars are UTF-16 LE - let loader_u16_bytes: Vec = loader_bytes + let data_u16_bytes: Vec = data .chunks_exact(2) .map(|x| u16::from_le_bytes([x[0], x[1]])) .collect(); - let loader = String::from_utf16(&loader_u16_bytes) + let var_string = String::from_utf16(&data_u16_bytes) .map_err(|_| EfiError::InvalidData("EFI var is not UTF-16"))?; - return Ok(loader); + // EFI string variables are NUL-terminated; strip the trailing + // NUL(s) and any surrounding whitespace so the value compares + // cleanly against + return Ok(var_string.trim_matches(|c| c == '\0').trim().to_string()); } Err(e) if e.kind() == std::io::ErrorKind::NotFound => { From 73b08ad4c27b2fbe157099a8a9f900fde38431ce Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 8 Oct 2026 11:01:24 +0530 Subject: [PATCH 2/4] store: Select the booted ESP via LoaderDevicePartUUID On dual-booted systems, walking up to the root disk(s) and picking the first colocated ESP can return the wrong one, since there may be multiple ESPs. Instead, read the LoaderDevicePartUUID EFI variable set by the boot loader and match it against the partuuid of the candidate partitions, so we operate on the ESP we actually booted from. The install paths still use find_first_colocated_esp(), since at that point we are not booted into a bootc system and the EFI variable does not describe the target system. Fixes: #2554 Signed-off-by: Pragyan Poudyal --- crates/lib/src/bootc_composefs/boot.rs | 23 ++++++------ crates/lib/src/store/mod.rs | 48 ++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index f8e12b7d2..a55093492 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -104,6 +104,7 @@ use crate::bootc_kargs::compute_new_kargs; use crate::composefs_consts::{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, @@ -812,6 +813,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 +851,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 +1785,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 +1801,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/store/mod.rs b/crates/lib/src/store/mod.rs index dbda82819..f3cbb62db 100644 --- a/crates/lib/src/store/mod.rs +++ b/crates/lib/src/store/mod.rs @@ -123,7 +123,7 @@ 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::{deployment_fd, open_dir_remount_rw, read_uefi_var}; /// See pub type ComposefsRepository = composefs::repository::Repository; @@ -490,6 +490,38 @@ 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 device_uuid = read_uefi_var(LOADER_DEVICE_PART_UUID) + .map_err(|e| anyhow::anyhow!("Reading {LOADER_DEVICE_PART_UUID}: {e:?}"))?; + + let root_dev = bootc_blockdev::list_dev_by_dir(&physical_root)?; + let all_roots = root_dev.find_all_roots()?; + + 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 +538,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)?, From 18dd8c6cc874d3b68ecf5ef546a6cbd94426233a Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Thu, 8 Oct 2026 12:56:47 +0530 Subject: [PATCH 3/4] esp: Handle non-BLS bootloaders Older versions of Grub, the one in c10s and c9s, don't fully implement the BLS spec, which means we don't get the efivar LoaderDevicePartUUID-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f. In such a case, go through all the ESPs and try to find bootc owned BLS config file in `ESP/loader/entries` Signed-off-by: Pragyan Poudyal --- crates/lib/src/bootc_composefs/boot.rs | 8 ++-- crates/lib/src/composefs_consts.rs | 3 ++ crates/lib/src/store/mod.rs | 63 ++++++++++++++++++++++++-- 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/crates/lib/src/bootc_composefs/boot.rs b/crates/lib/src/bootc_composefs/boot.rs index a55093492..265fd58c4 100644 --- a/crates/lib/src/bootc_composefs/boot.rs +++ b/crates/lib/src/bootc_composefs/boot.rs @@ -101,7 +101,9 @@ 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; @@ -422,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)) } @@ -519,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. 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 f3cbb62db..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, read_uefi_var}; +use crate::utils::{EfiError, deployment_fd, open_dir_remount_rw, read_uefi_var}; /// See pub type ComposefsRepository = composefs::repository::Repository; @@ -499,12 +500,64 @@ const LOADER_DEVICE_PART_UUID: &str = "LoaderDevicePartUUID-4a67b082-0a4c-41cf-b /// 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 device_uuid = read_uefi_var(LOADER_DEVICE_PART_UUID) - .map_err(|e| anyhow::anyhow!("Reading {LOADER_DEVICE_PART_UUID}: {e:?}"))?; - 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; From a57ab346a5ada0599d91d15fee25c99e01a444bc Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 9 Oct 2026 12:08:30 +0530 Subject: [PATCH 4/4] test: Add tests for reading EFI variables AssistedBy: LLM Signed-off-by: Pragyan Poudyal --- crates/lib/src/utils.rs | 125 ++++++++++++++++++++++++++++------------ 1 file changed, 89 insertions(+), 36 deletions(-) diff --git a/crates/lib/src/utils.rs b/crates/lib/src/utils.rs index 58f4147e4..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,46 +249,51 @@ pub fn read_uefi_var(var_name: &str) -> Result { Err(e) => Err(e)?, }; - match efivarfs.read(var_name) { - Ok(loader_bytes) => { - // Ref: https://www.kernel.org/doc/html/latest/filesystems/efivarfs.html - // - // 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 - let data = &loader_bytes[4..]; - - if data.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 data_u16_bytes: Vec = data - .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 var_string = String::from_utf16(&data_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")); + }; - // EFI string variables are NUL-terminated; strip the trailing - // NUL(s) and any surrounding whitespace so the value compares - // cleanly against - return Ok(var_string.trim_matches(|c| c == '\0').trim().to_string()); - } + 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`. @@ -334,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";