From bb575452200c96f37dda8a8ad91962f8c802b041 Mon Sep 17 00:00:00 2001 From: maheeraeron Date: Mon, 19 Oct 2026 19:17:57 +0000 Subject: [PATCH] uefi: support major version 2.0 and consolidated FV for version information --- .../src/worker/vm_loaders/uefi.rs | 60 ++++++-- vm/loader/src/uefi/firmware_version.rs | 139 ++++++++++++++++-- 2 files changed, 172 insertions(+), 27 deletions(-) diff --git a/openvmm/openvmm_core/src/worker/vm_loaders/uefi.rs b/openvmm/openvmm_core/src/worker/vm_loaders/uefi.rs index ddfadd31b8f..b35a1efcb17 100644 --- a/openvmm/openvmm_core/src/worker/vm_loaders/uefi.rs +++ b/openvmm/openvmm_core/src/worker/vm_loaders/uefi.rs @@ -26,12 +26,13 @@ pub enum Error { #[error("invalid UEFI firmware version information")] FirmwareVersion(#[source] loader::uefi::firmware_version::Error), #[error( - "incompatible UEFI firmware interface version {actual_major}.{actual_minor}; expected {expected_major}.{minimum_minor} or newer compatible minor" + "incompatible UEFI firmware interface version {actual_major}.{actual_minor}; expected major {minimum_major} through {maximum_major}, minor {minimum_minor} or newer" )] IncompatibleFirmwareVersion { actual_major: u16, actual_minor: u16, - expected_major: u16, + minimum_major: u16, + maximum_major: u16, minimum_minor: u16, }, #[error("failed to build PCIe ACPI tables")] @@ -76,11 +77,12 @@ pub struct UefiLoadSettings { pub force_firmware_version: bool, } -const FIRMWARE_INTERFACE_MAJOR: u16 = 1; +const FIRMWARE_INTERFACE_MINIMUM_MAJOR: u16 = 1; +const FIRMWARE_INTERFACE_MAXIMUM_MAJOR: u16 = 2; const FIRMWARE_INTERFACE_MINIMUM_MINOR: u16 = 0; fn firmware_interface_is_compatible(major: u16, minor: u16) -> bool { - major == FIRMWARE_INTERFACE_MAJOR + (FIRMWARE_INTERFACE_MINIMUM_MAJOR..=FIRMWARE_INTERFACE_MAXIMUM_MAJOR).contains(&major) && minor .checked_sub(FIRMWARE_INTERFACE_MINIMUM_MINOR) .is_some() @@ -139,8 +141,10 @@ fn check_firmware_version(image: &[u8], force: bool) -> Result<(), Error> { version.interface_version_major, version.interface_version_minor ), required_interface = format_args!( - "{}.{}+", - FIRMWARE_INTERFACE_MAJOR, FIRMWARE_INTERFACE_MINIMUM_MINOR + "major {}..={}, minor {}+", + FIRMWARE_INTERFACE_MINIMUM_MAJOR, + FIRMWARE_INTERFACE_MAXIMUM_MAJOR, + FIRMWARE_INTERFACE_MINIMUM_MINOR ), "UEFI firmware interface is incompatible; continuing because force_firmware_version is set" ); @@ -148,7 +152,8 @@ fn check_firmware_version(image: &[u8], force: bool) -> Result<(), Error> { return Err(Error::IncompatibleFirmwareVersion { actual_major: version.interface_version_major, actual_minor: version.interface_version_minor, - expected_major: FIRMWARE_INTERFACE_MAJOR, + minimum_major: FIRMWARE_INTERFACE_MINIMUM_MAJOR, + maximum_major: FIRMWARE_INTERFACE_MAXIMUM_MAJOR, minimum_minor: FIRMWARE_INTERFACE_MINIMUM_MINOR, }); } @@ -432,7 +437,7 @@ mod tests { let mut image = vec![ 0; if cfg!(guest_arch = "aarch64") { - 0x20_0000 + 0x1000 } else { 0 } @@ -548,20 +553,28 @@ mod tests { assert!(firmware_interface_is_compatible(1, 0)); assert!(firmware_interface_is_compatible(1, 1)); assert!(!firmware_interface_is_compatible(0, 0)); - assert!(!firmware_interface_is_compatible(2, 0)); + assert!(firmware_interface_is_compatible(2, 0)); + assert!(firmware_interface_is_compatible(2, 1)); + assert!(firmware_interface_is_compatible(1, u16::MAX)); + assert!(firmware_interface_is_compatible(2, u16::MAX)); + assert!(!firmware_interface_is_compatible(3, 0)); + assert!(!firmware_interface_is_compatible(u16::MAX, 0)); } #[test] fn firmware_version_validation_policy() { assert!(check_firmware_version(&firmware_image(Some((1, 0))), false).is_ok()); + assert!(check_firmware_version(&firmware_image(Some((2, 0))), false).is_ok()); + assert!(check_firmware_version(&firmware_image(Some((2, 1))), false).is_ok()); - let incompatible = firmware_image(Some((2, 0))); + let incompatible = firmware_image(Some((3, 0))); assert!(matches!( check_firmware_version(&incompatible, false), Err(Error::IncompatibleFirmwareVersion { - actual_major: 2, + actual_major: 3, actual_minor: 0, - expected_major: 1, + minimum_major: 1, + maximum_major: 2, minimum_minor: 0, }) )); @@ -569,7 +582,7 @@ mod tests { let mut malformed = firmware_image(Some((1, 0))); let signature_offset = if cfg!(guest_arch = "aarch64") { - 0x20_0000 + 0x1000 } else { 0 } + size_of::() @@ -589,4 +602,25 @@ mod tests { assert!(check_firmware_version(&missing, false).is_ok()); assert!(check_firmware_version(&missing, true).is_ok()); } + + #[test] + #[cfg(guest_arch = "aarch64")] + fn firmware_version_validation_in_split_layout() { + for (interface, compatible) in [(None, true), (Some((1, 0)), true), (Some((3, 0)), false)] { + let mut image = firmware_image(None); + image.resize(0x200000, 0xff); + image[0x1000 + 32..0x1000 + 40].copy_from_slice(&0x1ff000_u64.to_le_bytes()); + image.extend_from_slice(&firmware_image(interface)[0x1000..]); + let result = check_firmware_version(&image, false); + if compatible { + assert!(result.is_ok(), "{result:?}"); + } else { + assert!(matches!( + result, + Err(Error::IncompatibleFirmwareVersion { .. }) + )); + } + assert!(check_firmware_version(&image, true).is_ok()); + } + } } diff --git a/vm/loader/src/uefi/firmware_version.rs b/vm/loader/src/uefi/firmware_version.rs index c90f8adcdb3..e6935f51f5e 100644 --- a/vm/loader/src/uefi/firmware_version.rs +++ b/vm/loader/src/uefi/firmware_version.rs @@ -13,9 +13,9 @@ const FILE_ALIGNMENT: usize = 8; const SECTION_ALIGNMENT: usize = 4; const ERASED_BYTE: u8 = 0xff; #[cfg(guest_arch = "x86_64")] -const DXE_FIRMWARE_VOLUME_OFFSET: usize = 0; +const FIRST_FIRMWARE_VOLUME_OFFSET: usize = 0; #[cfg(guest_arch = "aarch64")] -const DXE_FIRMWARE_VOLUME_OFFSET: usize = 0x20_0000; +const FIRST_FIRMWARE_VOLUME_OFFSET: usize = 0x1000; /// A parsed view of an MSVM firmware version record. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -187,14 +187,42 @@ fn find_in_firmware_volume(fv: &[u8]) -> Result>, /// `Ok(None)` means the dedicated version FFS file is not present. pub fn find_in_firmware_image(image: &[u8]) -> Result>, Error> { let fv = image - .get(DXE_FIRMWARE_VOLUME_OFFSET..) + .get(FIRST_FIRMWARE_VOLUME_OFFSET..) .ok_or(Error::VolumeHeader)?; - find_in_firmware_volume(fv) + #[cfg(guest_arch = "x86_64")] + return find_in_firmware_volume(fv); + #[cfg(guest_arch = "aarch64")] + { + // Locate the record before interpreting its interface version. Older + // images have separate PEI/DXE volumes; newer ones combine them. + let mut volumes = fv; + loop { + let header = firmware_volume::FirmwareVolumeHeader::read_from_prefix(volumes) + .map_err(|_| Error::VolumeHeader)? + .0; + let length = usize::try_from(header.fv_length) + .ok() + .filter(|&length| { + length >= size_of::() + && length <= volumes.len() + }) + .ok_or(Error::VolumeHeader)?; + let (fv, remaining) = volumes.split_at(length); + if let Some(version) = find_in_firmware_volume(fv)? { + return Ok(Some(version)); + } + if remaining.is_empty() { + return Ok(None); + } + volumes = remaining; + } + } } #[cfg(test)] mod tests { use super::*; + use test_with_tracing::test; use zerocopy::IntoBytes; fn size24(value: usize) -> [u8; 3] { @@ -246,19 +274,20 @@ mod tests { ); } - #[test] - fn finds_record_by_ffs_file_guid() { - let record = record(1, 0); - let section_size = size_of::() + record.len(); - let file_size = size_of::() + section_size; - let file_offset = size_of::().next_multiple_of(8); - let mut fv = [0xff; 0x1000]; - let fv_length = fv.len() as u64; - fv[32..40].copy_from_slice(&fv_length.to_le_bytes()); + fn volume(length: usize, interface: Option<(u16, u16)>) -> Vec { + let mut fv = vec![0xff; length]; + fv[32..40].copy_from_slice(&(length as u64).to_le_bytes()); fv[40..44].copy_from_slice(b"_FVH"); fv[48..50].copy_from_slice( &(size_of::() as u16).to_le_bytes(), ); + let Some((major, minor)) = interface else { + return fv; + }; + let record = record(major, minor); + let section_size = size_of::() + record.len(); + let file_size = size_of::() + section_size; + let file_offset = size_of::().next_multiple_of(8); fv[file_offset..file_offset + 16].copy_from_slice(firmware_version::FILE_GUID.as_bytes()); fv[file_offset + 20..file_offset + 23].copy_from_slice(&size24(file_size)); let section_offset = file_offset + size_of::(); @@ -267,15 +296,97 @@ mod tests { fv[section_offset + size_of::() ..file_offset + file_size] .copy_from_slice(&record); + fv + } + #[test] + fn finds_record_by_ffs_file_guid() { + let fv = volume(0x1000, Some((1, 0))); let version = find_in_firmware_volume(&fv).unwrap().unwrap(); assert_eq!(version.interface_version_major, 1); assert_eq!(version.interface_version_minor, 0); - let mut image = vec![0; DXE_FIRMWARE_VOLUME_OFFSET]; + let mut image = vec![0; FIRST_FIRMWARE_VOLUME_OFFSET]; image.extend_from_slice(&fv); let version = find_in_firmware_image(&image).unwrap().unwrap(); assert_eq!(version.interface_version_major, 1); assert_eq!(version.interface_version_minor, 0); } + + fn image(split: bool, interface: Option<(u16, u16)>) -> Vec { + #[cfg(guest_arch = "x86_64")] + let mut image = Vec::new(); + #[cfg(guest_arch = "aarch64")] + let mut image = vec![0; 0x1000]; + if split { + image.extend(volume(0x1ff000, None)); + } + image.extend(volume(0x600000, interface)); + image + } + + #[test] + fn parses_combined_layout_with_both_interface_versions() { + for major in [1, 2] { + let image = image(false, Some((major, 0))); + let version = find_in_firmware_image(&image).unwrap().unwrap(); + assert_eq!(version.interface_version_major, major); + assert_eq!(version.interface_version_minor, 0); + } + assert_eq!(find_in_firmware_image(&image(false, None)).unwrap(), None); + } + + #[test] + #[cfg(guest_arch = "aarch64")] + fn parses_old_split_layout() { + let image = image(true, Some((1, 0))); + let version = find_in_firmware_image(&image).unwrap().unwrap(); + assert_eq!(version.interface_version_major, 1); + assert_eq!(version.interface_version_minor, 0); + assert_eq!( + find_in_firmware_image(&self::image(true, None)).unwrap(), + None + ); + } + + #[test] + fn rejects_malformed_volumes_and_records() { + #[cfg(guest_arch = "x86_64")] + let layouts = [(false, 0)]; + #[cfg(guest_arch = "aarch64")] + let layouts = [(false, 0x1000), (true, 0x200000)]; + for (split, offset) in layouts { + let original = image(split, Some((2, 0))); + let mut malformed = original.clone(); + malformed[offset + 32..offset + 40].copy_from_slice(&u64::MAX.to_le_bytes()); + assert_eq!(find_in_firmware_image(&malformed), Err(Error::VolumeHeader)); + let mut malformed = original.clone(); + malformed[offset + 40..offset + 44].fill(0); + assert_eq!(find_in_firmware_image(&malformed), Err(Error::VolumeHeader)); + let mut truncated = original.clone(); + truncated.pop(); + assert_eq!(find_in_firmware_image(&truncated), Err(Error::VolumeHeader)); + let mut malformed = original; + let record_offset = offset + + size_of::().next_multiple_of(8) + + size_of::() + + size_of::(); + malformed[record_offset] = 0; + assert_eq!( + find_in_firmware_image(&malformed), + Err(Error::InvalidSignature) + ); + } + assert_eq!(find_in_firmware_image(&[]), Err(Error::VolumeHeader)); + } + + #[test] + #[cfg(guest_arch = "aarch64")] + fn rejects_non_progressing_volume_lengths() { + for length in [0_u64, 1] { + let mut malformed = image(true, Some((1, 0))); + malformed[0x1000 + 32..0x1000 + 40].copy_from_slice(&length.to_le_bytes()); + assert_eq!(find_in_firmware_image(&malformed), Err(Error::VolumeHeader)); + } + } }