Use the ESP we booted from via LoaderDevicePartUUID

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<TempMount> {
+pub fn mount_esp(device: &str) -> Result<TempMount> {
     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

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;
 

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 <https://github.com/containers/composefs-rs/issues/159>
 pub type ComposefsRepository = composefs::repository::Repository<Sha512HashValue>;
@@ -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<String> {
+    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)?,

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<String, EfiError> {
         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<u16> = 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: <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
+fn parse_efi_var(mut reader: impl Read) -> Result<String, EfiError> {
+    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<u16> = 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<u8> {
+        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";