diff --git a/openhcl/underhill_core/src/hibernate.rs b/openhcl/underhill_core/src/hibernate.rs index d8e395e678e..0428f948e7e 100644 --- a/openhcl/underhill_core/src/hibernate.rs +++ b/openhcl/underhill_core/src/hibernate.rs @@ -1,11 +1,15 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -//! OpenHCL hibernate token handling: the [`Token`] recorded in -//! [`vmgs::FileId::HIBERNATION_TOKEN`] and helpers to read, write, and delete -//! it. +//! OpenHCL hibernate state stored in VMGS: the [`Token`] recorded in +//! [`vmgs::FileId::HIBERNATION_TOKEN`] and the UEFI firmware image snapshot in +//! [`vmgs::FileId::HIBERNATION_FIRMWARE`], with helpers to read, write, and +//! delete both. +use anyhow::Context; use cvm_tracing::CVM_ALLOWED; +use guestmem::GuestMemory; +use memory_range::MemoryRange; use std::fmt; use vmgs_broker::VmgsBrokerError; use vmgs_broker::VmgsClientError; @@ -154,6 +158,149 @@ pub async fn read_token(vmgs_client: &vmgs_broker::VmgsClient) -> Option } } +/// The minimum overall VMGS backing-store size, in bytes, required before a UEFI +/// firmware image snapshot ([`vmgs::FileId::HIBERNATION_FIRMWARE`]) is stored +/// for hibernation. A minimum overall size so the image fits alongside the other +/// VMGS files, not just a tight fit of the image itself. +pub const VMGS_HIBERNATION_FIRMWARE_MIN_SIZE: u64 = 32 * 1024 * 1024; + +/// The result of attempting to store a firmware image into VMGS for hibernation. +pub enum StoreFirmwareOutcome { + /// The firmware image was stored to VMGS. + Stored, + /// The VMGS backing store is too small to hold the firmware image. This is + /// not an error: hibernation falls back to not preserving the firmware + /// image across resume. + InsufficientSpace { + firmware_size: u64, + device_size: u64, + }, +} + +/// Stores the pristine UEFI firmware image out of VTL0 guest memory and into +/// [`vmgs::FileId::HIBERNATION_FIRMWARE`] so it can be restored identically on a +/// hibernation resume where the host cannot overload the firmware itself. +/// +/// This must be called *before* any dynamic configuration is written into the +/// firmware region (i.e. before `write_uefi_config`), so that the stored image +/// matches the measured firmware as loaded from the IGVM file. +/// +/// The image is only stored if the VMGS backing store is large enough to hold +/// it; otherwise [`StoreFirmwareOutcome::InsufficientSpace`] is returned and the +/// caller falls back to not preserving the firmware across hibernation. An +/// `Err` is only returned for an actual failure to query or write VMGS. +pub async fn store_firmware( + gm: &GuestMemory, + firmware_memory: MemoryRange, + vmgs_client: &vmgs_broker::VmgsClient, +) -> anyhow::Result { + let len = firmware_memory.len(); + + // The firmware image can only be stored if the VMGS backing store is large + // enough. Require a minimum overall size so there is room for the firmware + // image alongside the other VMGS files, and also verify the image fits. + // Insufficient space is a normal fallback, not an error. + let device_size = vmgs_client + .device_size() + .await + .context("failed to query VMGS size")?; + if device_size < VMGS_HIBERNATION_FIRMWARE_MIN_SIZE || len > device_size { + return Ok(StoreFirmwareOutcome::InsufficientSpace { + firmware_size: len, + device_size, + }); + } + + let firmware_len = usize::try_from(len).context("firmware image size does not fit in usize")?; + let mut firmware = vec![0u8; firmware_len]; + gm.read_at(firmware_memory.start(), &mut firmware) + .context("failed to read UEFI firmware image from VTL0 memory")?; + vmgs_client + .write_file(vmgs::FileId::HIBERNATION_FIRMWARE, firmware) + .await + .context("failed to write UEFI firmware snapshot to VMGS")?; + tracing::info!( + CVM_ALLOWED, + size_bytes = len, + "stored UEFI firmware image to VMGS for hibernation" + ); + Ok(StoreFirmwareOutcome::Stored) +} + +/// Reads a stored UEFI firmware image from VMGS, returning `None` if no image is +/// stored (the usual case, e.g. first hibernation or a VMGS too small to hold +/// one) or if the stored image's size is not exactly `expected_len`. The size is +/// checked via the file metadata *before* reading, so a corrupt or oversized +/// entry cannot force a large allocation. +pub async fn read_firmware( + vmgs_client: &vmgs_broker::VmgsClient, + expected_len: u64, +) -> Option> { + let info = match vmgs_client + .get_file_info(vmgs::FileId::HIBERNATION_FIRMWARE) + .await + { + Ok(info) => info, + // No stored image is the usual case; not an error. + Err(VmgsClientError::Vmgs(VmgsBrokerError::FileInfoNotAllocated)) => return None, + Err(err) => { + tracing::warn!( + CVM_ALLOWED, + error = &err as &dyn std::error::Error, + "failed to query stored UEFI firmware image metadata" + ); + return None; + } + }; + + // A resumed image is only usable if it is bit-for-bit the same size as the + // firmware region; requiring an exact match here also bounds the allocation + // `read_file` makes below. + if info.valid_bytes != expected_len { + tracing::warn!( + CVM_ALLOWED, + stored_size = info.valid_bytes, + expected_size = expected_len, + "stored UEFI firmware image size does not match the firmware region; not restoring" + ); + return None; + } + + match vmgs_client + .read_file(vmgs::FileId::HIBERNATION_FIRMWARE) + .await + { + Ok(firmware) => Some(firmware), + Err(err) => { + tracing::warn!( + CVM_ALLOWED, + error = &err as &dyn std::error::Error, + "failed to read UEFI firmware image from VMGS on hibernation resume" + ); + None + } + } +} + +/// Best-effort deletion of any stored hibernation firmware image, used on a +/// clean power off / reset so a later boot does not restore a stale image. A +/// missing image is the common case and is not logged. +pub async fn delete_firmware(vmgs_client: &vmgs_broker::VmgsClient) { + match vmgs_client + .delete_file(vmgs::FileId::HIBERNATION_FIRMWARE) + .await + { + Ok(()) | Err(VmgsClientError::Vmgs(VmgsBrokerError::FileInfoNotAllocated)) => {} + Err(err) => { + tracing::error!( + CVM_ALLOWED, + error = &err as &dyn std::error::Error, + "failed to delete stored UEFI firmware image from VMGS" + ); + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -193,6 +340,50 @@ mod tests { assert_eq!(read_token(&client).await, Some(Token::NotHibernated)); } + #[async_test] + async fn firmware_store_read_delete(driver: DefaultDriver) { + // The store is gated on VMGS_HIBERNATION_FIRMWARE_MIN_SIZE, so use a + // backing store larger than that minimum. + let disk = ram_disk(48 * 1024 * 1024, false).unwrap(); + let vmgs = Vmgs::format_new(disk, None).await.unwrap(); + let (client, _task) = spawn_vmgs_broker(driver.clone(), vmgs); + + // No image stored initially. + assert!(read_firmware(&client, 0x1000).await.is_none()); + + let gm = GuestMemory::allocate(0x2000); + let firmware_memory = MemoryRange::new(0x1000..0x2000); + let image: Vec = (0..0x1000u32).map(|i| i as u8).collect(); + gm.write_at(firmware_memory.start(), &image).unwrap(); + + assert!(matches!( + store_firmware(&gm, firmware_memory, &client).await.unwrap(), + StoreFirmwareOutcome::Stored + )); + assert_eq!( + read_firmware(&client, 0x1000).await.as_deref(), + Some(image.as_slice()) + ); + // A size mismatch is rejected without loading. + assert!(read_firmware(&client, 0x800).await.is_none()); + + delete_firmware(&client).await; + assert!(read_firmware(&client, 0x1000).await.is_none()); + } + + #[async_test] + async fn firmware_store_insufficient_space(driver: DefaultDriver) { + // `new_client` uses a 4 MB disk, below VMGS_HIBERNATION_FIRMWARE_MIN_SIZE. + let (client, _task) = new_client(&driver).await; + let gm = GuestMemory::allocate(0x1000); + let firmware_memory = MemoryRange::new(0..0x1000); + assert!(matches!( + store_firmware(&gm, firmware_memory, &client).await.unwrap(), + StoreFirmwareOutcome::InsufficientSpace { .. } + )); + assert!(read_firmware(&client, 0x1000).await.is_none()); + } + #[test] fn constants_encode_as_expected() { assert_eq!(u64::from(Token::NotHibernated), 0); diff --git a/openhcl/underhill_core/src/worker.rs b/openhcl/underhill_core/src/worker.rs index f3fcbb1a46e..61766af5dfd 100644 --- a/openhcl/underhill_core/src/worker.rs +++ b/openhcl/underhill_core/src/worker.rs @@ -3701,6 +3701,10 @@ async fn new_underhill_vm( // firmware is overloaded (via the GET `LoadFirmware` host request) with the // hibernated version before it runs; on success that version becomes the // recorded token. + // + // Set when a hibernated firmware image is restored from VMGS on resume, so + // the cold-boot snapshot below can skip re-storing an image already in VMGS. + let mut firmware_loaded_from_vmgs = false; let current_hibernate_token = if !dps.general.hibernation_enabled || !matches!(firmware_type, FirmwareType::Uefi) { @@ -3730,66 +3734,88 @@ async fn new_underhill_vm( // Consume any token, valid or corrupt, so it is not re-read next boot. hibernate::delete_token(vmgs_client).await; match resume_token { - Some(token @ hibernate::Token::Hibernated { .. }) - if token != hibernate::Token::CURRENT => - { - if !dps - .general - .management_vtl_features - .load_firmware_supported() + // Clean prior power-off (NotHibernated) or no/unreadable token + // (None): a normal cold boot on the current firmware; no load needed. + Some(hibernate::Token::NotHibernated) | None => Some(hibernate::Token::CURRENT), + Some(token) => { + // Prefer restoring the exact firmware image snapshotted to VMGS + // at the original cold boot: it reproduces the hibernated + // firmware bit-for-bit without needing host support. Only if no + // usable image is stored do we fall back to the version-based + // logic (host overload, then the current firmware). + if restore_vtl0_firmware_from_vmgs(gm.vtl0(), &mut measured_vtl0_info, vmgs_client) + .await { - // The host must advertise LoadFirmware support; without it - // we can't overload, so resume on the current firmware. - tracing::warn!( - CVM_ALLOWED, - resume = %token, - "host does not support firmware overload (LoadFirmware); \ - resuming with the current firmware version despite a \ - hibernation token mismatch" - ); - Some(hibernate::Token::CURRENT) - } else { + firmware_loaded_from_vmgs = true; tracing::info!( CVM_ALLOWED, resume = %token, - current = %hibernate::Token::CURRENT, - "hibernation resume under a different firmware version; requesting firmware overload" + "restored the hibernated UEFI firmware image from VMGS" ); - // If the overload succeeds VTL0 now runs the hibernated - // version, so record it; otherwise fall back to the current one. - if overload_vtl0_firmware( - &get_client, - u64::from(token), - &mut measured_vtl0_info, - ) - .await - { - Some(token) - } else { - Some(hibernate::Token::CURRENT) + Some(token) + } else { + match token { + token @ hibernate::Token::Hibernated { .. } + if token != hibernate::Token::CURRENT => + { + if dps + .general + .management_vtl_features + .load_firmware_supported() + { + tracing::info!( + CVM_ALLOWED, + resume = %token, + current = %hibernate::Token::CURRENT, + "hibernation resume under a different firmware version; requesting firmware overload" + ); + // If the overload succeeds VTL0 now runs the + // hibernated version, so record it; otherwise fall + // back to the current one. + if overload_vtl0_firmware( + &get_client, + u64::from(token), + &mut measured_vtl0_info, + ) + .await + { + Some(token) + } else { + Some(hibernate::Token::CURRENT) + } + } else { + tracing::warn!( + CVM_ALLOWED, + resume = %token, + "no stored firmware image and host does not support firmware \ + overload (LoadFirmware); resuming with the current firmware \ + version despite a hibernation token mismatch" + ); + Some(hibernate::Token::CURRENT) + } + } + hibernate::Token::Hibernated { .. } => { + // The token matches the current firmware version. + tracing::info!( + CVM_ALLOWED, + "hibernation resume under the current firmware version" + ); + Some(hibernate::Token::CURRENT) + } + hibernate::Token::Other(raw) => { + // An out-of-range/corrupt token value; ignore it. + tracing::warn!( + CVM_ALLOWED, + raw, + "ignoring unrecognized hibernate token; using the current firmware version" + ); + Some(hibernate::Token::CURRENT) + } + // NotHibernated is handled by the outer arm. + hibernate::Token::NotHibernated => Some(hibernate::Token::CURRENT), } } } - Some(hibernate::Token::Hibernated { .. }) => { - // Guarded above: the token matches the current firmware version. - tracing::info!( - CVM_ALLOWED, - "hibernation resume under the current firmware version" - ); - Some(hibernate::Token::CURRENT) - } - Some(hibernate::Token::Other(raw)) => { - // An out-of-range/corrupt token value; ignore it. - tracing::warn!( - CVM_ALLOWED, - raw, - "ignoring unrecognized hibernate token; using the current firmware version" - ); - Some(hibernate::Token::CURRENT) - } - // Clean prior power-off, or no/unreadable token (e.g. first boot): - // a normal cold boot on the current firmware. - Some(hibernate::Token::NotHibernated) | None => Some(hibernate::Token::CURRENT), } } else { // Hibernation was requested but there is no VMGS to persist the token, @@ -3801,6 +3827,48 @@ async fn new_underhill_vm( None }; + // Snapshot the pristine UEFI firmware image now present in VTL0 memory into + // VMGS so a future hibernation resume can restore it bit-for-bit on a host + // that cannot overload the firmware itself. This runs after any firmware + // overload/restore above (so it captures the image the guest will actually + // run) and before `write_uefi_config` (in `load_firmware`) layers the + // dynamic config on top, regardless of the resolved token. Skipped for a + // servicing restore (its firmware already has dynamic config applied and is + // no longer pristine) and when the image was just restored from VMGS (it is + // already saved there). + if dps.general.hibernation_enabled && !is_restoring && !firmware_loaded_from_vmgs { + if let (Some(uefi_info), Some(vmgs_client)) = ( + measured_vtl0_info + .as_ref() + .and_then(|info| info.supports_uefi.as_ref()), + vmgs_client.as_ref(), + ) { + match hibernate::store_firmware(gm.vtl0(), uefi_info.firmware_memory, vmgs_client).await + { + Ok(hibernate::StoreFirmwareOutcome::Stored) => {} + Ok(hibernate::StoreFirmwareOutcome::InsufficientSpace { + firmware_size, + device_size, + }) => { + tracing::info!( + CVM_ALLOWED, + firmware_size, + device_size, + minimum_size = hibernate::VMGS_HIBERNATION_FIRMWARE_MIN_SIZE, + "VMGS backing store too small to preserve UEFI firmware across hibernation" + ); + } + Err(err) => { + tracing::error!( + CVM_ALLOWED, + error = err.as_ref() as &dyn std::error::Error, + "failed to store UEFI firmware image for hibernation" + ); + } + } + } + } + // A Some token means hibernation is enabled and populated; pair it with a // VMGS client to drive token persistence at halt time. let hibernate_halt = @@ -4131,6 +4199,9 @@ async fn halt_task( hibernate::Token::NotHibernated, ) .await; + // Drop any stored firmware image so a later boot does not + // restore a stale one. + hibernate::delete_firmware(&hibernate_halt.vmgs_client).await; } get_client.send_power_off() } @@ -4142,6 +4213,9 @@ async fn halt_task( hibernate::Token::NotHibernated, ) .await; + // Drop any stored firmware image so a later boot does not + // restore a stale one. + hibernate::delete_firmware(&hibernate_halt.vmgs_client).await; } get_client.send_reset() } @@ -4175,6 +4249,112 @@ async fn wait_for_flush_logs(control_send: &Arc, + vmgs_client: &vmgs_broker::VmgsClient, +) -> bool { + let Some(firmware_memory) = measured_vtl0_info + .as_ref() + .and_then(|info| info.supports_uefi.as_ref()) + .map(|uefi_info| uefi_info.firmware_memory) + else { + // Not a UEFI boot; nothing to restore. + return false; + }; + + // `read_firmware` requires the stored image to be exactly `region_len`, so + // no separate size check is needed here. + let region_len = firmware_memory.len(); + let Some(firmware) = hibernate::read_firmware(vmgs_client, region_len).await else { + // No usable stored image; leave the current firmware in place. + return false; + }; + + // On x86_64, compute the restored image's SEC entry point before touching + // guest memory so a malformed image is a clean fallback (nothing written). + // The image is untrusted (may be corrupt): the entry point must land inside + // the firmware region and must not overflow. + #[cfg(guest_arch = "x86_64")] + let new_rip = { + let Some(new_rip) = + loader::uefi::get_sec_entry_point_offset(&firmware).and_then(|offset| { + firmware_memory + .start() + .checked_add(offset) + .filter(|_| offset < region_len) + }) + else { + tracing::warn!( + CVM_ALLOWED, + "stored UEFI firmware image has no valid SEC entry point; not restoring" + ); + return false; + }; + new_rip + }; + + if let Err(err) = gm.write_at(firmware_memory.start(), &firmware) { + tracing::error!( + CVM_ALLOWED, + error = &err as &dyn std::error::Error, + "failed to write restored UEFI firmware image into VTL0 memory" + ); + return false; + } + + // Rebase VTL0's RIP to the restored image's entry point; `load_firmware` + // applies the updated measured UEFI context to VTL0. + #[cfg(guest_arch = "x86_64")] + set_vtl0_uefi_rip(measured_vtl0_info, new_rip); + + tracing::info!( + CVM_ALLOWED, + size_bytes = region_len, + "restored UEFI firmware image from VMGS for hibernation resume" + ); + true +} + +/// Rebase VTL0's initial RIP in the measured UEFI context to `new_rip`, so +/// `load_firmware` starts VTL0 at the entry point of a firmware image swapped in +/// at runtime (host overload or VMGS restore). A no-op if there is no measured +/// UEFI context or RIP register. +#[cfg(guest_arch = "x86_64")] +fn set_vtl0_uefi_rip(measured_vtl0_info: &mut Option, new_rip: u64) { + let Some(uefi_info) = measured_vtl0_info + .as_mut() + .and_then(|info| info.supports_uefi.as_mut()) + else { + return; + }; + let crate::loader::VpContext::Vbs(registers) = &mut uefi_info.vp_context; + for reg in registers { + if let loader::importer::X86Register::Rip(rip) = reg { + if *rip != new_rip { + tracing::info!( + CVM_ALLOWED, + old_rip = *rip, + new_rip, + "rebased VTL0 RIP for hibernation firmware swap" + ); + *rip = new_rip; + } + } + } +} + /// Ask the host to overwrite VTL0's firmware image in guest RAM with the version /// identified by `token` (used on a hibernation resume under a different firmware /// version). On success, updates the measured UEFI context so VTL0 starts at the @@ -4211,13 +4391,17 @@ async fn overload_vtl0_firmware( // so point the measured UEFI context's RIP at the host-computed offset; // load_firmware later applies it to VTL0. #[cfg(guest_arch = "x86_64")] - if let Some(uefi_info) = measured_vtl0_info - .as_mut() - .and_then(|info| info.supports_uefi.as_mut()) - { - if offset != 0 { - let base = uefi_info.firmware_memory.start(); - let len = uefi_info.firmware_memory.len(); + if offset != 0 { + if let Some((base, len)) = measured_vtl0_info + .as_ref() + .and_then(|info| info.supports_uefi.as_ref()) + .map(|uefi_info| { + ( + uefi_info.firmware_memory.start(), + uefi_info.firmware_memory.len(), + ) + }) + { // `offset` is host-provided (untrusted): the entry point must land // inside the measured firmware region and must not overflow. let Some(new_rip) = base.checked_add(offset).filter(|_| offset < len) else { @@ -4230,20 +4414,7 @@ async fn overload_vtl0_firmware( ); return false; }; - let crate::loader::VpContext::Vbs(registers) = &mut uefi_info.vp_context; - for reg in registers { - if let loader::importer::X86Register::Rip(rip) = reg { - if *rip != new_rip { - tracing::info!( - CVM_ALLOWED, - old_rip = *rip, - new_rip, - "rebased VTL0 RIP for overloaded firmware" - ); - *rip = new_rip; - } - } - } + set_vtl0_uefi_rip(measured_vtl0_info, new_rip); } } #[cfg(guest_arch = "aarch64")] diff --git a/vm/loader/src/uefi/mod.rs b/vm/loader/src/uefi/mod.rs index 0c66b3682d6..73c3cc1b4ae 100644 --- a/vm/loader/src/uefi/mod.rs +++ b/vm/loader/src/uefi/mod.rs @@ -150,27 +150,25 @@ fn pe_get_entry_point_offset(pe32_data: &[u8]) -> Option { let dos_header = ImageDosHeader::read_from_prefix(pe32_data).ok()?.0; // TODO: zerocopy: use-rest-of-range, option-to-error (https://github.com/microsoft/openvmm/issues/759) let nt_headers_offset = if dos_header.e_magic == IMAGE_DOS_SIGNATURE { // DOS image header is present, so read the PE header after the DOS image header. - dos_header.e_lfanew as usize + usize::try_from(dos_header.e_lfanew).ok()? } else { // DOS image header is not present, so PE header is at the image base. 0 }; - let signature = u32::read_from_prefix(&pe32_data[nt_headers_offset..]) - .ok()? - .0; // TODO: zerocopy: use-rest-of-range, option-to-error (https://github.com/microsoft/openvmm/issues/759) + // Bounds-checked: the image may be corrupt/untrusted (e.g. restored from VMGS). + let nt_headers = pe32_data.get(nt_headers_offset..)?; + let signature = u32::read_from_prefix(nt_headers).ok()?.0; // TODO: zerocopy: use-rest-of-range, option-to-error (https://github.com/microsoft/openvmm/issues/759) // Calculate the entry point relative to the start of the image. // AddressOfEntryPoint is common for PE32 & PE32+ if signature as u16 == TE_IMAGE_HEADER_SIGNATURE { - let te = TeImageHeader::read_from_prefix(&pe32_data[nt_headers_offset..]) - .ok()? - .0; // TODO: zerocopy: use-rest-of-range, option-to-error (https://github.com/microsoft/openvmm/issues/759) - Some(te.address_of_entry_point + size_of_val(&te) as u32 - te.stripped_size as u32) + let te = TeImageHeader::read_from_prefix(nt_headers).ok()?.0; // TODO: zerocopy: use-rest-of-range, option-to-error (https://github.com/microsoft/openvmm/issues/759) + te.address_of_entry_point + .checked_add(size_of_val(&te) as u32)? + .checked_sub(te.stripped_size as u32) } else if signature == IMAGE_NT_SIGNATURE { - let pe = ImageNtHeaders32::read_from_prefix(&pe32_data[nt_headers_offset..]) - .ok()? - .0; // TODO: zerocopy: use-rest-of-range, option-to-error (https://github.com/microsoft/openvmm/issues/759) + let pe = ImageNtHeaders32::read_from_prefix(nt_headers).ok()?.0; // TODO: zerocopy: use-rest-of-range, option-to-error (https://github.com/microsoft/openvmm/issues/759) Some(pe.optional_header.address_of_entry_point) } else { None @@ -215,12 +213,18 @@ struct EFI_COMMON_SECTION_HEADER { const EFI_SECTION_PE32: u8 = 0x10; /// Get the SEC entry point offset from the firmware base. -fn get_sec_entry_point_offset(image: &[u8]) -> Option { +/// +/// On x86_64 the initial VTL0 RIP is the firmware base plus this offset. It is +/// exposed so callers that swap the firmware image at runtime (e.g. restoring a +/// hibernated firmware image) can recompute the entry point from the new image. +/// Returns `None` if the image has no recognizable SEC firmware volume / entry +/// point. +pub fn get_sec_entry_point_offset(image: &[u8]) -> Option { // Skip to SEC volume start. let mut image_offset = SEC_FIRMWARE_VOLUME_OFFSET; // Expect a firmware volume header for SEC volume. - let fvh = EFI_FIRMWARE_VOLUME_HEADER::read_from_prefix(&image[image_offset as usize..]) + let fvh = EFI_FIRMWARE_VOLUME_HEADER::read_from_prefix(image.get(image_offset as usize..)?) .ok()? .0; // TODO: zerocopy: use-rest-of-range, option-to-error (https://github.com/microsoft/openvmm/issues/759) if fvh.signature != EFI_FVH_SIGNATURE { @@ -228,18 +232,17 @@ fn get_sec_entry_point_offset(image: &[u8]) -> Option { } // Skip past firmware volume header to beginning of firmware volume. - image_offset += fvh.header_length as u64; + image_offset = image_offset.checked_add(fvh.header_length as u64)?; // Find the first SEC CORE file type. let mut sec_core_file_header = None; let mut volume_offset = 0; while volume_offset < fvh.fv_length { - let new_volume_offset = (volume_offset + 7) & !7; - if new_volume_offset > volume_offset { - image_offset += new_volume_offset - volume_offset; - volume_offset = new_volume_offset; - } - let fh = EFI_FFS_FILE_HEADER::read_from_prefix(&image[image_offset as usize..]) + let new_volume_offset = volume_offset.checked_add(7)? & !7; + image_offset = image_offset.checked_add(new_volume_offset - volume_offset)?; + volume_offset = new_volume_offset; + + let fh = EFI_FFS_FILE_HEADER::read_from_prefix(image.get(image_offset as usize..)?) .ok()? .0; // TODO: zerocopy: use-rest-of-range, option-to-error (https://github.com/microsoft/openvmm/issues/759) if fh.typ == EFI_FV_FILETYPE_SECURITY_CORE { @@ -247,45 +250,62 @@ fn get_sec_entry_point_offset(image: &[u8]) -> Option { break; } - image_offset += expand_3byte_integer(fh.size); - volume_offset += expand_3byte_integer(fh.size); + // A zero-size file would not advance the scan; treat as malformed. + let file_size = expand_3byte_integer(fh.size); + if file_size == 0 { + return None; + } + image_offset = image_offset.checked_add(file_size)?; + volume_offset = volume_offset.checked_add(file_size)?; } // There should always be a Security Core file. let sec_core_file_header = sec_core_file_header?; let sec_core_file_size = expand_3byte_integer(sec_core_file_header.size); + // A zero-size SEC file has no sections; reject it rather than returning an + // offset that is not an entry point. + if sec_core_file_size == 0 { + return None; + } // Move past the firmware file header. - image_offset += size_of::() as u64; - volume_offset += size_of::() as u64; + let file_header_size = size_of::() as u64; + image_offset = image_offset.checked_add(file_header_size)?; + volume_offset = volume_offset.checked_add(file_header_size)?; - // Loop through the firmware file sections looking for PE section. + // Loop through the firmware file sections looking for the PE section, which + // holds the entry point. let mut file_offset = volume_offset; + let mut pe_entry = None; while file_offset < sec_core_file_size { // // Section headers are 8 byte aligned with respect to the beginning of the file stream. // - let new_file_offset = (file_offset + 3) & !3; - if new_file_offset > file_offset { - image_offset += new_file_offset - file_offset; - file_offset += new_file_offset - file_offset; - } + let new_file_offset = file_offset.checked_add(3)? & !3; + image_offset = image_offset.checked_add(new_file_offset - file_offset)?; + file_offset = new_file_offset; - let sh = EFI_COMMON_SECTION_HEADER::read_from_prefix(&image[image_offset as usize..]) + let sh = EFI_COMMON_SECTION_HEADER::read_from_prefix(image.get(image_offset as usize..)?) .ok()? .0; // TODO: zerocopy: use-rest-of-range, option-to-error (https://github.com/microsoft/openvmm/issues/759) if sh.typ == EFI_SECTION_PE32 { - let pe_offset = pe_get_entry_point_offset( - &image[image_offset as usize + size_of::()..], - )?; - image_offset += size_of::() as u64 + pe_offset as u64; + let section_header_size = size_of::() as u64; + let pe_data_offset = image_offset.checked_add(section_header_size)?; + let pe_offset = pe_get_entry_point_offset(image.get(pe_data_offset as usize..)?)?; + pe_entry = Some(pe_data_offset.checked_add(pe_offset as u64)?); break; } - image_offset += expand_3byte_integer(sh.size); - file_offset += expand_3byte_integer(sh.size); + // A zero-size section would not advance the scan; treat as malformed. + let section_size = expand_3byte_integer(sh.size); + if section_size == 0 { + return None; + } + image_offset = image_offset.checked_add(section_size)?; + file_offset = file_offset.checked_add(section_size)?; } - Some(image_offset) + // No PE section means no entry point. + pe_entry } /// Definitions shared by UEFI and the loader when loaded with parameters passed in IGVM format. diff --git a/vm/vmgs/vmgs/src/storage.rs b/vm/vmgs/vmgs/src/storage.rs index 8f252a2e9cd..71720502631 100644 --- a/vm/vmgs/vmgs/src/storage.rs +++ b/vm/vmgs/vmgs/src/storage.rs @@ -160,8 +160,10 @@ impl VmgsStorage { self.disk.sector_count() } - fn capacity(&self) -> u64 { - (self.sector_count() * self.sector_size() as u64).min(vmgs_format::VMGS_MAX_CAPACITY_BYTES) + pub fn capacity(&self) -> u64 { + self.sector_count() + .saturating_mul(self.sector_size() as u64) + .min(vmgs_format::VMGS_MAX_CAPACITY_BYTES) } /// Capacity in VMGS blocks. diff --git a/vm/vmgs/vmgs/src/vmgs_impl.rs b/vm/vmgs/vmgs/src/vmgs_impl.rs index 9a4e7692bd9..3bb1a5bc93f 100644 --- a/vm/vmgs/vmgs/src/vmgs_impl.rs +++ b/vm/vmgs/vmgs/src/vmgs_impl.rs @@ -641,6 +641,12 @@ impl Vmgs { Ok(vmgs) } + /// Returns the total size, in bytes, of the underlying VMGS backing store, + /// clamped to the maximum VMGS capacity. + pub fn device_size(&self) -> u64 { + self.storage.capacity() + } + /// Get allocated and valid bytes from File Control Block for file_id. /// /// When reading data from a file, the buffer must be at least `valid_bytes` long. diff --git a/vm/vmgs/vmgs_broker/src/broker.rs b/vm/vmgs/vmgs_broker/src/broker.rs index 40ebb9a531a..cd9b0bf00b8 100644 --- a/vm/vmgs/vmgs_broker/src/broker.rs +++ b/vm/vmgs/vmgs_broker/src/broker.rs @@ -56,6 +56,9 @@ pub enum VmgsBrokerRpc { WriteFileEncrypted(Rpc<(BrokerFileId, Vec), Result<(), VmgsBrokerError>>), Save(Rpc<(), vmgs::save_restore::state::SavedVmgsState>), DeleteFile(Rpc>), + // N.B. `MeshPayload` numbers fields by declaration order; append new RPCs + // here so existing ones keep their wire numbers across revisions. + DeviceSize(Rpc<(), u64>), } pub struct VmgsBrokerTask { @@ -84,6 +87,7 @@ impl VmgsBrokerTask { } VmgsBrokerRpc::GetFileInfo(rpc) => rpc .handle_sync(|file_id| self.vmgs.get_file_info(file_id.into()).map_err(Into::into)), + VmgsBrokerRpc::DeviceSize(rpc) => rpc.handle_sync(|()| self.vmgs.device_size()), VmgsBrokerRpc::ReadFile(rpc) => { rpc.handle(async |file_id| { self.vmgs diff --git a/vm/vmgs/vmgs_broker/src/client.rs b/vm/vmgs/vmgs_broker/src/client.rs index 42feeb4f41e..0b1d583c781 100644 --- a/vm/vmgs/vmgs_broker/src/client.rs +++ b/vm/vmgs/vmgs_broker/src/client.rs @@ -63,6 +63,14 @@ impl VmgsClient { Ok(res) } + /// Returns the total size, in bytes, of the underlying VMGS backing store. + #[instrument(skip_all)] + pub async fn device_size(&self) -> Result { + let res = self.control.call(VmgsBrokerRpc::DeviceSize, ()).await?; + + Ok(res) + } + /// Reads the specified `file_id`. #[instrument(skip_all, fields(file_id = %file_id))] pub async fn read_file(&self, file_id: FileId) -> Result, VmgsClientError> { diff --git a/vm/vmgs/vmgs_format/src/lib.rs b/vm/vmgs/vmgs_format/src/lib.rs index 837bec09b99..e8472f2b31f 100644 --- a/vm/vmgs/vmgs_format/src/lib.rs +++ b/vm/vmgs/vmgs_format/src/lib.rs @@ -58,6 +58,7 @@ open_enum! { PROVENANCE_DOC = 16, TPM_NVRAM_BACKUP = 17, PROVISIONING_MARKER = 18, + HIBERNATION_FIRMWARE = 20, EXTENDED_FILE_TABLE = 63, } diff --git a/vm/vmgs/vmgstool/src/main.rs b/vm/vmgs/vmgstool/src/main.rs index 3661dd3b2ef..954111fee4d 100644 --- a/vm/vmgs/vmgstool/src/main.rs +++ b/vm/vmgs/vmgstool/src/main.rs @@ -373,6 +373,7 @@ fn parse_file_id(file_id: &str) -> Result { "PLATFORM_SEED" => FileId::PLATFORM_SEED, "PROVENANCE_DOC" => FileId::PROVENANCE_DOC, "TPM_NVRAM_BACKUP" => FileId::TPM_NVRAM_BACKUP, + "HIBERNATION_FIRMWARE" => FileId::HIBERNATION_FIRMWARE, "EXTENDED_FILE_TABLE" => FileId::EXTENDED_FILE_TABLE, v => FileId(v.parse::()?), })