diff --git a/block/src/formats/vhd/mod.rs b/block/src/formats/vhd/mod.rs index 52cf6772b..a442a2fc0 100644 --- a/block/src/formats/vhd/mod.rs +++ b/block/src/formats/vhd/mod.rs @@ -132,15 +132,12 @@ impl disk_file::AsyncDiskFile for VhdDisk { mod unit_tests { use std::fs::File; use std::io::{Seek, SeekFrom, Write}; - #[cfg(feature = "io_uring")] use std::os::fd::AsRawFd; use vmm_sys_util::tempfile::TempFile; use super::*; - use crate::async_io::AsyncIo; - #[cfg(feature = "io_uring")] - use crate::async_io::{AsyncIoError, AsyncIoOperation, OwnedIoBuffer}; + use crate::async_io::{AsyncIo, AsyncIoError, AsyncIoOperation, OwnedIoBuffer}; use crate::disk_file::{AsyncDiskFile, DiskSize, PhysicalSize, Resizable}; /// Minimal fixed VHD footer (disk type = 2, current_size = 0x11223344). @@ -204,9 +201,56 @@ mod unit_tests { assert_async_io(&disk, true); } + #[test] + fn sync_rejects_read_straddling_logical_size() { + let file = TempFile::new().unwrap().into_file(); + file.set_len(0x2000).unwrap(); + let mut sync_io = FixedVhdSync::new(file.as_raw_fd(), 0x1000).unwrap(); + let op = AsyncIoOperation::read_to_vec(0x800, OwnedIoBuffer::from_vec(vec![0; 0x900]), 1); + + assert!(matches!( + sync_io.submit_data_operation(op), + Err(AsyncIoError::ReadVectored(_)) + )); + } + + #[test] + fn sync_rejects_write_straddling_logical_size() { + let file = TempFile::new().unwrap().into_file(); + file.set_len(0x2000).unwrap(); + let mut sync_io = FixedVhdSync::new(file.as_raw_fd(), 0x1000).unwrap(); + let op = + AsyncIoOperation::write_from_vec(0x800, OwnedIoBuffer::from_vec(vec![0; 0x900]), 1); + + assert!(matches!( + sync_io.submit_data_operation(op), + Err(AsyncIoError::WriteVectored(_)) + )); + } + + #[test] + fn sync_accepts_operation_exactly_filling_logical_size() { + let file = TempFile::new().unwrap().into_file(); + file.set_len(0x2000).unwrap(); + let mut sync_io = FixedVhdSync::new(file.as_raw_fd(), 0x1000).unwrap(); + // end == size: boundary must be accepted + let op = AsyncIoOperation::read_to_vec(0, OwnedIoBuffer::from_vec(vec![0; 0x1000]), 1); + sync_io.submit_data_operation(op).unwrap(); + } + + #[test] + fn sync_accepts_operation_at_last_byte() { + let file = TempFile::new().unwrap().into_file(); + file.set_len(0x2000).unwrap(); + let mut sync_io = FixedVhdSync::new(file.as_raw_fd(), 0x1000).unwrap(); + // end = 0xFFF + 1 = 0x1000 == size: boundary must be accepted + let op = AsyncIoOperation::read_to_vec(0xFFF, OwnedIoBuffer::from_vec(vec![0; 1]), 1); + sync_io.submit_data_operation(op).unwrap(); + } + #[cfg(feature = "io_uring")] #[test] - fn io_uring_batch_rejects_request_past_logical_size() { + fn io_uring_batch_rejects_request_straddling_logical_size() { let file = TempFile::new().unwrap().into_file(); file.set_len(0x2000).unwrap(); let mut async_io = FixedVhdAsync::new(file.as_raw_fd(), 8, 0x1000).unwrap(); @@ -218,6 +262,20 @@ mod unit_tests { )); } + #[cfg(feature = "io_uring")] + #[test] + fn io_uring_rejects_single_op_straddling_logical_size() { + let file = TempFile::new().unwrap().into_file(); + file.set_len(0x2000).unwrap(); + let mut async_io = FixedVhdAsync::new(file.as_raw_fd(), 8, 0x1000).unwrap(); + let op = AsyncIoOperation::read_to_vec(0x800, OwnedIoBuffer::from_vec(vec![0; 0x900]), 1); + + assert!(matches!( + async_io.submit_data_operation(op), + Err(AsyncIoError::ReadVectored(_)) + )); + } + #[test] fn try_clone_preserves_sync_dispatch() { let file = make_vhd_file(); diff --git a/block/src/formats/vhd/worker/async_uring.rs b/block/src/formats/vhd/worker/async_uring.rs index 294459678..aa042b470 100644 --- a/block/src/formats/vhd/worker/async_uring.rs +++ b/block/src/formats/vhd/worker/async_uring.rs @@ -12,6 +12,7 @@ use vmm_sys_util::eventfd::EventFd; use crate::async_io::{AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult}; use crate::error::BlockResult; use crate::formats::raw::worker::async_uring::RawAsync; +use crate::formats::vhd::worker::common::validate_operation_bounds; pub struct FixedVhdAsync { raw_file_async: RawAsync, @@ -27,37 +28,6 @@ impl FixedVhdAsync { size, }) } - - fn validate_operation_bounds(&self, op: &AsyncIoOperation) -> AsyncIoResult<()> { - let offset = u64::try_from(op.offset()).map_err(|_| self.bounds_error(op))?; - let len = u64::try_from(op.total_len()).map_err(|_| self.bounds_error(op))?; - let end = offset - .checked_add(len) - .ok_or_else(|| self.bounds_error(op))?; - - if end > self.size { - return Err(self.bounds_error(op)); - } - - Ok(()) - } - - fn bounds_error(&self, op: &AsyncIoOperation) -> AsyncIoError { - let error = io::Error::new( - io::ErrorKind::InvalidData, - format!( - "Invalid request offset {} and length {}, can't exceed file size {}", - op.offset(), - op.total_len(), - self.size - ), - ); - if op.is_read() { - AsyncIoError::ReadVectored(error) - } else { - AsyncIoError::WriteVectored(error) - } - } } impl AsyncIo for FixedVhdAsync { @@ -66,7 +36,7 @@ impl AsyncIo for FixedVhdAsync { } fn submit_data_operation(&mut self, op: AsyncIoOperation) -> AsyncIoResult<()> { - self.validate_operation_bounds(&op)?; + validate_operation_bounds(&op, self.size)?; self.raw_file_async.submit_data_operation(op) } @@ -96,7 +66,7 @@ impl AsyncIo for FixedVhdAsync { fn submit_batch_requests(&mut self, batch_request: Vec) -> AsyncIoResult<()> { for op in &batch_request { - self.validate_operation_bounds(op)?; + validate_operation_bounds(op, self.size)?; } self.raw_file_async.submit_batch_requests(batch_request) diff --git a/block/src/formats/vhd/worker/common.rs b/block/src/formats/vhd/worker/common.rs new file mode 100644 index 000000000..4562c7577 --- /dev/null +++ b/block/src/formats/vhd/worker/common.rs @@ -0,0 +1,38 @@ +// Copyright 2026 The Cloud Hypervisor Authors. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 + +use std::io; + +use crate::async_io::{AsyncIoError, AsyncIoOperation, AsyncIoResult}; + +pub(super) fn validate_operation_bounds(op: &AsyncIoOperation, size: u64) -> AsyncIoResult<()> { + let offset = u64::try_from(op.offset()).map_err(|_| bounds_error(op, size))?; + let len = u64::try_from(op.total_len()).map_err(|_| bounds_error(op, size))?; + let end = offset + .checked_add(len) + .ok_or_else(|| bounds_error(op, size))?; + + if end > size { + return Err(bounds_error(op, size)); + } + + Ok(()) +} + +fn bounds_error(op: &AsyncIoOperation, size: u64) -> AsyncIoError { + let error = io::Error::new( + io::ErrorKind::InvalidData, + format!( + "Invalid request offset {} and length {}, can't exceed file size {}", + op.offset(), + op.total_len(), + size + ), + ); + if op.is_read() { + AsyncIoError::ReadVectored(error) + } else { + AsyncIoError::WriteVectored(error) + } +} diff --git a/block/src/formats/vhd/worker/mod.rs b/block/src/formats/vhd/worker/mod.rs index 39769731c..f0a3eaf41 100644 --- a/block/src/formats/vhd/worker/mod.rs +++ b/block/src/formats/vhd/worker/mod.rs @@ -9,4 +9,5 @@ #[cfg(feature = "io_uring")] pub(crate) mod async_uring; +mod common; pub(crate) mod sync; diff --git a/block/src/formats/vhd/worker/sync.rs b/block/src/formats/vhd/worker/sync.rs index d0a91e183..59aa5bca9 100644 --- a/block/src/formats/vhd/worker/sync.rs +++ b/block/src/formats/vhd/worker/sync.rs @@ -11,6 +11,7 @@ use vmm_sys_util::eventfd::EventFd; use crate::async_io::{AsyncIo, AsyncIoCompletion, AsyncIoError, AsyncIoOperation, AsyncIoResult}; use crate::formats::raw::worker::sync::RawSync; +use crate::formats::vhd::worker::common::validate_operation_bounds; pub struct FixedVhdSync { raw_file_sync: RawSync, @@ -32,22 +33,7 @@ impl AsyncIo for FixedVhdSync { } fn submit_data_operation(&mut self, op: AsyncIoOperation) -> AsyncIoResult<()> { - let offset = op.offset(); - if offset as u64 >= self.size { - let error = io::Error::new( - io::ErrorKind::InvalidData, - format!( - "Invalid offset {}, can't be larger than file size {}", - offset, self.size - ), - ); - return Err(if op.is_read() { - AsyncIoError::ReadVectored(error) - } else { - AsyncIoError::WriteVectored(error) - }); - } - + validate_operation_bounds(&op, self.size)?; self.raw_file_sync.submit_data_operation(op) }