From fa7cad4aee6296a6eb13174df79e10abc04a33c3 Mon Sep 17 00:00:00 2001 From: Alexander Lvov Date: Mon, 6 Jul 2026 11:54:44 +0300 Subject: [PATCH] block: vhdx: enable bounds check in sync I/O worker VhdxSync::submit_data_operation() passed every read/write straight to the underlying Vhdx without checking the request against the virtual disk's logical size. A request that started inside the image but extended past its end (or an offset past the end entirely) was passed through unchecked, silently reading/writing out of the intended bounds. Call AsyncIoOperation::validate_bounds() from submit_data_operation() before dispatching the operation, the same way the VHD sync worker does. The check rejects any request whose offset + length exceeds the logical size. Signed-off-by: Alexander Lvov --- block/src/formats/vhdx/mod.rs | 3 +- block/src/formats/vhdx/worker/sync.rs | 101 +++++++++++++++++++++++++- 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/block/src/formats/vhdx/mod.rs b/block/src/formats/vhdx/mod.rs index 139401029..f9d7d580c 100644 --- a/block/src/formats/vhdx/mod.rs +++ b/block/src/formats/vhdx/mod.rs @@ -108,6 +108,7 @@ impl disk_file::AsyncDiskFile for VhdxDisk { } fn create_async_io(&self, _ring_depth: u32) -> BlockResult> { - Ok(Box::new(VhdxSync::new(Arc::clone(&self.vhdx_file)))) + let size = self.vhdx_file.lock().unwrap().virtual_disk_size(); + Ok(Box::new(VhdxSync::new(Arc::clone(&self.vhdx_file), size))) } } diff --git a/block/src/formats/vhdx/worker/sync.rs b/block/src/formats/vhdx/worker/sync.rs index 19a101a83..f75526972 100644 --- a/block/src/formats/vhdx/worker/sync.rs +++ b/block/src/formats/vhdx/worker/sync.rs @@ -17,15 +17,17 @@ pub struct VhdxSync { vhdx_file: Arc>, eventfd: EventFd, completion_list: VecDeque, + size: u64, } impl VhdxSync { - pub fn new(vhdx_file: Arc>) -> Self { + pub fn new(vhdx_file: Arc>, size: u64) -> Self { VhdxSync { vhdx_file, eventfd: EventFd::new(libc::EFD_NONBLOCK) .expect("Failed creating EventFd for VhdxSync"), completion_list: VecDeque::new(), + size, } } @@ -63,6 +65,7 @@ impl AsyncIo for VhdxSync { } fn submit_data_operation(&mut self, op: AsyncIoOperation) -> AsyncIoResult<()> { + op.validate_bounds(self.size)?; let is_read = op.is_read(); let mut op = op; let result = if is_read { @@ -107,3 +110,99 @@ impl AsyncIo for VhdxSync { ))) } } + +#[cfg(test)] +mod tests { + use std::fs; + use std::sync::{Arc, Mutex}; + + use vmm_sys_util::tempfile::TempFile; + + use super::*; + use crate::async_io::{AsyncIo, AsyncIoError, AsyncIoOperation, OwnedIoBuffer}; + use crate::formats::vhdx::internal::Vhdx; + use crate::formats::vhdx::test_util::create_dynamic_vhdx; + + fn make_vhdx_sync(tf: &TempFile) -> (VhdxSync, u64) { + let file = fs::OpenOptions::new() + .read(true) + .write(true) + .open(tf.as_path()) + .unwrap(); + let vhdx = Vhdx::new(file, false).unwrap(); + let size = vhdx.virtual_disk_size(); + let sync = VhdxSync::new(Arc::new(Mutex::new(vhdx)), size); + (sync, size) + } + + /// Builds a `VhdxSync` from a fresh 1 MiB dynamic VHDX, or `None` + /// if `qemu-img` is unavailable to generate one. + fn setup() -> Option<(VhdxSync, u64)> { + let tf = create_dynamic_vhdx(1)?; + Some(make_vhdx_sync(&tf)) + } + + #[test] + fn sync_rejects_read_straddling_logical_size() { + let Some((mut sync, size)) = setup() else { + eprintln!("skipping: qemu-img unavailable"); + return; + }; + + let op = AsyncIoOperation::read_to_vec( + (size - 512) as i64, + OwnedIoBuffer::from_vec(vec![0u8; 1024]), + 1, + ); + assert!(matches!( + sync.submit_data_operation(op), + Err(AsyncIoError::ReadVectored(_)) + )); + } + + #[test] + fn sync_rejects_write_straddling_logical_size() { + let Some((mut sync, size)) = setup() else { + eprintln!("skipping: qemu-img unavailable"); + return; + }; + + let op = AsyncIoOperation::write_from_vec( + (size - 512) as i64, + OwnedIoBuffer::from_vec(vec![0u8; 1024]), + 1, + ); + assert!(matches!( + sync.submit_data_operation(op), + Err(AsyncIoError::WriteVectored(_)) + )); + } + + #[test] + fn sync_accepts_operation_exactly_filling_logical_size() { + let Some((mut sync, size)) = setup() else { + eprintln!("skipping: qemu-img unavailable"); + return; + }; + + let op = + AsyncIoOperation::read_to_vec(0, OwnedIoBuffer::from_vec(vec![0u8; size as usize]), 1); + sync.submit_data_operation(op).unwrap(); + } + + #[test] + fn sync_accepts_operation_at_last_sector() { + let Some((mut sync, size)) = setup() else { + eprintln!("skipping: qemu-img unavailable"); + return; + }; + + // VHDX operates in 512-byte sectors; read exactly the last sector. + let op = AsyncIoOperation::read_to_vec( + (size - 512) as i64, + OwnedIoBuffer::from_vec(vec![0u8; 512]), + 1, + ); + sync.submit_data_operation(op).unwrap(); + } +}