The inflate and deflate queues carry device-readable PFN arrays: the
driver writes them and the device only reads. The device never writes
to the descriptor buffers, so the used ring entry should report 0
bytes written rather than the descriptor length.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Fix "Fail tp signal" typo, replace copy-pasted "entropy" comment
with "balloon", and remove EventFdWriteFail and QueueIterator error
variants that have no call sites.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
When address translation or the entropy read fails mid-chain the
handler reset total_len to zero before breaking out of the loop.
If earlier descriptors in the same chain were already filled, the
used ring entry under-reports the bytes actually written to guest
memory. Drop the reset so the used length reflects reality.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
A device-readable descriptor anywhere in the chain caused the handler
to reset the byte count to zero and abandon the rest of the chain.
This discards valid device-writable descriptors that follow and
misreports bytes already written to earlier descriptors.
Skip device-readable and zero-length descriptors individually so
the remaining device-writable buffers still get filled with entropy.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Worker threads spawned through spawn_virtio_thread previously wrote to
exit_evt on any clean Err return, taking the whole VMM down on a single
failed device worker. A guest induced fault in any virtio device thus
propagated into a host wide failure.
Route the Err return through the shared mark_device_needs_reset helper
instead. The helper sets the DEVICE_NEEDS_RESET bit on device_status
and triggers a config change interrupt, so the device goes idle and
the guest is informed. The thread exits cleanly without killing the
rest of the VMM.
The panic and the seccomp filter apply paths keep writing to exit_evt.
A panicked worker may have left poisoned locks or partially mutated
state, so a hard exit remains the right policy there.
spawn_virtio_thread now takes the device_status and the interrupt
callback. Every native virtio and vhost-user call site is updated to
pass them in.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Both block and net implement the same DEVICE_NEEDS_RESET bookkeeping
when a corrupted virtqueue request is detected. They set the bit,
trigger a config change interrupt and log a warning. Move that logic
into shared device_needs_reset and mark_device_needs_reset helpers in
lib.rs and update both call sites to use them.
No functional change.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The per-domain mapping cap bounds memory inside one domain, but a
guest can still grow the domains map indefinitely with ATTACH
requests for distinct domain IDs. Reject ATTACH with
VIRTIO_IOMMU_S_NOMEM at 64K domains.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
The virtio spec says the device must never present a value other
than 0 or 1 for bypass. Mask off the upper bits on write so a
later read does not return whatever the driver wrote.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
The device offers VIRTIO_IOMMU_F_INPUT_RANGE when the address
width is constrained, but never validates that guest MAP requests
fall inside the advertised range. The virtio spec requires such
requests to fail with VIRTIO_IOMMU_S_RANGE.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
The virtio spec mandates VIRTIO_IOMMU_S_NOENT, not S_INVAL, when
the target domain does not exist.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
The handler accepted unknown flag bits, unaligned ranges, and
overlapping mappings, and returned the wrong status code when the
target domain did not exist. The virtio spec requires explicit
rejections for each of these.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
The virtio spec requires the device to reject ATTACH with a
non-zero reserved field, an unknown flag bit, or a bypass flag
that conflicts with an existing domain. The current handler
silently accepts all three.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
The virtio spec requires the device to leave the reply buffer
untouched and report a used length of zero for an unrecognised
request type, so the driver can tell the request was not handled.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
Domain::mappings only shrinks on UNMAP. Without a bound a guest can
issue MAP for arbitrarily many distinct virt_start values and drive
the VMM heap until the host runs out.
Reject MAP with VIRTIO_IOMMU_S_NOMEM at 1M entries per domain.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
A concurrent DETACH between the read-lock check and the write-lock
get_mut().unwrap() in MAP/UNMAP would panic the worker. Replace the
unwrap with a let-else.
A failure on a later endpoint in the per-endpoint MAP loop, or a
DETACH that races the missing-domain branch, must roll back the
external mappings already installed; otherwise domain.mappings
diverges from VFIO state.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
An UNMAP that would split an existing mapping must be rejected with
VIRTIO_IOMMU_S_RANGE without removing anything. The previous start-only
retain silently left mappings that started outside the unmap range but
overlapped it.
Walk bookkeeping under a read lock and reject before touching VFIO so a
rejection cannot leave VFIO and bookkeeping out of sync.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
The DmaRemapping translate_gva and translate_gpa entry points discarded
the size argument that AccessPlatform's signature already carries and
only checked the base address. A buffer beginning inside a mapping but
extending past it was treated as fully translated, allowing reads or
writes outside the IOMMU-authorized window.
Add `size` to the trait, validate the full span fits in a single
mapping, and propagate it through AccessPlatformMapping.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
The reply length was `hdr_len + size_of::<tail>()`, computed twice.
Make it explicit via checked_add and reuse the result.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
Verify that firing a config change interrupt with msix_config
vector beyond the table size returns Ok without panicking.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Verify that a valid in bounds vector with MSI-X enabled
successfully triggers the interrupt source group.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Verify that requesting a notifier with an out-of-bounds MSI-X
vector returns None instead of panicking.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Verify that triggering an interrupt when the vector is set to
VIRTQ_MSI_NO_VECTOR short-circuits and returns Ok.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Verify that firing an interrupt with a queue vector beyond the
MSI-X table size returns Ok without panicking.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
A malicious or buggy guest can write an out-of-bounds value to
queue_msix_vector or msix_config. When the device later triggers
an interrupt, it indexes into table_entries with the unchecked
vector, causing a panic.
Validate the vector against the MSI-X table size in both trigger()
and notifier() paths, logging a warning and returning early when
the vector exceeds the table bounds.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Follow the same pattern as other virtio devices using a bool to check if
it needs notification and propagating its own Error enum.
Sadly this does still use `anyhow!()` but this does match with the
behaviour of the other devices in their implementations.
As a side effect we can now remove two errors from the top-level Error
enum in virtio-devices as these were only used by this module and those
errors had mangled descriptions.
Signed-off-by: Rob Bradford <rbradford@meta.com>
`get_host_address_range` used `check_range(addr, size)` as a guard then
unwrapped `get_slice(addr, size)`. This allowed a span across two
regions to hit the unwrap (get_slice limits to one range).
If `size` were zero, then the checks were all skipped. Causing a panic
later on for an invalid address.
Make get_slice the sole authority and reject size==0 explicitly.
Callers already handle None.
Signed-off-by: Dylan Reid <dgreid@fb.com>
The guest can cause submit and completion failures with malformed chains
or invalid addresses. However, this shouldn't permanently stall the
device and terminate the worker.
Genuine reset-worthy failures set needs_reset and return `Ok` anyways
and will more cleanly reset the worker.
Signed-off-by: Dylan Reid <dgreid@fb.com>
process_queue_submit's drain loop builds a fresh queue.iter() per
iteration, which re-reads the guest avail index on every call and has
no per-call cap (the per-iter gap check in virtio-queue only protects
against avail_idx jumping more than queue_size between two reads).
In theory, a malicous or buggy guest could keep adding descriptors and
cause this loop to overflow the iouring submit queue.
Cap a single drain at queue_size. A spec-compliant driver never
produces more than queue_size outstanding entries simultaneously, so
the cap is invisible to well-behaved guests.
Signed-off-by: Dylan Reid <dgreid@fb.com>
If querying the fd's MTU fails (because it was from a different network
namespace). Degrade gracefully by not advertising the VIRTIO_NET_F_MTU
feature and instead let the guest kernel use the default 1500 Ethernet
MTU.
Signed-off-by: Rob Bradford <rbradford@meta.com>
The TX path's inline-data branch didn't check the inline buffer length
against the guest-supplied pkt.len() field. The worker will later panic
when it tries to index the packet.
Add the missing check, mirroring the other TX branches.
Signed-off-by: Dylan Reid <dgreid@fb.com>
dma_unmap computed `iova + size - 1` unchecked while the sibling dma_map
already used checked_add/checked_sub. A guest reaching dma_unmap via
VIRTIO_IOMMU_T_UNMAP could cause a panic.
Signed-off-by: Dylan Reid <dgreid@fb.com>
Catch u64 overflows on map so that later translation requests from the
guest don't have a vector for causing a host panic.
Signed-off-by: Dylan Reid <dgreid@fb.com>
prepare_activator() called queue.is_valid() and only logged the failure,
then still pushed the queue to the activator. This would later panic.
Technically this is a fixup for:
a10508970 "virtio-devices: Support driver programming fewer queues"
But that's been in there since 2021. The intent was to allow a subset of
possible queues to be configured, but the invalid queues slipped through
too.
Signed-off-by: Dylan Reid <dgreid@fb.com>
Add support for backend that is connected via the vhost-user-generic
frontend to generate an interrupt into the guest when it has made a
change to the configuration. This is useful for devices that can change
the exposed configuration at runtime.
Assisted-by: Claude:Opus-4.7
Signed-off-by: Rob Bradford <rbradford@meta.com>
The VirtioInterrupt is now always created so the Option<..> can always
be removed.
As a side effect the interrupt_source_group can also be removed from the
struct.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Since this is always created there is no need to make it an Option type
simplifying the code. Historically it was an Option to support INTx
based virtio but that was removed long ago.
Signed-off-by: Rob Bradford <rbradford@meta.com>
msix_num is guaranteed to be at least 1 so this check (and the Option)
that it returns can be removed.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Rather than calculate in the DeviceManager and pass it through do it in
the device where it already has all the required information.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Previously the interrupt was created in VirtioPciDevice, moved via the
Option::take() to the VirtioPciDeviceActivator and then moved to the
VirtioDevice upon activation. On reset it would be moved back ready for
reactivation.
Since this already an Arc type remove the wrapping Option and instead
refcount it such that the VirtioPciDevice can continue to hold onto it
for later activations.
This significantly simplifies the reset() logic as there is no need to
hand back the interrupt.
A few devices used whether the interrupt was Some to make triggering an
interrupt a no-op. However the MSI-X interrupt routing already drops the
interrupt if the driver hasn't yet configured the vector so it is safe
to trigger the interrupt before device activation (e.g. balloon resize
request before driver loaded).
VirtioCommon still retains an Option<..> for the interrupt as the
interrupt is not known until activation time (after this has been
created). A helper VirtioCommon::trigger_interrupt() has been added to
handle this.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Per the virtio spec a device reset must return the device to its
power-on state. The reset path was only zeroing queue_select. Add
VirtioPciCommonConfig::reset() and call it from the transport's reset
path so the configuration is cleared. Also relax the condition to allow
the device to be reset at any time to match the spec.
Signed-off-by: Rob Bradford <rbradford@meta.com>
The virtio specification treats reset as the recovery operation and so
must take the device back to a fresh state, and the driver waits for the
status read-back to converge before continuing. There is no defined way
for the device to report a reset failure to the driver.
Previously the implementations of reset() would return early and not
complete all their cleanup leaving them in an inconsistent state. Now
log errors and continue through the execution.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
The four vhost-user device wrappers (blk, fs, generic_vhost_user, net)
each carried an identical reset() body that resumed the worker thread,
asked the backend to reset, signalled kill_evt and dropped interrupt_cb.
Move the shared body into VhostUserCommon::reset() so behaviour stays in
one place.
No behavioural change.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-7
release_memory_range took (range_base, range_len) verbatim from a
guest-controlled descriptor (free-page-reporting and inflate paths) and
called fallocate(PUNCH_HOLE | KEEP_SIZE) on the backing file before any
length check ran. find_region only validates that range_base lands
inside *some* region; range_len can extend past the region's end. When
the operator uses --memory-zone file=PATH against a host file larger
than the zone, the punch zeroes host file content past the guest's
memory extent.
Free-page reporting is advisory, so the VMM is allowed to act on a
subset of a free range. Clamp range_len to the bytes that actually fit
within the region returned by find_region. Going past the end of a
region is most likely a guest bug so log it.
Signed-off-by: Dylan Reid <dgreid@fb.com>
All disk format backends now implement AsyncFullDiskFile directly.
The DiskBackend enum that dispatched between Legacy and Next arms
is no longer needed since the factory returns trait objects and vmm
no longer constructs format types manually.
Replace DiskBackend with Box<dyn AsyncFullDiskFile> in the Block
struct and its constructor. Remove the DiskBackend::Next wrapping
in device_manager and the fuzz target.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
There is no reason for most of the Request struct to be writable from
anywhere in the codebase. Encapsulate it.
Use getter functions for access outside the request module. Replace the
trivial setter for the writeback field with direct assignment.
No functional change intended.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
The new_ prefix in Rust conventionally denotes constructors that return
Self (e.g. Vec::new(), File::new()). AsyncDiskFile::new_async_io does
not return Self. It is a factory method that constructs and returns a
Box<dyn AsyncIo> worker bound to the disk file descriptor and
metadata. The create_ prefix communicates this: the caller receives
a freshly constructed object of a different type.
This rename touches every format backend in block plus two external
callers in virtio-devices and performance-metrics. Every change is a
mechanical s/new_async_io/create_async_io/ substitution. No functional
change.
Ref: #7877 (task 3.2.8)
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The driver needs to be notified with the number of bytes written by the
device. Ensure that the correct number of bytes is reported.
Signed-off-by: Rob Bradford <rbradford@meta.com>
The virtio spec allows a chain of writable descriptors however the rng
device was assuming just a single writable descriptor. Instead fill in
all writable descriptors. There is no status byte (unlike e.g. block)
and instead 0 bytes used is used to indicate error.
Signed-off-by: Rob Bradford <rbradford@meta.com>
