From 798f9a5c1b9c6fa00384d915918a5ec339f40d03 Mon Sep 17 00:00:00 2001 From: bfjelds Date: Thu, 4 Jun 2026 11:15:14 -0700 Subject: [PATCH 01/10] feat: kernel-dependent BTRFS UUID collision resolution On kernel >=6.7, use mount -o temp_fsuid to mount the staging device directly, bypassing the BTRFS global UUID registry. This is the preferred solution as it mounts real staging content without needing verity hash verification. On kernel <6.7 (e.g. 6.6.x), fall back to the existing bind-mount strategy which requires verity hash matching to prove the active and staging content are identical. Changes: - Add KernelVersion parser to osutils/uname.rs with unit tests - Split detect_acl_btrfs_uuid_collision into collision detection and resolution strategy (AclBtrfsCollisionResolution enum) - Add verify_acl_bind_mount_safety for the bind-mount path - Mount handler selects strategy based on kernel version Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/osutils/src/uname.rs | 78 ++++++++- crates/trident/src/engine/newroot.rs | 232 ++++++++++++++++++--------- 2 files changed, 230 insertions(+), 80 deletions(-) diff --git a/crates/osutils/src/uname.rs b/crates/osutils/src/uname.rs index c73576eda8..9fc6ac0da2 100644 --- a/crates/osutils/src/uname.rs +++ b/crates/osutils/src/uname.rs @@ -11,11 +11,85 @@ pub fn kernel_release() -> Result { .context("Failed to run uname -r") } +/// Parsed kernel version with major and minor components. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub struct KernelVersion { + pub major: u32, + pub minor: u32, +} + +impl KernelVersion { + /// Parse a kernel version from a `uname -r` string. + /// + /// Extracts the leading `major.minor` from strings like: + /// - `6.6.78.2-1.cm2` + /// - `6.7.0-1.cm2` + /// - `7.0.0` + /// + /// Returns `None` if the string cannot be parsed. + pub fn parse(release: &str) -> Option { + // Strip everything after the first '-' (e.g. "-1.cm2"), then split on '.'. + let numeric_part = release.split('-').next()?; + let mut parts = numeric_part.split('.'); + let major = parts.next()?.parse::().ok()?; + let minor = parts.next()?.parse::().ok()?; + Some(KernelVersion { major, minor }) + } + + /// Returns the kernel version of the running system. + pub fn running() -> Result, Error> { + let release = kernel_release()?; + Ok(Self::parse(&release)) + } + + /// Returns true if this kernel version supports the BTRFS `temp_fsuid` + /// mount option, which was introduced in Linux 6.7. + pub fn supports_btrfs_temp_fsuid(&self) -> bool { + (self.major, self.minor) >= (6, 7) + } +} + #[cfg(test)] mod tests { - use crate::uname; + use super::*; + #[test] fn test_kernel_release() { - uname::kernel_release().unwrap(); + kernel_release().unwrap(); + } + + #[test] + fn test_parse_azl_kernel() { + let v = KernelVersion::parse("6.6.78.2-1.cm2").unwrap(); + assert_eq!(v, KernelVersion { major: 6, minor: 6 }); + assert!(!v.supports_btrfs_temp_fsuid()); + } + + #[test] + fn test_parse_67_kernel() { + let v = KernelVersion::parse("6.7.0-1.cm2").unwrap(); + assert_eq!(v, KernelVersion { major: 6, minor: 7 }); + assert!(v.supports_btrfs_temp_fsuid()); + } + + #[test] + fn test_parse_major_7() { + let v = KernelVersion::parse("7.0.0").unwrap(); + assert_eq!(v, KernelVersion { major: 7, minor: 0 }); + assert!(v.supports_btrfs_temp_fsuid()); + } + + #[test] + fn test_parse_simple() { + let v = KernelVersion::parse("5.15").unwrap(); + assert_eq!(v, KernelVersion { major: 5, minor: 15 }); + assert!(!v.supports_btrfs_temp_fsuid()); + } + + #[test] + fn test_parse_garbage() { + assert!(KernelVersion::parse("not-a-version").is_none()); + assert!(KernelVersion::parse("").is_none()); + assert!(KernelVersion::parse("6").is_none()); } } diff --git a/crates/trident/src/engine/newroot.rs b/crates/trident/src/engine/newroot.rs index 73c837a336..4c7266a0ce 100644 --- a/crates/trident/src/engine/newroot.rs +++ b/crates/trident/src/engine/newroot.rs @@ -179,10 +179,9 @@ impl NewrootMount { } } - // Check for ACL BTRFS UUID collision before mounting. - let acl_collision_uuid = - detect_acl_btrfs_uuid_collision(update_volume, staging_usr_roothash) - .structured(ServicingError::MountNewroot)?; + // Check for ACL BTRFS UUID collision and determine resolution strategy. + let acl_collision_resolution = + resolve_acl_btrfs_uuid_collision(update_volume, staging_usr_roothash); // Mount all block devices in the newroot mount_points_map(host_config) @@ -223,33 +222,68 @@ impl NewrootMount { let fs_type = block_device.fstype.and_then(|fs_type| KernelFilesystemType::from(fs_type.as_str()).try_as_real()); // ACL-specific: if the staging device has a BTRFS filesystem UUID that - // collides with the active USR partition, bind-mount from the host's - // /usr instead. The verity-protected filesystem is read-only and the - // content is identical when UUIDs match, so the bind mount provides - // equivalent content for chroot provisioning. - if let Some(ref collision_uuid) = acl_collision_uuid { + // collides with the active USR partition, resolve based on kernel version: + // - Kernel >=6.7: mount with -o temp_fsuid (staging device directly) + // - Kernel <6.7: bind-mount from active /usr (verity-verified identical) + if let Some(ref resolution) = acl_collision_resolution { + let collision_uuid = match resolution { + AclBtrfsCollisionResolution::TempFsuid { collision_uuid } => collision_uuid, + AclBtrfsCollisionResolution::BindMountActiveUsr { collision_uuid } => { + collision_uuid + } + }; if *path == Path::new(USR_MOUNT_POINT_PATH) && fs_type == Some(RealFilesystemType::Btrfs) && block_device.fsuuid.as_ref() == Some(collision_uuid) { - let active_usr = Path::new(USR_MOUNT_POINT_PATH); - warn!( - "Block device '{}' has BTRFS filesystem UUID '{}' which collides \ - with the active ACL USR partition. Bind-mounting '{}' to '{}' instead.", - target_id, - collision_uuid, - active_usr.display(), - target_path.display() - ); - do_bind_mount(active_usr, &target_path, MountFlags::RDONLY) - .with_context(|| { - format!( - "Failed to bind mount '{}' to '{}' \ - for ACL BTRFS UUID collision workaround", - active_usr.display(), - target_path.display(), + match resolution { + AclBtrfsCollisionResolution::TempFsuid { .. } => { + // Kernel >=6.7: mount the staging device with temp_fsuid. + let mut options = mp.options.to_string_vec(); + options.push("temp_fsuid".to_string()); + warn!( + "Block device '{}' has BTRFS filesystem UUID '{}' which \ + collides with the active ACL USR partition. Mounting with \ + temp_fsuid option (kernel >=6.7).", + target_id, collision_uuid, + ); + mount::mount( + device_path, + &target_path, + MountFileSystemType::Auto, + &options, ) - })?; + .context(format!( + "Failed to mount block device '{}' with temp_fsuid \ + for ACL BTRFS UUID collision (device path '{}', target '{}')", + target_id, + device_path.display(), + target_path.display() + ))?; + } + AclBtrfsCollisionResolution::BindMountActiveUsr { .. } => { + // Kernel <6.7: bind-mount from active /usr. + let active_usr = Path::new("/usr"); + warn!( + "Block device '{}' has BTRFS filesystem UUID '{}' which \ + collides with the active ACL USR partition. Bind-mounting \ + '{}' to '{}' instead (kernel <6.7).", + target_id, + collision_uuid, + active_usr.display(), + target_path.display() + ); + do_bind_mount(active_usr, &target_path, MountFlags::RDONLY) + .with_context(|| { + format!( + "Failed to bind mount '{}' to '{}' \ + for ACL BTRFS UUID collision workaround", + active_usr.display(), + target_path.display(), + ) + })?; + } + } self.add_mount(target_path.clone()); return Ok(()); } @@ -392,28 +426,77 @@ fn should_be_bind_mounted(fs_type: Option) -> bool { } } -/// Detects a BTRFS filesystem UUID collision on ACL's USR A/B partitions. +/// How to resolve a BTRFS UUID collision on ACL's USR A/B partitions. +#[derive(Debug)] +enum AclBtrfsCollisionResolution { + /// Kernel ≥6.7: mount the staging device with `-o temp_fsuid` so BTRFS + /// assigns a temporary in-memory UUID, bypassing the global registry. + TempFsuid { collision_uuid: OsUuid }, + /// Kernel <6.7: bind-mount from the active `/usr` (requires verity hash + /// verification to prove the content is identical). + BindMountActiveUsr { collision_uuid: OsUuid }, +} + +/// Detects a BTRFS filesystem UUID collision on ACL's USR A/B partitions and +/// determines how to resolve it based on the running kernel version. /// /// BTRFS maintains a kernel-global UUID registry and refuses to mount a filesystem /// whose UUID is already registered by another mounted device. During A/B updates /// where the COSI image shares filesystem UUIDs with the active OS, the staging -/// verity device cannot be mounted. +/// verity device cannot be mounted directly. /// -/// This function checks whether the active and update USR partitions (identified by -/// their well-known ACL PARTUUIDs) have the same BTRFS filesystem UUID. If so, it -/// returns the colliding UUID so the caller can substitute a bind mount from the -/// active `/usr`. +/// Resolution strategy: +/// - Kernel ≥6.7: use `mount -o temp_fsuid` (mounts the real staging device) +/// - Kernel <6.7: bind-mount from active `/usr` (requires verity hash match) /// -/// Returns: -/// - `Ok(Some(uuid))` — collision detected and verity-verified; use bind mount -/// - `Ok(None)` — no collision (not ACL, not BTRFS, or different UUIDs) -/// - `Err(...)` — collision detected but content identity could not be verified; -/// mounting will fail so the caller should surface this error rather than -/// letting BTRFS produce a confusing kernel-level error -fn detect_acl_btrfs_uuid_collision( +/// Returns `None` if no collision exists or if the bind-mount path is unsafe. +fn resolve_acl_btrfs_uuid_collision( update_volume: AbVolumeSelection, staging_usr_roothash: Option<&str>, -) -> Result, Error> { +) -> Option { + // 1. Detect whether a UUID collision exists. + let collision_uuid = detect_acl_btrfs_uuid_collision(update_volume)?; + + // 2. Determine resolution strategy based on kernel version. + let kernel_version = osutils::uname::KernelVersion::running() + .map_err(|e| warn!("Failed to determine kernel version: {e}")) + .ok() + .flatten(); + + if let Some(kv) = kernel_version { + debug!( + "Running kernel {}.{}, BTRFS temp_fsuid supported: {}", + kv.major, + kv.minor, + kv.supports_btrfs_temp_fsuid() + ); + if kv.supports_btrfs_temp_fsuid() { + // Kernel ≥6.7: mount the staging device directly with temp_fsuid. + // No verity hash check needed — we're mounting the real staging content. + return Some(AclBtrfsCollisionResolution::TempFsuid { collision_uuid }); + } + } else { + warn!( + "Could not parse kernel version; falling back to bind-mount strategy \ + for ACL BTRFS UUID collision" + ); + } + + // 3. Kernel <6.7 (or unknown): bind-mount from active /usr. + // This requires verity hash verification to prove content is identical. + if !verify_acl_bind_mount_safety(staging_usr_roothash) { + return None; + } + + Some(AclBtrfsCollisionResolution::BindMountActiveUsr { collision_uuid }) +} + +/// Detects a BTRFS filesystem UUID collision on ACL's USR A/B partitions. +/// +/// Returns the colliding UUID if both the active and update USR partitions +/// (identified by well-known ACL PARTUUIDs) are BTRFS and share the same +/// filesystem UUID. Returns `None` otherwise. +fn detect_acl_btrfs_uuid_collision(update_volume: AbVolumeSelection) -> Option { let (active_partuuid, update_partuuid) = match update_volume { AbVolumeSelection::VolumeA => (acl::ACL_USR_B_PARTUUID, acl::ACL_USR_A_PARTUUID), AbVolumeSelection::VolumeB => (acl::ACL_USR_A_PARTUUID, acl::ACL_USR_B_PARTUUID), @@ -423,21 +506,13 @@ fn detect_acl_btrfs_uuid_collision( let update_path = block_devices::part_uuid_path(update_partuuid); // On non-ACL systems these PARTUUID paths won't exist. Check before - // calling lsblk so we return Ok(None) instead of a confusing error. + // calling lsblk so we return None instead of a confusing error. if !active_path.exists() || !update_path.exists() { - return Ok(None); + return None; } - let Some(active_dev) = - lsblk::try_get(&active_path).context("Failed to query active ACL USR partition")? - else { - return Ok(None); - }; - let Some(update_dev) = - lsblk::try_get(&update_path).context("Failed to query update ACL USR partition")? - else { - return Ok(None); - }; + let active_dev = lsblk::try_get(&active_path).ok().flatten()?; + let update_dev = lsblk::try_get(&update_path).ok().flatten()?; let active_fstype = active_dev .fstype @@ -449,18 +524,18 @@ fn detect_acl_btrfs_uuid_collision( .and_then(|fs| KernelFilesystemType::from(fs).try_as_real()); if active_fstype != Some(RealFilesystemType::Btrfs) { - return Ok(None); + return None; } if update_fstype != Some(RealFilesystemType::Btrfs) { - return Ok(None); + return None; } let (Some(active_uuid), Some(update_uuid)) = (active_dev.fsuuid, update_dev.fsuuid) else { - return Ok(None); + return None; }; if active_uuid != update_uuid { - return Ok(None); + return None; } debug!( @@ -468,23 +543,25 @@ fn detect_acl_btrfs_uuid_collision( share filesystem UUID '{active_uuid}'" ); - // When a staging root hash is available, verify that the active USR - // partition has the same verity root hash. This provides a cryptographic - // guarantee that the filesystems are byte-identical, not just a UUID match. + Some(active_uuid) +} + +/// Verifies that bind-mounting from the active `/usr` is safe by comparing +/// verity root hashes. Returns true if the hashes match, false otherwise. +fn verify_acl_bind_mount_safety(staging_usr_roothash: Option<&str>) -> bool { let Some(staging_hash) = staging_usr_roothash else { - bail!( - "ACL BTRFS UUID collision detected (filesystem UUID '{active_uuid}') but no \ - staging USR verity root hash is available to verify content identity. \ - Cannot safely bind-mount or directly mount the USR partition." - ); + // No staging hash available — can't verify, but allow the bind mount + // since the upstream validation (validate_acl_duplicate_uuid) already + // verified the hashes match when they were available. + return true; }; let Some(staging) = VerityRootHash::new(staging_hash) else { - bail!( - "ACL BTRFS UUID collision detected (filesystem UUID '{active_uuid}') but \ - staging USR verity root hash is empty. \ - Cannot safely bind-mount or directly mount the USR partition." + warn!( + "Staging USR verity root hash is empty. \ + Refusing bind-mount despite UUID collision." ); + return false; }; match VerityRootHash::from_proc_cmdline() { @@ -495,26 +572,25 @@ fn detect_acl_btrfs_uuid_collision( partitions have matching root hash ({}...)", staging.preview() ); + true } else { - bail!( - "ACL BTRFS UUID collision detected (filesystem UUID '{active_uuid}') \ - but verity root hash mismatch: active USR has '{}...', staging has '{}...'. \ - Cannot safely bind-mount or directly mount the USR partition.", + warn!( + "Verity root hash mismatch: active USR has '{}...', staging has '{}...'. \ + Refusing bind-mount despite UUID collision.", active.preview(), staging.preview() ); + false } } None => { - bail!( - "ACL BTRFS UUID collision detected (filesystem UUID '{active_uuid}') \ - but cannot read active USR verity root hash from /proc/cmdline. \ - Cannot safely bind-mount or directly mount the USR partition." + warn!( + "Cannot read active USR verity root hash from /proc/cmdline. \ + Refusing bind-mount despite UUID collision." ); + false } } - - Ok(Some(active_uuid)) } /// Returns an ordered map of mount points to their corresponding FileSystem objects. From 8a3ed81879fd8655ac6805a19c4c92686919baa2 Mon Sep 17 00:00:00 2001 From: bfjelds Date: Thu, 4 Jun 2026 11:16:38 -0700 Subject: [PATCH 02/10] style: rustfmt fix in uname tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/osutils/src/uname.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/osutils/src/uname.rs b/crates/osutils/src/uname.rs index 9fc6ac0da2..7a291ef600 100644 --- a/crates/osutils/src/uname.rs +++ b/crates/osutils/src/uname.rs @@ -82,7 +82,13 @@ mod tests { #[test] fn test_parse_simple() { let v = KernelVersion::parse("5.15").unwrap(); - assert_eq!(v, KernelVersion { major: 5, minor: 15 }); + assert_eq!( + v, + KernelVersion { + major: 5, + minor: 15 + } + ); assert!(!v.supports_btrfs_temp_fsuid()); } From 0635702f80ca1f0c8b15c21b6967b61eaf9ecc14 Mon Sep 17 00:00:00 2001 From: bfjelds Date: Thu, 4 Jun 2026 11:18:22 -0700 Subject: [PATCH 03/10] docs: note temp_fsuid codepath is aspirational/untested Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/trident/src/engine/newroot.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/trident/src/engine/newroot.rs b/crates/trident/src/engine/newroot.rs index 4c7266a0ce..f74026e759 100644 --- a/crates/trident/src/engine/newroot.rs +++ b/crates/trident/src/engine/newroot.rs @@ -239,6 +239,9 @@ impl NewrootMount { match resolution { AclBtrfsCollisionResolution::TempFsuid { .. } => { // Kernel >=6.7: mount the staging device with temp_fsuid. + // NOTE: This codepath is aspirational. We believe it will + // work, but until trident A/B update and ACL run on a + // kernel >6.6, it is untested in production. let mut options = mp.options.to_string_vec(); options.push("temp_fsuid".to_string()); warn!( From ce38e103735f3ecbcd737b8c274c6bd445951783 Mon Sep 17 00:00:00 2001 From: bfjelds Date: Fri, 5 Jun 2026 11:37:38 -0700 Subject: [PATCH 04/10] feat: gate BTRFS temp_fsuid behind enableAzl4 internal param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The temp_fsuid mount path (kernel >=6.7) is aspirational and untested in production. Gate it behind the enableAzl4 internal parameter so it only activates when explicitly opted in. When the flag is absent, the bind-mount fallback is used. No special warning or fallback from temp_fsuid failure — mount errors propagate as-is to surface issues. The enableAzl4 flag is intentionally broad: it will gate additional Azure Linux 4 behaviors as they are added. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/trident/src/engine/newroot.rs | 73 ++++++++++++++++------------ crates/trident_api/src/constants.rs | 6 +++ 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/crates/trident/src/engine/newroot.rs b/crates/trident/src/engine/newroot.rs index f74026e759..3c9ef2083c 100644 --- a/crates/trident/src/engine/newroot.rs +++ b/crates/trident/src/engine/newroot.rs @@ -22,8 +22,8 @@ use sysdefs::{ use trident_api::{ config::{FileSystem, HostConfiguration}, constants::{ - NONE_MOUNT_POINT, ROOT_MOUNT_POINT_PATH, UPDATE_ROOT_FALLBACK_PATH, UPDATE_ROOT_PATH, - USR_MOUNT_POINT_PATH, + internal_params, NONE_MOUNT_POINT, ROOT_MOUNT_POINT_PATH, UPDATE_ROOT_FALLBACK_PATH, + UPDATE_ROOT_PATH, USR_MOUNT_POINT_PATH, }, error::{InternalError, ReportError, ServicingError, TridentError, TridentResultExt}, status::AbVolumeSelection, @@ -180,8 +180,13 @@ impl NewrootMount { } // Check for ACL BTRFS UUID collision and determine resolution strategy. - let acl_collision_resolution = - resolve_acl_btrfs_uuid_collision(update_volume, staging_usr_roothash); + let acl_collision_resolution = resolve_acl_btrfs_uuid_collision( + update_volume, + staging_usr_roothash, + host_config + .internal_params + .get_flag(internal_params::ENABLE_AZL4), + ); // Mount all block devices in the newroot mount_points_map(host_config) @@ -441,7 +446,8 @@ enum AclBtrfsCollisionResolution { } /// Detects a BTRFS filesystem UUID collision on ACL's USR A/B partitions and -/// determines how to resolve it based on the running kernel version. +/// determines how to resolve it based on the running kernel version and +/// the `enableAzl4` internal parameter. /// /// BTRFS maintains a kernel-global UUID registry and refuses to mount a filesystem /// whose UUID is already registered by another mounted device. During A/B updates @@ -449,44 +455,51 @@ enum AclBtrfsCollisionResolution { /// verity device cannot be mounted directly. /// /// Resolution strategy: -/// - Kernel ≥6.7: use `mount -o temp_fsuid` (mounts the real staging device) -/// - Kernel <6.7: bind-mount from active `/usr` (requires verity hash match) +/// - `enable_azl4` + Kernel ≥6.7: use `mount -o temp_fsuid` (mounts the real staging device) +/// - Otherwise: bind-mount from active `/usr` (requires verity hash match) /// /// Returns `None` if no collision exists or if the bind-mount path is unsafe. fn resolve_acl_btrfs_uuid_collision( update_volume: AbVolumeSelection, staging_usr_roothash: Option<&str>, + enable_azl4: bool, ) -> Option { // 1. Detect whether a UUID collision exists. let collision_uuid = detect_acl_btrfs_uuid_collision(update_volume)?; // 2. Determine resolution strategy based on kernel version. - let kernel_version = osutils::uname::KernelVersion::running() - .map_err(|e| warn!("Failed to determine kernel version: {e}")) - .ok() - .flatten(); - - if let Some(kv) = kernel_version { - debug!( - "Running kernel {}.{}, BTRFS temp_fsuid supported: {}", - kv.major, - kv.minor, - kv.supports_btrfs_temp_fsuid() - ); - if kv.supports_btrfs_temp_fsuid() { - // Kernel ≥6.7: mount the staging device directly with temp_fsuid. - // No verity hash check needed — we're mounting the real staging content. - return Some(AclBtrfsCollisionResolution::TempFsuid { collision_uuid }); + // The temp_fsuid path requires the enableAzl4 internal param to be set. + // When the flag is absent, skip directly to the bind-mount path — failure + // to mount is desired so that missing configuration is surfaced early. + if enable_azl4 { + let kernel_version = osutils::uname::KernelVersion::running() + .map_err(|e| warn!("Failed to determine kernel version: {e}")) + .ok() + .flatten(); + + if let Some(kv) = kernel_version { + debug!( + "Running kernel {}.{}, BTRFS temp_fsuid supported: {}", + kv.major, + kv.minor, + kv.supports_btrfs_temp_fsuid() + ); + if kv.supports_btrfs_temp_fsuid() { + // Kernel ≥6.7: mount the staging device directly with temp_fsuid. + // No verity hash check needed — we're mounting the real staging content. + return Some(AclBtrfsCollisionResolution::TempFsuid { collision_uuid }); + } + } else { + warn!( + "Could not parse kernel version; falling back to bind-mount strategy \ + for ACL BTRFS UUID collision" + ); } - } else { - warn!( - "Could not parse kernel version; falling back to bind-mount strategy \ - for ACL BTRFS UUID collision" - ); } - // 3. Kernel <6.7 (or unknown): bind-mount from active /usr. - // This requires verity hash verification to prove content is identical. + // 3. Kernel <6.7, unknown kernel, or enableAzl4 not set: bind-mount from + // active /usr. This requires verity hash verification to prove content + // is identical. if !verify_acl_bind_mount_safety(staging_usr_roothash) { return None; } diff --git a/crates/trident_api/src/constants.rs b/crates/trident_api/src/constants.rs index 7e7549b875..ec4fd0a763 100644 --- a/crates/trident_api/src/constants.rs +++ b/crates/trident_api/src/constants.rs @@ -211,6 +211,12 @@ pub mod internal_params { /// Run dracut in debug mode to capture more output. pub const DRACUT_DEBUG: &str = "dracutDebug"; + /// Enable Azure Linux 4 specific behaviors. Gates features that depend on + /// AZL4 kernel capabilities (e.g., BTRFS temp_fsuid mount option on + /// kernel ≥6.7). Must be explicitly set; absence means AZL4 codepaths + /// are not activated. + pub const ENABLE_AZL4: &str = "enableAzl4"; + /// Enable support for Harpoon to query for updated Host Config documents. pub const ENABLE_HARPOON_SUPPORT: &str = "harpoon"; From 62427f6e704d6704d733160c043b70d99311c3c7 Mon Sep 17 00:00:00 2001 From: bfjelds Date: Fri, 5 Jun 2026 11:58:50 -0700 Subject: [PATCH 05/10] refactor: address deep review findings DR-002 through DR-005 DR-002: Move BTRFS temp_fsuid domain knowledge out of osutils. Remove supports_btrfs_temp_fsuid() from KernelVersion (generic layer) and define BTRFS_TEMP_FSUID_MIN_KERNEL constant in the consumer (newroot.rs). KernelVersion now relies on derived Ord for version comparisons. DR-003: Distinguish uname execution failure from parse failure. The match on KernelVersion::running() now logs different warnings for Err (uname command failed) vs Ok(None) (output not parseable). DR-004: Add doc comment explaining why verity hash verification is intentionally skipped for the temp_fsuid path (it mounts real staging content, not a bind-mount of active, so no identity assumption to verify). DR-005: Eliminate double pattern match on AclBtrfsCollisionResolution in the mount loop. Add collision_uuid() accessor method so the UUID is extracted once, then dispatch on the resolution variant in a single match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/osutils/src/uname.rs | 20 ++++----- crates/trident/src/engine/newroot.rs | 65 +++++++++++++++++----------- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/crates/osutils/src/uname.rs b/crates/osutils/src/uname.rs index 7a291ef600..f6f48ccc41 100644 --- a/crates/osutils/src/uname.rs +++ b/crates/osutils/src/uname.rs @@ -12,6 +12,9 @@ pub fn kernel_release() -> Result { } /// Parsed kernel version with major and minor components. +/// +/// Implements `Ord` so callers can compare against feature thresholds +/// (e.g., `kv >= KernelVersion { major: 6, minor: 7 }`). #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub struct KernelVersion { pub major: u32, @@ -37,16 +40,13 @@ impl KernelVersion { } /// Returns the kernel version of the running system. + /// + /// Returns `Err` if the `uname` command fails to execute, or `Ok(None)` + /// if the output cannot be parsed into a major.minor version. pub fn running() -> Result, Error> { let release = kernel_release()?; Ok(Self::parse(&release)) } - - /// Returns true if this kernel version supports the BTRFS `temp_fsuid` - /// mount option, which was introduced in Linux 6.7. - pub fn supports_btrfs_temp_fsuid(&self) -> bool { - (self.major, self.minor) >= (6, 7) - } } #[cfg(test)] @@ -62,21 +62,21 @@ mod tests { fn test_parse_azl_kernel() { let v = KernelVersion::parse("6.6.78.2-1.cm2").unwrap(); assert_eq!(v, KernelVersion { major: 6, minor: 6 }); - assert!(!v.supports_btrfs_temp_fsuid()); + assert!(v < KernelVersion { major: 6, minor: 7 }); } #[test] fn test_parse_67_kernel() { let v = KernelVersion::parse("6.7.0-1.cm2").unwrap(); assert_eq!(v, KernelVersion { major: 6, minor: 7 }); - assert!(v.supports_btrfs_temp_fsuid()); + assert!(v >= KernelVersion { major: 6, minor: 7 }); } #[test] fn test_parse_major_7() { let v = KernelVersion::parse("7.0.0").unwrap(); assert_eq!(v, KernelVersion { major: 7, minor: 0 }); - assert!(v.supports_btrfs_temp_fsuid()); + assert!(v >= KernelVersion { major: 6, minor: 7 }); } #[test] @@ -89,7 +89,7 @@ mod tests { minor: 15 } ); - assert!(!v.supports_btrfs_temp_fsuid()); + assert!(v < KernelVersion { major: 6, minor: 7 }); } #[test] diff --git a/crates/trident/src/engine/newroot.rs b/crates/trident/src/engine/newroot.rs index 3c9ef2083c..706bd0b1c1 100644 --- a/crates/trident/src/engine/newroot.rs +++ b/crates/trident/src/engine/newroot.rs @@ -227,26 +227,17 @@ impl NewrootMount { let fs_type = block_device.fstype.and_then(|fs_type| KernelFilesystemType::from(fs_type.as_str()).try_as_real()); // ACL-specific: if the staging device has a BTRFS filesystem UUID that - // collides with the active USR partition, resolve based on kernel version: - // - Kernel >=6.7: mount with -o temp_fsuid (staging device directly) - // - Kernel <6.7: bind-mount from active /usr (verity-verified identical) + // collides with the active USR partition, resolve based on strategy: + // - enableAzl4 + kernel >=6.7: mount with -o temp_fsuid (staging device directly) + // - Otherwise: bind-mount from active /usr (verity-verified identical) if let Some(ref resolution) = acl_collision_resolution { - let collision_uuid = match resolution { - AclBtrfsCollisionResolution::TempFsuid { collision_uuid } => collision_uuid, - AclBtrfsCollisionResolution::BindMountActiveUsr { collision_uuid } => { - collision_uuid - } - }; + let collision_uuid = resolution.collision_uuid(); if *path == Path::new(USR_MOUNT_POINT_PATH) && fs_type == Some(RealFilesystemType::Btrfs) && block_device.fsuuid.as_ref() == Some(collision_uuid) { match resolution { AclBtrfsCollisionResolution::TempFsuid { .. } => { - // Kernel >=6.7: mount the staging device with temp_fsuid. - // NOTE: This codepath is aspirational. We believe it will - // work, but until trident A/B update and ACL run on a - // kernel >6.6, it is untested in production. let mut options = mp.options.to_string_vec(); options.push("temp_fsuid".to_string()); warn!( @@ -270,7 +261,6 @@ impl NewrootMount { ))?; } AclBtrfsCollisionResolution::BindMountActiveUsr { .. } => { - // Kernel <6.7: bind-mount from active /usr. let active_usr = Path::new("/usr"); warn!( "Block device '{}' has BTRFS filesystem UUID '{}' which \ @@ -434,6 +424,15 @@ fn should_be_bind_mounted(fs_type: Option) -> bool { } } +/// Minimum kernel version required for the BTRFS `temp_fsuid` mount option +/// (introduced in Linux 6.7). Domain-specific threshold owned by the consumer, +/// not by the generic `KernelVersion` type in osutils. +const BTRFS_TEMP_FSUID_MIN_KERNEL: osutils::uname::KernelVersion = + osutils::uname::KernelVersion { + major: 6, + minor: 7, + }; + /// How to resolve a BTRFS UUID collision on ACL's USR A/B partitions. #[derive(Debug)] enum AclBtrfsCollisionResolution { @@ -445,6 +444,16 @@ enum AclBtrfsCollisionResolution { BindMountActiveUsr { collision_uuid: OsUuid }, } +impl AclBtrfsCollisionResolution { + fn collision_uuid(&self) -> &OsUuid { + match self { + Self::TempFsuid { collision_uuid } | Self::BindMountActiveUsr { collision_uuid } => { + collision_uuid + } + } + } +} + /// Detects a BTRFS filesystem UUID collision on ACL's USR A/B partitions and /// determines how to resolve it based on the running kernel version and /// the `enableAzl4` internal parameter. @@ -472,27 +481,33 @@ fn resolve_acl_btrfs_uuid_collision( // When the flag is absent, skip directly to the bind-mount path — failure // to mount is desired so that missing configuration is surfaced early. if enable_azl4 { - let kernel_version = osutils::uname::KernelVersion::running() - .map_err(|e| warn!("Failed to determine kernel version: {e}")) - .ok() - .flatten(); + let kernel_version = match osutils::uname::KernelVersion::running() { + Ok(kv) => kv, + Err(e) => { + // DR-003: distinguish uname execution failure from parse failure. + warn!("Failed to execute uname: {e}; cannot determine kernel version"); + None + } + }; if let Some(kv) = kernel_version { + let supports_temp_fsuid = kv >= BTRFS_TEMP_FSUID_MIN_KERNEL; debug!( "Running kernel {}.{}, BTRFS temp_fsuid supported: {}", - kv.major, - kv.minor, - kv.supports_btrfs_temp_fsuid() + kv.major, kv.minor, supports_temp_fsuid ); - if kv.supports_btrfs_temp_fsuid() { + if supports_temp_fsuid { // Kernel ≥6.7: mount the staging device directly with temp_fsuid. - // No verity hash check needed — we're mounting the real staging content. + // Verity hash verification is intentionally skipped here: temp_fsuid + // mounts the real staging device content (not a bind-mount of the + // active partition), so there is no identity assumption to verify. return Some(AclBtrfsCollisionResolution::TempFsuid { collision_uuid }); } } else { + // uname succeeded but output could not be parsed into major.minor. warn!( - "Could not parse kernel version; falling back to bind-mount strategy \ - for ACL BTRFS UUID collision" + "Could not parse kernel version from uname output; \ + falling back to bind-mount strategy for ACL BTRFS UUID collision" ); } } From 66af46490c14129c835adcfe910f5687e226d349 Mon Sep 17 00:00:00 2001 From: bfjelds Date: Fri, 5 Jun 2026 12:11:18 -0700 Subject: [PATCH 06/10] style: rustfmt fix for BTRFS_TEMP_FSUID_MIN_KERNEL Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/trident/src/engine/newroot.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/trident/src/engine/newroot.rs b/crates/trident/src/engine/newroot.rs index 706bd0b1c1..f65172d36b 100644 --- a/crates/trident/src/engine/newroot.rs +++ b/crates/trident/src/engine/newroot.rs @@ -428,10 +428,7 @@ fn should_be_bind_mounted(fs_type: Option) -> bool { /// (introduced in Linux 6.7). Domain-specific threshold owned by the consumer, /// not by the generic `KernelVersion` type in osutils. const BTRFS_TEMP_FSUID_MIN_KERNEL: osutils::uname::KernelVersion = - osutils::uname::KernelVersion { - major: 6, - minor: 7, - }; + osutils::uname::KernelVersion { major: 6, minor: 7 }; /// How to resolve a BTRFS UUID collision on ACL's USR A/B partitions. #[derive(Debug)] From 83309feb2129629e28a4cc4a9a96ab65b3e8571d Mon Sep 17 00:00:00 2001 From: bfjelds Date: Tue, 14 Jul 2026 10:03:47 -0700 Subject: [PATCH 07/10] fix: address PR review - fail closed on ACL BTRFS collision edge cases - verify_acl_bind_mount_safety: refuse bind-mount when no staging verity hash is available instead of allowing it, matching the BindMountActiveUsr verity-proof contract. - detect_acl_btrfs_uuid_collision: warn when lsblk errors instead of silently swallowing the error. - resolve_acl_btrfs_uuid_collision: correct a misleading comment about enableAzl4-absent behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/trident/src/engine/newroot.rs | 46 +++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/crates/trident/src/engine/newroot.rs b/crates/trident/src/engine/newroot.rs index f65172d36b..edd0013b52 100644 --- a/crates/trident/src/engine/newroot.rs +++ b/crates/trident/src/engine/newroot.rs @@ -475,8 +475,9 @@ fn resolve_acl_btrfs_uuid_collision( // 2. Determine resolution strategy based on kernel version. // The temp_fsuid path requires the enableAzl4 internal param to be set. - // When the flag is absent, skip directly to the bind-mount path — failure - // to mount is desired so that missing configuration is surfaced early. + // When the flag is absent (or the running kernel predates 6.7), skip the + // temp_fsuid path and fall through to the verity-verified bind-mount + // strategy below. if enable_azl4 { let kernel_version = match osutils::uname::KernelVersion::running() { Ok(kv) => kv, @@ -539,8 +540,30 @@ fn detect_acl_btrfs_uuid_collision(update_volume: AbVolumeSelection) -> Option dev?, + Err(e) => { + warn!( + "Failed to query block device '{}' via lsblk while detecting an ACL BTRFS \ + UUID collision: {e}. Treating as no collision; a genuine collision will \ + surface later as a mount failure.", + active_path.display() + ); + return None; + } + }; + let update_dev = match lsblk::try_get(&update_path) { + Ok(dev) => dev?, + Err(e) => { + warn!( + "Failed to query block device '{}' via lsblk while detecting an ACL BTRFS \ + UUID collision: {e}. Treating as no collision; a genuine collision will \ + surface later as a mount failure.", + update_path.display() + ); + return None; + } + }; let active_fstype = active_dev .fstype @@ -578,10 +601,17 @@ fn detect_acl_btrfs_uuid_collision(update_volume: AbVolumeSelection) -> Option) -> bool { let Some(staging_hash) = staging_usr_roothash else { - // No staging hash available — can't verify, but allow the bind mount - // since the upstream validation (validate_acl_duplicate_uuid) already - // verified the hashes match when they were available. - return true; + // No staging verity root hash available. A genuine ACL /usr UUID collision + // cannot reach this point without upstream validation + // (validate_acl_duplicate_uuid) having already confirmed a staging verity + // hash exists, so a missing hash here is anomalous. Fail closed: refuse the + // bind-mount rather than mounting the active /usr without cryptographic + // identity proof. The collision then surfaces as an explicit mount failure. + warn!( + "No staging USR verity root hash available for ACL BTRFS UUID collision. \ + Refusing bind-mount to avoid mounting /usr without verity verification." + ); + return false; }; let Some(staging) = VerityRootHash::new(staging_hash) else { From 69ea2d9605ab15fe9d9141e4bef9f235ec7d4d22 Mon Sep 17 00:00:00 2001 From: bfjelds Date: Tue, 14 Jul 2026 10:18:40 -0700 Subject: [PATCH 08/10] fix: correct BTRFS mount option to temp_fsid; clarify uname failure logging The Linux 6.7 BTRFS option is temp_fsid, not temp_fsuid; the old string would fail at mount time with an unknown-option error. Rename the runtime mount string, enum variant, constant, local var, and all comments/docstrings. Restructure the kernel-version gating so uname execution failure and parse failure emit distinct warnings instead of both logging 'could not parse'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/trident/src/engine/newroot.rs | 79 ++++++++++++++-------------- crates/trident_api/src/constants.rs | 2 +- 2 files changed, 41 insertions(+), 40 deletions(-) diff --git a/crates/trident/src/engine/newroot.rs b/crates/trident/src/engine/newroot.rs index edd0013b52..8265406143 100644 --- a/crates/trident/src/engine/newroot.rs +++ b/crates/trident/src/engine/newroot.rs @@ -228,7 +228,7 @@ impl NewrootMount { // ACL-specific: if the staging device has a BTRFS filesystem UUID that // collides with the active USR partition, resolve based on strategy: - // - enableAzl4 + kernel >=6.7: mount with -o temp_fsuid (staging device directly) + // - enableAzl4 + kernel >=6.7: mount with -o temp_fsid (staging device directly) // - Otherwise: bind-mount from active /usr (verity-verified identical) if let Some(ref resolution) = acl_collision_resolution { let collision_uuid = resolution.collision_uuid(); @@ -237,13 +237,13 @@ impl NewrootMount { && block_device.fsuuid.as_ref() == Some(collision_uuid) { match resolution { - AclBtrfsCollisionResolution::TempFsuid { .. } => { + AclBtrfsCollisionResolution::TempFsid { .. } => { let mut options = mp.options.to_string_vec(); - options.push("temp_fsuid".to_string()); + options.push("temp_fsid".to_string()); warn!( "Block device '{}' has BTRFS filesystem UUID '{}' which \ collides with the active ACL USR partition. Mounting with \ - temp_fsuid option (kernel >=6.7).", + temp_fsid option (kernel >=6.7).", target_id, collision_uuid, ); mount::mount( @@ -253,7 +253,7 @@ impl NewrootMount { &options, ) .context(format!( - "Failed to mount block device '{}' with temp_fsuid \ + "Failed to mount block device '{}' with temp_fsid \ for ACL BTRFS UUID collision (device path '{}', target '{}')", target_id, device_path.display(), @@ -424,18 +424,18 @@ fn should_be_bind_mounted(fs_type: Option) -> bool { } } -/// Minimum kernel version required for the BTRFS `temp_fsuid` mount option +/// Minimum kernel version required for the BTRFS `temp_fsid` mount option /// (introduced in Linux 6.7). Domain-specific threshold owned by the consumer, /// not by the generic `KernelVersion` type in osutils. -const BTRFS_TEMP_FSUID_MIN_KERNEL: osutils::uname::KernelVersion = +const BTRFS_TEMP_FSID_MIN_KERNEL: osutils::uname::KernelVersion = osutils::uname::KernelVersion { major: 6, minor: 7 }; /// How to resolve a BTRFS UUID collision on ACL's USR A/B partitions. #[derive(Debug)] enum AclBtrfsCollisionResolution { - /// Kernel ≥6.7: mount the staging device with `-o temp_fsuid` so BTRFS + /// Kernel ≥6.7: mount the staging device with `-o temp_fsid` so BTRFS /// assigns a temporary in-memory UUID, bypassing the global registry. - TempFsuid { collision_uuid: OsUuid }, + TempFsid { collision_uuid: OsUuid }, /// Kernel <6.7: bind-mount from the active `/usr` (requires verity hash /// verification to prove the content is identical). BindMountActiveUsr { collision_uuid: OsUuid }, @@ -444,7 +444,7 @@ enum AclBtrfsCollisionResolution { impl AclBtrfsCollisionResolution { fn collision_uuid(&self) -> &OsUuid { match self { - Self::TempFsuid { collision_uuid } | Self::BindMountActiveUsr { collision_uuid } => { + Self::TempFsid { collision_uuid } | Self::BindMountActiveUsr { collision_uuid } => { collision_uuid } } @@ -461,7 +461,7 @@ impl AclBtrfsCollisionResolution { /// verity device cannot be mounted directly. /// /// Resolution strategy: -/// - `enable_azl4` + Kernel ≥6.7: use `mount -o temp_fsuid` (mounts the real staging device) +/// - `enable_azl4` + Kernel ≥6.7: use `mount -o temp_fsid` (mounts the real staging device) /// - Otherwise: bind-mount from active `/usr` (requires verity hash match) /// /// Returns `None` if no collision exists or if the bind-mount path is unsafe. @@ -474,39 +474,40 @@ fn resolve_acl_btrfs_uuid_collision( let collision_uuid = detect_acl_btrfs_uuid_collision(update_volume)?; // 2. Determine resolution strategy based on kernel version. - // The temp_fsuid path requires the enableAzl4 internal param to be set. + // The temp_fsid path requires the enableAzl4 internal param to be set. // When the flag is absent (or the running kernel predates 6.7), skip the - // temp_fsuid path and fall through to the verity-verified bind-mount + // temp_fsid path and fall through to the verity-verified bind-mount // strategy below. if enable_azl4 { - let kernel_version = match osutils::uname::KernelVersion::running() { - Ok(kv) => kv, - Err(e) => { - // DR-003: distinguish uname execution failure from parse failure. - warn!("Failed to execute uname: {e}; cannot determine kernel version"); - None + match osutils::uname::KernelVersion::running() { + Ok(Some(kv)) => { + let supports_temp_fsid = kv >= BTRFS_TEMP_FSID_MIN_KERNEL; + debug!( + "Running kernel {}.{}, BTRFS temp_fsid supported: {}", + kv.major, kv.minor, supports_temp_fsid + ); + if supports_temp_fsid { + // Kernel ≥6.7: mount the staging device directly with temp_fsid. + // Verity hash verification is intentionally skipped here: temp_fsid + // mounts the real staging device content (not a bind-mount of the + // active partition), so there is no identity assumption to verify. + return Some(AclBtrfsCollisionResolution::TempFsid { collision_uuid }); + } } - }; - - if let Some(kv) = kernel_version { - let supports_temp_fsuid = kv >= BTRFS_TEMP_FSUID_MIN_KERNEL; - debug!( - "Running kernel {}.{}, BTRFS temp_fsuid supported: {}", - kv.major, kv.minor, supports_temp_fsuid - ); - if supports_temp_fsuid { - // Kernel ≥6.7: mount the staging device directly with temp_fsuid. - // Verity hash verification is intentionally skipped here: temp_fsuid - // mounts the real staging device content (not a bind-mount of the - // active partition), so there is no identity assumption to verify. - return Some(AclBtrfsCollisionResolution::TempFsuid { collision_uuid }); + Ok(None) => { + // uname succeeded but output could not be parsed into major.minor. + warn!( + "Could not parse kernel version from uname output; \ + falling back to bind-mount strategy for ACL BTRFS UUID collision" + ); + } + Err(e) => { + // uname could not be executed at all (DR-003: distinct from parse failure). + warn!( + "Failed to execute uname: {e}; cannot determine kernel version, \ + falling back to bind-mount strategy for ACL BTRFS UUID collision" + ); } - } else { - // uname succeeded but output could not be parsed into major.minor. - warn!( - "Could not parse kernel version from uname output; \ - falling back to bind-mount strategy for ACL BTRFS UUID collision" - ); } } diff --git a/crates/trident_api/src/constants.rs b/crates/trident_api/src/constants.rs index ec4fd0a763..d152488ab3 100644 --- a/crates/trident_api/src/constants.rs +++ b/crates/trident_api/src/constants.rs @@ -212,7 +212,7 @@ pub mod internal_params { pub const DRACUT_DEBUG: &str = "dracutDebug"; /// Enable Azure Linux 4 specific behaviors. Gates features that depend on - /// AZL4 kernel capabilities (e.g., BTRFS temp_fsuid mount option on + /// AZL4 kernel capabilities (e.g., BTRFS temp_fsid mount option on /// kernel ≥6.7). Must be explicitly set; absence means AZL4 codepaths /// are not activated. pub const ENABLE_AZL4: &str = "enableAzl4"; From 9971ecf65c481cba82c6026129475fa389c3243d Mon Sep 17 00:00:00 2001 From: bfjelds Date: Tue, 14 Jul 2026 10:36:20 -0700 Subject: [PATCH 09/10] fix: harden ACL BTRFS collision handling per review - KernelVersion::parse: trim uname output so a trailing newline on a bare major.minor release (e.g. '5.15\n') no longer fails to parse. - detect_acl_btrfs_uuid_collision: warn explicitly when lsblk returns Ok(None) instead of swallowing it, matching the Err treatment. - resolve_acl_btrfs_uuid_collision: return Result and fail with a structured AclBtrfsUuidCollisionUnresolved error (carrying the verity reason) when a collision exists but cannot be safely resolved, instead of returning None and deferring to an opaque mount failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/osutils/src/uname.rs | 20 +++++- crates/trident/src/engine/newroot.rs | 96 +++++++++++++++++----------- crates/trident_api/src/error.rs | 6 ++ 3 files changed, 84 insertions(+), 38 deletions(-) diff --git a/crates/osutils/src/uname.rs b/crates/osutils/src/uname.rs index f6f48ccc41..dfc08afde1 100644 --- a/crates/osutils/src/uname.rs +++ b/crates/osutils/src/uname.rs @@ -31,8 +31,9 @@ impl KernelVersion { /// /// Returns `None` if the string cannot be parsed. pub fn parse(release: &str) -> Option { - // Strip everything after the first '-' (e.g. "-1.cm2"), then split on '.'. - let numeric_part = release.split('-').next()?; + // Trim surrounding whitespace/newlines (uname output is not trimmed), + // strip everything after the first '-' (e.g. "-1.cm2"), then split on '.'. + let numeric_part = release.trim().split('-').next()?; let mut parts = numeric_part.split('.'); let major = parts.next()?.parse::().ok()?; let minor = parts.next()?.parse::().ok()?; @@ -98,4 +99,19 @@ mod tests { assert!(KernelVersion::parse("").is_none()); assert!(KernelVersion::parse("6").is_none()); } + + #[test] + fn test_parse_trailing_newline() { + // uname output is not trimmed, so parse must tolerate trailing whitespace. + let v = KernelVersion::parse("5.15\n").unwrap(); + assert_eq!( + v, + KernelVersion { + major: 5, + minor: 15 + } + ); + let v = KernelVersion::parse("6.7.0-1.cm2\n").unwrap(); + assert_eq!(v, KernelVersion { major: 6, minor: 7 }); + } } diff --git a/crates/trident/src/engine/newroot.rs b/crates/trident/src/engine/newroot.rs index 8265406143..111a28be0b 100644 --- a/crates/trident/src/engine/newroot.rs +++ b/crates/trident/src/engine/newroot.rs @@ -186,7 +186,7 @@ impl NewrootMount { host_config .internal_params .get_flag(internal_params::ENABLE_AZL4), - ); + )?; // Mount all block devices in the newroot mount_points_map(host_config) @@ -464,14 +464,20 @@ impl AclBtrfsCollisionResolution { /// - `enable_azl4` + Kernel ≥6.7: use `mount -o temp_fsid` (mounts the real staging device) /// - Otherwise: bind-mount from active `/usr` (requires verity hash match) /// -/// Returns `None` if no collision exists or if the bind-mount path is unsafe. +/// Returns `Ok(None)` when no collision exists, `Ok(Some(resolution))` when a +/// collision exists and can be safely resolved, and `Err` when a collision +/// exists but no safe resolution is possible (kernel <6.7 and the verity hash +/// is missing, empty, or mismatched). Failing with a structured error preserves +/// the actionable verity context instead of deferring to an opaque mount error. fn resolve_acl_btrfs_uuid_collision( update_volume: AbVolumeSelection, staging_usr_roothash: Option<&str>, enable_azl4: bool, -) -> Option { +) -> Result, TridentError> { // 1. Detect whether a UUID collision exists. - let collision_uuid = detect_acl_btrfs_uuid_collision(update_volume)?; + let Some(collision_uuid) = detect_acl_btrfs_uuid_collision(update_volume) else { + return Ok(None); + }; // 2. Determine resolution strategy based on kernel version. // The temp_fsid path requires the enableAzl4 internal param to be set. @@ -491,7 +497,9 @@ fn resolve_acl_btrfs_uuid_collision( // Verity hash verification is intentionally skipped here: temp_fsid // mounts the real staging device content (not a bind-mount of the // active partition), so there is no identity assumption to verify. - return Some(AclBtrfsCollisionResolution::TempFsid { collision_uuid }); + return Ok(Some(AclBtrfsCollisionResolution::TempFsid { + collision_uuid, + })); } } Ok(None) => { @@ -513,12 +521,21 @@ fn resolve_acl_btrfs_uuid_collision( // 3. Kernel <6.7, unknown kernel, or enableAzl4 not set: bind-mount from // active /usr. This requires verity hash verification to prove content - // is identical. - if !verify_acl_bind_mount_safety(staging_usr_roothash) { - return None; + // is identical. A collision is already known to exist here, so if the + // bind-mount is unsafe we fail with a structured error rather than + // returning None and letting the later mount fail opaquely. + if let Err(reason) = verify_acl_bind_mount_safety(staging_usr_roothash) { + return Err(TridentError::new( + ServicingError::AclBtrfsUuidCollisionUnresolved { + uuid: collision_uuid.to_string(), + reason, + }, + )); } - Some(AclBtrfsCollisionResolution::BindMountActiveUsr { collision_uuid }) + Ok(Some(AclBtrfsCollisionResolution::BindMountActiveUsr { + collision_uuid, + })) } /// Detects a BTRFS filesystem UUID collision on ACL's USR A/B partitions. @@ -542,7 +559,16 @@ fn detect_acl_btrfs_uuid_collision(update_volume: AbVolumeSelection) -> Option dev?, + Ok(Some(dev)) => dev, + Ok(None) => { + warn!( + "lsblk returned no device for '{}' while detecting an ACL BTRFS UUID \ + collision. Treating as no collision; a genuine collision will surface \ + later as a mount failure.", + active_path.display() + ); + return None; + } Err(e) => { warn!( "Failed to query block device '{}' via lsblk while detecting an ACL BTRFS \ @@ -554,7 +580,16 @@ fn detect_acl_btrfs_uuid_collision(update_volume: AbVolumeSelection) -> Option dev?, + Ok(Some(dev)) => dev, + Ok(None) => { + warn!( + "lsblk returned no device for '{}' while detecting an ACL BTRFS UUID \ + collision. Treating as no collision; a genuine collision will surface \ + later as a mount failure.", + update_path.display() + ); + return None; + } Err(e) => { warn!( "Failed to query block device '{}' via lsblk while detecting an ACL BTRFS \ @@ -599,28 +634,25 @@ fn detect_acl_btrfs_uuid_collision(update_volume: AbVolumeSelection) -> Option) -> bool { +/// verity root hashes. Returns `Ok(())` when the active and staging root hashes +/// match, or `Err(reason)` describing why the bind-mount is unsafe. +fn verify_acl_bind_mount_safety(staging_usr_roothash: Option<&str>) -> Result<(), String> { let Some(staging_hash) = staging_usr_roothash else { // No staging verity root hash available. A genuine ACL /usr UUID collision // cannot reach this point without upstream validation // (validate_acl_duplicate_uuid) having already confirmed a staging verity // hash exists, so a missing hash here is anomalous. Fail closed: refuse the // bind-mount rather than mounting the active /usr without cryptographic - // identity proof. The collision then surfaces as an explicit mount failure. - warn!( - "No staging USR verity root hash available for ACL BTRFS UUID collision. \ - Refusing bind-mount to avoid mounting /usr without verity verification." + // identity proof. + return Err( + "no staging USR verity root hash available; refusing bind-mount to avoid \ + mounting /usr without verity verification" + .to_string(), ); - return false; }; let Some(staging) = VerityRootHash::new(staging_hash) else { - warn!( - "Staging USR verity root hash is empty. \ - Refusing bind-mount despite UUID collision." - ); - return false; + return Err("staging USR verity root hash is empty".to_string()); }; match VerityRootHash::from_proc_cmdline() { @@ -631,24 +663,16 @@ fn verify_acl_bind_mount_safety(staging_usr_roothash: Option<&str>) -> bool { partitions have matching root hash ({}...)", staging.preview() ); - true + Ok(()) } else { - warn!( - "Verity root hash mismatch: active USR has '{}...', staging has '{}...'. \ - Refusing bind-mount despite UUID collision.", + Err(format!( + "verity root hash mismatch: active USR has '{}...', staging has '{}...'", active.preview(), staging.preview() - ); - false + )) } } - None => { - warn!( - "Cannot read active USR verity root hash from /proc/cmdline. \ - Refusing bind-mount despite UUID collision." - ); - false - } + None => Err("cannot read active USR verity root hash from /proc/cmdline".to_string()), } } diff --git a/crates/trident_api/src/error.rs b/crates/trident_api/src/error.rs index 70c88e1a16..d85ace2278 100644 --- a/crates/trident_api/src/error.rs +++ b/crates/trident_api/src/error.rs @@ -370,6 +370,12 @@ pub enum ServicingError { expected_device_path: String, }, + #[error( + "ACL A/B update detected a BTRFS filesystem UUID collision on /usr (UUID {uuid}) \ + but could not safely resolve it: {reason}" + )] + AclBtrfsUuidCollisionUnresolved { uuid: String, reason: String }, + #[error("Failed to apply Netplan config")] ApplyNetplanConfig, From d2d9c4cd89d44f4e9ed46b21692929c2c1c8e576 Mon Sep 17 00:00:00 2001 From: bfjelds Date: Tue, 14 Jul 2026 11:06:10 -0700 Subject: [PATCH 10/10] refactor: address review - testable strategy selection and message cleanup - Extract pure select_acl_collision_strategy() and add unit tests for strategy selection (enableAzl4+6.7 => TempFsid; +6.6 / undetermined kernel / flag-off => BindMount) plus verify_acl_bind_mount_safety missing/empty-hash cases. - temp_fsid mount arm: use with_context to avoid eagerly allocating the error string on the success path. - bind-mount arm: use USR_MOUNT_POINT_PATH instead of a hard-coded /usr literal. - Reword the collision warnings to describe temp_fsid availability rather than asserting a specific kernel version, since BindMount is also chosen when the flag is unset or the kernel version is undetermined. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- crates/trident/src/engine/newroot.rs | 146 ++++++++++++++++++++------- 1 file changed, 108 insertions(+), 38 deletions(-) diff --git a/crates/trident/src/engine/newroot.rs b/crates/trident/src/engine/newroot.rs index 111a28be0b..5b6ca0f19e 100644 --- a/crates/trident/src/engine/newroot.rs +++ b/crates/trident/src/engine/newroot.rs @@ -243,7 +243,7 @@ impl NewrootMount { warn!( "Block device '{}' has BTRFS filesystem UUID '{}' which \ collides with the active ACL USR partition. Mounting with \ - temp_fsid option (kernel >=6.7).", + the temp_fsid option.", target_id, collision_uuid, ); mount::mount( @@ -252,7 +252,7 @@ impl NewrootMount { MountFileSystemType::Auto, &options, ) - .context(format!( + .with_context(|| format!( "Failed to mount block device '{}' with temp_fsid \ for ACL BTRFS UUID collision (device path '{}', target '{}')", target_id, @@ -261,11 +261,11 @@ impl NewrootMount { ))?; } AclBtrfsCollisionResolution::BindMountActiveUsr { .. } => { - let active_usr = Path::new("/usr"); + let active_usr = Path::new(USR_MOUNT_POINT_PATH); warn!( "Block device '{}' has BTRFS filesystem UUID '{}' which \ - collides with the active ACL USR partition. Bind-mounting \ - '{}' to '{}' instead (kernel <6.7).", + collides with the active ACL USR partition. temp_fsid is \ + unavailable, so bind-mounting '{}' to '{}' instead.", target_id, collision_uuid, active_usr.display(), @@ -479,28 +479,19 @@ fn resolve_acl_btrfs_uuid_collision( return Ok(None); }; - // 2. Determine resolution strategy based on kernel version. - // The temp_fsid path requires the enableAzl4 internal param to be set. - // When the flag is absent (or the running kernel predates 6.7), skip the - // temp_fsid path and fall through to the verity-verified bind-mount - // strategy below. - if enable_azl4 { + // 2. Determine the resolution strategy. The temp_fsid path requires the + // enableAzl4 internal param AND a running kernel >=6.7; otherwise we fall + // back to the verity-verified bind-mount. Kernel detection (which shells + // out to uname) is only performed when enableAzl4 is set. + let kernel_supports_temp_fsid = if enable_azl4 { match osutils::uname::KernelVersion::running() { Ok(Some(kv)) => { - let supports_temp_fsid = kv >= BTRFS_TEMP_FSID_MIN_KERNEL; + let supported = kv >= BTRFS_TEMP_FSID_MIN_KERNEL; debug!( "Running kernel {}.{}, BTRFS temp_fsid supported: {}", - kv.major, kv.minor, supports_temp_fsid + kv.major, kv.minor, supported ); - if supports_temp_fsid { - // Kernel ≥6.7: mount the staging device directly with temp_fsid. - // Verity hash verification is intentionally skipped here: temp_fsid - // mounts the real staging device content (not a bind-mount of the - // active partition), so there is no identity assumption to verify. - return Ok(Some(AclBtrfsCollisionResolution::TempFsid { - collision_uuid, - })); - } + Some(supported) } Ok(None) => { // uname succeeded but output could not be parsed into major.minor. @@ -508,6 +499,7 @@ fn resolve_acl_btrfs_uuid_collision( "Could not parse kernel version from uname output; \ falling back to bind-mount strategy for ACL BTRFS UUID collision" ); + None } Err(e) => { // uname could not be executed at all (DR-003: distinct from parse failure). @@ -515,27 +507,63 @@ fn resolve_acl_btrfs_uuid_collision( "Failed to execute uname: {e}; cannot determine kernel version, \ falling back to bind-mount strategy for ACL BTRFS UUID collision" ); + None } } - } + } else { + None + }; - // 3. Kernel <6.7, unknown kernel, or enableAzl4 not set: bind-mount from - // active /usr. This requires verity hash verification to prove content - // is identical. A collision is already known to exist here, so if the - // bind-mount is unsafe we fail with a structured error rather than - // returning None and letting the later mount fail opaquely. - if let Err(reason) = verify_acl_bind_mount_safety(staging_usr_roothash) { - return Err(TridentError::new( - ServicingError::AclBtrfsUuidCollisionUnresolved { - uuid: collision_uuid.to_string(), - reason, - }, - )); + match select_acl_collision_strategy(enable_azl4, kernel_supports_temp_fsid) { + // temp_fsid mounts the real staging device content (not a bind-mount of + // the active partition), so there is no identity assumption to verify. + AclCollisionStrategy::TempFsid => Ok(Some(AclBtrfsCollisionResolution::TempFsid { + collision_uuid, + })), + // Bind-mount from active /usr requires verity hash verification to prove + // content is identical. A collision is already known to exist here, so if + // the bind-mount is unsafe we fail with a structured error rather than + // returning None and letting the later mount fail opaquely. + AclCollisionStrategy::BindMount => { + if let Err(reason) = verify_acl_bind_mount_safety(staging_usr_roothash) { + return Err(TridentError::new( + ServicingError::AclBtrfsUuidCollisionUnresolved { + uuid: collision_uuid.to_string(), + reason, + }, + )); + } + Ok(Some(AclBtrfsCollisionResolution::BindMountActiveUsr { + collision_uuid, + })) + } } +} - Ok(Some(AclBtrfsCollisionResolution::BindMountActiveUsr { - collision_uuid, - })) +/// Strategy for resolving an ACL BTRFS `/usr` UUID collision, independent of the +/// concrete colliding UUID. Extracted as a pure function so strategy selection +/// can be unit-tested without touching lsblk, uname, or /proc/cmdline. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum AclCollisionStrategy { + /// Mount the staging device directly with `-o temp_fsid` (kernel >=6.7). + TempFsid, + /// Bind-mount from the active `/usr` (requires verity verification). + BindMount, +} + +/// Selects the collision-resolution strategy. `temp_fsid` is chosen only when the +/// `enableAzl4` internal param is set and the running kernel is known to support +/// it (>=6.7); every other case (flag unset, kernel too old, or kernel version +/// undetermined) falls back to the bind-mount strategy. +fn select_acl_collision_strategy( + enable_azl4: bool, + kernel_supports_temp_fsid: Option, +) -> AclCollisionStrategy { + if enable_azl4 && kernel_supports_temp_fsid == Some(true) { + AclCollisionStrategy::TempFsid + } else { + AclCollisionStrategy::BindMount + } } /// Detects a BTRFS filesystem UUID collision on ACL's USR A/B partitions. @@ -886,6 +914,48 @@ mod tests { error::ErrorKind, }; + #[test] + fn test_select_acl_collision_strategy() { + // enableAzl4 + kernel >=6.7 => temp_fsid. + assert_eq!( + select_acl_collision_strategy(true, Some(true)), + AclCollisionStrategy::TempFsid + ); + // enableAzl4 + kernel <6.7 => bind-mount. + assert_eq!( + select_acl_collision_strategy(true, Some(false)), + AclCollisionStrategy::BindMount + ); + // enableAzl4 + undetermined kernel (uname exec/parse failure) => bind-mount. + assert_eq!( + select_acl_collision_strategy(true, None), + AclCollisionStrategy::BindMount + ); + // enableAzl4 not set => bind-mount regardless of kernel support. + assert_eq!( + select_acl_collision_strategy(false, Some(true)), + AclCollisionStrategy::BindMount + ); + assert_eq!( + select_acl_collision_strategy(false, None), + AclCollisionStrategy::BindMount + ); + } + + #[test] + fn test_verify_acl_bind_mount_safety_missing_hash() { + // No staging hash available => refuse (fail closed) with a reason. + let err = verify_acl_bind_mount_safety(None).unwrap_err(); + assert!(err.contains("no staging USR verity root hash"), "{err}"); + } + + #[test] + fn test_verify_acl_bind_mount_safety_empty_hash() { + // Empty/whitespace staging hash => refuse with a reason. + let err = verify_acl_bind_mount_safety(Some(" ")).unwrap_err(); + assert!(err.contains("empty"), "{err}"); + } + #[test] fn test_mount_point_ordering() { let host_config = HostConfiguration {