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 <alexander.lvov.git@gmail.com>
Reuse global validate_bounds() operation helper instead of
having a local implementation in vhd/worker/common.rs
Signed-off-by: Alexander Lvov <alexander.lvov.git@gmail.com>
Extract the dynamic VHDX qemu-img helper into a shared vhdx::
test_util module to reuse inside the upcoming VhdxSync bounds-check.
Signed-off-by: Alexander Lvov <alexander.lvov.git@gmail.com>
```
warning: using `chunks_exact` with a constant chunk size
--> block/src/formats/qcow/internal/header.rs:253:39
|
253 | for entry in data.chunks_exact(FEATURE_NAME_ENTRY_SIZE) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `as_chunks` instead
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#chunks_exact_to_as_chunks
= note: `-D clippy::chunks-exact-to-as-chunks` implied by `-D clippy::all`
```
Signed-off-by: Rob Bradford <rbradford@meta.com>
The region table overlap fix in RegionInfo::new computes each entry's
end offset as `file_offset + length`. Both values are taken verbatim
from the image, so a crafted or corrupt VHDX can set a file offset near
u64::MAX and make that addition wrap. A wrapped end offset compares as a
small value, which can slip a genuinely overlapping region past the
half-open interval check that #8483 added.
Use checked_add for the end offset and return a new RegionEntryOverflow
error when it wraps, so a malformed entry is rejected instead of being
folded into a valid-looking range. The computed end is now reused for
the region_entries map so the bound is only calculated once. Add a
regression test for a wrapping entry.
Signed-off-by: Sayed Kaif <metsw24@gmail.com>
Nothing reads or writes the AlignedFile through a cursor anymore, so
remove the Read, Write, and Seek impls together with the in memory
position field. SeekHole no longer tracks a position. The cursor unit
tests move to read_at and write_at, dropping the one that duplicated
existing coverage.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Request::execute seeked the disk to the request sector and then read or
wrote sequentially through the cursor. Walk a running offset with
read_exact_at and write_all_at instead, and flush with fsync, so the
bound becomes FileExt and FileSync rather than Seek, Read, and Write.
The seek step is gone, so drop the now unused ExecuteError::Seek.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
detect_image_type read the first block through the AlignedFile Read
cursor. Read it with read_exact_at at offset 0 instead, so image type
detection no longer depends on the cursor.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Read the trailing footer sector with query_device_size and
read_exact_at instead of seeking to the end of the AlignedFile and
reading through its cursor.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the qcow image compressor test helper to positional access
instead of the cursor. Test only change.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the qcow sync worker test helpers to positional access instead
of the cursor. Test only change.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the qcow header test helpers to positional access instead of
the AlignedFile cursor. Test only change.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the QcowRawFile unit test verifications to positional access
instead of the cursor. Test only change.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the qcow header feature bit writes to positional access and
drop the now unused Seek imports. The result is unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the qcow header reader and writer to positional access instead
of the AlignedFile Read, Write, and Seek cursor. The writes still go
through the AlignedFile O_DIRECT bounce, so alignment is unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
zfs and virtiofs do not eagerly account a mode-0 fallocate() in
st_blocks, so detect those filesystems with fstatfs() and skip the
physical-size assertions there. A skip now names a proven platform
limitation instead of being inferred from the value under test, and
every assertion still runs unconditionally on ext4/xfs.
Fixes#8296
Signed-off-by: Adel-Ayoub <adelayoub.maaziz@gmail.com>
Add a write side counterpart to read_unaligned_propagates_closure_error.
The test drives write_unaligned with a gather closure that returns an
error and checks that write_unaligned surfaces it unchanged.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Cover the qcow2 magic and the non qcow magic cases of the AlignedFile
detect_image_type, which now reads the magic positionally.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Opening a raw backing file issued a seek to the end for its size and
then rewound the cursor. RawBacking reads through read_exact_at, so
the cursor reset was dead. Query the size through query_device_size,
matching the crate convention and returning the right size for a
block device backing file as well, and drop the rewind. The now
unused SeekFrom import is removed.
The result is unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
detect_image_type saved the cursor, rewound, read the magic, then
restored the cursor. Read the four magic bytes with read_exact_at at
offset 0 and decode with from_be_bytes, so the save, rewind, and
restore go away. BeUint moves to the test module, its only remaining
user in this file.
The result is unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The refcount rebuild check issued a seek to the refcount table and to
the first refblock before each cursor read. Read the fixed size fields
with read_exact_at at their offsets and decode with from_be_bytes. The
seeks and the matching error paths go away.
The result is unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The compressed cluster write and read and the L1 resize size query
went through a seek on the AlignedFile cursor before the access. Pass
the target offset to write_at and read_exact_at, and read the file
length from physical_size.
The result is unchanged. The compressed paths keep routing through the
AlignedFile O_DIRECT bounce.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Cover read_unaligned and write_unaligned directly: a scatter read at an
unaligned offset, a short read at EOF, a read-modify-write gather that
preserves head and tail padding, and error propagation from the
scatter closure.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
run_unaligned_operation staged every unaligned request in a plain Vec
and then handed it to AlignedFile, which bounced again through an
aligned buffer. That Vec only gave the operation a contiguous range to
scatter into or gather from, which the aligned buffer already is, so
each slow path request paid for an extra allocation and a full length
copy.
Add read_unaligned and write_unaligned on AlignedFile that own the
single aligned bounce and scatter or gather through a closure over the
staging slice. run_unaligned_operation and the FileExt read_at and
write_at impls both route through them, so the staging and
read-modify-write logic lives in one place. The closures keep
AlignedFile free of any AsyncIoOperation dependency.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Replace the use of unsafe struct casting with zerocopy trait derivation.
This fixes a Rust UB where the struct was being filled with a slice of
length greater than the size of the struct.
As a compromise the guid handling was changed to handle the uuids as
opaque bytes as they are mixed endian. This has no impact on the
functionality as they are only used for comparison and has the positive
impact of reducing some of the uuid handling complexity.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Rob Bradford <rbradford@meta.com>
Cover refcount block round trip for the byte aligned and sub byte
paths, and add_cluster_end appending an aligned cluster and staying
within the maximum offset bound.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
add_cluster_end queried the file length by seeking to the end. Use the
existing physical_size helper instead, which reads the length from the
file metadata. This removes the final cursor access in QcowRawFile, so
the Seek and SeekFrom imports are no longer needed.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The refcount block read and write helpers took a file whose cursor was
positioned by a preceding seek. Pass the target offset down instead and
use positional read_exact_at and write_all_at on the AlignedFile, so
the block methods no longer seek. The byte aligned and sub byte writers
build a buffer and issue one positional write, keeping the previous
batching.
The result is unchanged, as the calls still route through the
AlignedFile O_DIRECT bounce.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The `read_aligned_block_size()` function used `Vec::from_raw_parts()`
incorrectly, causing undefined behavior when deallocating the `Vec<u8>`.
One of the safety invariants of `Vec::from_raw_parts()` is that the
provided pointer must be allocated with the exact same alignment as `T`
(`u8` in this case), but this is clearly not true: `align_of::<u8>()` is
1, but the pointer was allocated with aligment of `blocksize` (typically
512 or greater).
Fix this by using the existing `AlignedFile` helper to read the header
block when probing the image type. This is slightly less efficient than
using `AlignedBuffer` directly, but since this is only called once per
disk image at startup, the difference is probably not worth the extra
verbosity.
Signed-off-by: Daniel Verkamp <drv@meta.com>
Cover read_pointer_table round trip and masking, and the
write_cluster then zero_cluster round trip, exercising the positional
read_exact_at, write_all_at, and write_all_zeroes_at paths on the
AlignedFile.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Replace the seek then read/write metadata access in QcowRawFile with
positional read_exact_at, write_all_at, and write_all_zeroes_at on the
AlignedFile. read_pointer_table, write_pointer_table,
write_pointer_table_direct, zero_cluster, and write_cluster no longer
move the file cursor.
These calls still route through AlignedFile, which implements FileExt
and WriteZeroesAt with the O_DIRECT alignment bounce, so the unaligned
behavior is preserved. Each access already issued an absolute seek
before touching the file, so the cursor never carried state between
calls and dropping it is unobservable.
Decoding the pointer table now uses native from_be_bytes over the read
buffer, matching the to_be_bytes path on the write side.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The region table overlap check in RegionInfo::new only rejected a new
region that strictly engulfed an existing one. Identical, fully
contained, and partially overlapping regions passed undetected, so a
malformed VHDX with overlapping region entries was wrongly accepted.
Per [MS-VHDX] all region objects MUST be non-overlapping, so such an
image should be rejected. Replace the faulty predicate with a correct
half-open interval overlap test, extracted into a small pure helper
(ranges_overlap).
Add a unit test for the predicate and an integration test that feeds a
crafted region table with two overlapping entries through the real
RegionInfo::new, confirming it is now rejected with RegionOverlap.
Related to #8009 (broader VHDX overlap validation).
Signed-off-by: Henry Hrvoje Tonkovac <htonkovac@gmail.com>
Assisted-by: Claude:Opus-4.8
size_of is part of std::prelude as of Rust 1.80 (with size_of_val,
align_of, align_of_val), and the workspace MSRV is 1.89, so qualifying
it (mem::size_of, std::mem::size_of, core::mem::size_of) is unnecessary.
Convert every qualified size_of call-site to the bare prelude form and
drop the now-redundant `use std::mem::size_of;` imports, keeping
`use std::mem;` where it still serves non-prelude items (transmute,
swap, replace, take, zeroed, MaybeUninit, offset_of). size_of is the
only one of the four currently used in the tree.
Pure refactor, no behavioural change. Follow-up to the
clippy::absolute_paths cleanup (#7670), as discussed in #8444.
Signed-off-by: Henry Hrvoje Tonkovac <htonkovac@gmail.com>
Assisted-by: Claude:Opus-4.8
BlockBackend predated the disk_file trait family and only carried
logical_size and physical_size, which the disk backends expose
through the disk_file traits DiskSize and PhysicalSize.
It added no polymorphism while its Read, Write and Seek supertraits
forced an unused cursor. Dropping the trait removes the dead code
it was keeping alive.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The qcow workers carried their own O_DIRECT alignment requirement
and bounced unaligned cluster accesses through AlignedBuffer. Now
that the data file is an AlignedFile that handles O_DIRECT
transparently, the qcow layer can read and write through plain
buffers and let AlignedFile perform the aligned bounce.
Remove the alignment field and the per cluster AlignedBuffer RMW
branches from both the sync and async workers. The async io_uring
fast path still needs to avoid submitting unaligned guest iovecs
under O_DIRECT, so gate it on is_direct rather than on a stored
alignment value.
Drop the QcowAsync alignment override so it reports the trait
default sector size, matching QcowSync. qcow never submits guest
iovecs to the kernel under O_DIRECT, so reporting a larger value
only forced the request layer into an extra bounce buffer.
This adds one buffer copy per unaligned O_DIRECT cluster but moves
all alignment handling into a single place. The buffered path is
unchanged.
With qcow no longer the only caller, AlignedBuffer::read_exact_from
becomes dead code, so remove it and switch its tests to read_from.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Thread the direct flag from the disk open options through VhdDisk into
the AlignedFile the workers run on, so a fixed VHD opened with direct=on
issues O_DIRECT I/O instead of buffered I/O. Alignment is probed once on
that AlignedFile and reused by the sync and io_uring workers.
Advertise host topology from VhdDisk::topology by probing the underlying
file. On a 4096 byte sector filesystem opened with O_DIRECT this reports
logical_block_size 4096 to the guest, so the guest never issues 512 byte
I/O that the host kernel would reject as misaligned. Falls back to the
default topology with a warning when the probe fails.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Read the trailing footer sector through an AlignedFile rather than
probing the device topology and reading a full logical block. The
AlignedFile bounce buffer serves the trailing sector of an O_DIRECT
fd whose offset is unaligned against the device block size, so the
read no longer fails with EINVAL on a 4k sector backing store.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The raw sync, io_uring and AIO workers now own an AlignedFile and use
it for the O_DIRECT alignment value and for the unaligned fallback.
Aligned operations keep the fast preadv and pwritev iovec path straight
to the kernel. When the offset or an iovec base or length is not a
multiple of the probed alignment, the worker gathers the iovecs into
one contiguous host buffer and runs a synchronous RMW through
AlignedFile, then scatters the result back into guest memory.
RawDisk constructs the AlignedFile from the disk file and the direct
flag and passes it into each worker, so alignment is probed once at
open time. The fixed VHD workers are threaded through the same
AlignedFile based constructors using a non-direct AlignedFile to
preserve current behavior.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
RawFile wrapped AlignedFile only to add a seek position and the file
trait impls that the qcow and vhost_user_block code expects. Fold that
position and every impl onto AlignedFile so the wrapper layer goes away
and callers work with a single O_DIRECT aware file type.
AlignedFile now tracks a cursor and implements Read, Write, Seek,
WriteZeroesAt, PunchHole, FileSync, SeekHole, BlockBackend, Clone,
AsRawFd and AsFd in addition to the positional FileExt path. The
direct_io flag is dropped because alignment already encodes it, where
a zero alignment means the file was not opened with O_DIRECT.
All RawFile uses in the qcow internals and vhost_user_block move to
AlignedFile, and raw_file.rs is removed.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Move the statx STATX_DIOALIGN probe out of DiskTopology into a free
probe_direct_alignment helper keyed on a raw fd. The helper gates on the
O_DIRECT open flag and returns the kernel reported alignment only when
direct I/O is in effect, and None otherwise. DiskTopology::probe keeps
the same call path and result.
AlignedFile::new now determines its O_DIRECT block alignment from
probe_direct_alignment instead of trial reads at 512 and 4096, falling
back to SECTOR_SIZE when the kernel does not report a value. This
matches how the raw and fixed VHD workers determine alignment, so all
backends agree on one source of truth.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Closing an io_uring fd does not synchronously finish requests that
already reached the kernel. During block worker teardown this can let
an io-wq worker keep using retained guest-memory iovecs after reset.
Drain UringDataIo in Drop: retry any published SQEs and wait for CQEs
until no retained operation remains. If draining fails, leak retained
buffers. Drop QcowAsync's ring before its data fd so retrying
published SQEs still uses a valid descriptor.
To avoid a potential infinite loop when completions fail to be delivered
cap the number of iterations of the loop (2x the number of inflight
requests).
Assisted-by: Codex:GPT-5
Signed-off-by: Rob Bradford <rbradford@meta.com>
Import the std modules used in the test module instead of spelling the
full paths at every use site, and drop the now-unnecessary
#[expect(clippy::absolute_paths)] on the vhdx internal test module.
Signed-off-by: Henry Hrvoje Tonkovac <htonkovac@gmail.com>
Assisted-by: Claude:Opus-4.8
Convert the still-needed #[allow]s to #[expect] so they warn if the
lints stop firing.
Part of #8326.
Signed-off-by: Tushar Khatri <hello@tusharkhatri.in>
The sync I/O worker only checked that the operation offset did
not start past the end of the virtual disk (offset >= size) -
did not verify that the operation end (offset + len) stays
within bounds.
A read or write that started inside the image but extended
beyond the logical size was silently passed to the raw backend.
The async io_uring worker already had the correct check
(offset + len > size with overflow protection). I extracted it
into a shared helper in worker/common.rs and reused inside the
sync path to eliminate duplication and close the gap.
Fixes#8311
Signed-off-by: Alexander Lvov <alexander.lvov.git@gmail.com>
Removal of absolute paths is currently in progress. To avoid regressing
those changes add a clippy deny at the workspace level and at the crate
level override with #[expect(clippy::absolute_paths)]
See: #7670
Signed-off-by: Rob Bradford <rbradford@meta.com>
Acquiring an image lock can be interrupted with EINTR. In this case, we
returned with an error. Instead, we now retry acquiring the lock.
Signed-off-by: Pascal Scholz <pascal.scholz@cyberus-technology.de>
On-behalf-of: SAP pascal.scholz@sap.com
By redirecting VHDx I/O through the AlignedFile the required RMW
semantics can be achieved for writes less than the logical block size
whilt reusing the same logic used for other backend implementations.
Assisted-by: Claude:Opus-4.8
Signed-off-by: Rob Bradford <rbradford@meta.com>
