Add a 'disconnected' flag shared between VhostUserCommon and
VhostUserEpollHandler. This flag is set whenever the run loop hits an
error that would cause an exit (failed reconnect, broken backend req
handler, unknown event).
Following commits will use this to gate backend calls in order to avoid
repeated timeouts and errors when a backend disappears. This will
simplify shutdown sequencing for orchestrators using vhost-user devices.
Signed-off-by: Dylan Reid <dgreid@fb.com>
When a guest resets a device by writing status=0 and reinitializes
without enabling queues before writing DRIVER_OK, the activation
path would collect zero ready queues and treat that as a fatal
error, killing the entire VMM process.
The PCI transport now checks that at least one queue is ready
before reporting that the device needs activation. This prevents
a spurious activation attempt that would fatally fail when no
queues are enabled.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
config_generation is a u8 and the spec mandates wrap around.
Verify the increment past 0xff lands on 0x00 without panicking
under overflow checks.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Cover the three observable cases of consume_config_change. With
the flag set the counter advances by one and the flag is cleared.
With the flag clear the call is a no-op. A burst of flag sets
between two reads results in only one bump, which is the wrap
hazard mitigation the spec asks for.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
A Config trigger must set the config_changed flag so the next
device specific configuration read can bump config_generation. A
Queue trigger must leave the flag alone.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Set a config_changed flag in VirtioInterruptMsix when a Config
interrupt fires, and increment config_generation only when the
driver next reads the device specific configuration region. The
flag is cleared by that read so the driver observes a stable
value across the read and a fresh value on any later read.
This avoids the wrap hazard of incrementing on every Config
event, where a burst of interrupts could roll the 8 bit counter
back to its previous value between two driver reads.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the u8 field to Arc<AtomicU8> so the interrupt path can
mutate it without holding the common config mutex. Reads at
offset 0x15 use Acquire ordering. State serialization preserves
the value.
Update the three unit tests that construct
VirtioPciCommonConfig directly.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Currently when the socket is disconnected by the other end there is no
logging of such an event. Add a log to aid identifying when this has
happened.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Abandon the reconnection to the vhost-user socket if the kill_evt is
fired because e.g. a device removal request has come in during the
reconnection.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Replace the use of sleeps with a TimerFd. Initially this is functionally
equivalent but it can be extended to also handle other events.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Drop inflate or deflate descriptors whose len exceeds the Linux
driver maximum of VIRTIO_BALLOON_ARRAY_PFNS_MAX PFN entries of 4
bytes each. Without the cap, a guest can submit a descriptor with
a huge len over a small backing and drive an unbounded warn loop
in the device thread.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
block::Error implements Display via thiserror, so the user facing
log lines do not need the Debug formatter.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
With the per handler needs_reset() gates removed from net and
block, nothing reads DEVICE_NEEDS_RESET anymore. Drop the
device_needs_reset helper and refresh the doc comment on
mark_device_needs_reset to reflect the central call site in
spawn_virtio_thread, where it runs after the worker has already
exited.
No functional change.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
With virtqueue iterator errors now killing the worker and
spawn_virtio_thread marking NEEDS_RESET centrally, the per
handler needs_reset() gate on process_queue_submit and
process_queue_complete is unreachable.
Drop needs_reset(), the two early returns, the unused
device_status field on BlockEpollHandler and its initializer,
and the device_needs_reset import.
No functional change.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
queue.iter() errors used to be swallowed by
handle_queue_iterator_error(), which marked the device as
NEEDS_RESET and returned Ok so the worker kept running while
disabled. spawn_virtio_thread now does the NEEDS_RESET marking
when the worker exits with an error.
Propagate the iterator error as Error::QueueIterator and escalate
it to EpollHelperError::HandleEvent in
process_queue_submit_and_signal so the worker exits. Per request
errors stay logged. Drop the now unused
handle_queue_iterator_error helper.
No functional change for the guest. NEEDS_RESET is still set and
the config interrupt is still raised on virtqueue corruption.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
spawn_virtio_thread now marks the device as NEEDS_RESET and notifies
the guest whenever the worker thread exits with an error, so the
per handler needs_reset() gate and the handle_queue_iterator_error()
helper in net are redundant.
Let virtqueue iterator errors propagate out of the worker thread
through DeviceError::NetQueuePair. Drop the unused device_status
field and the device_needs_reset and mark_device_needs_reset
imports.
No functional change for the guest. NEEDS_RESET is still set and
the config interrupt is still raised on virtqueue corruption.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Request::parse rejected unknown request types with an error, causing
process_queue to report the chain as used with no response written.
The device should write an error response so the driver knows the
request was handled. Move type validation out of parse into
process_queue where a proper response can be constructed.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Address translation, guest memory read, write, and flush failures on
the transmitq propagated errors that killed the console device thread.
A host-side I/O error such as a PTY disconnect would permanently
disable the console. Log warnings and continue processing instead.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Address translation or guest memory write failures on the receiveq
propagated errors that killed the console device thread. Log a
warning and break out of the descriptor loop instead.
Also fix a data-loss bug: bytes were drained from the input buffer
before the write to guest memory, so a failed write would silently
discard the data. Copy first, write, then drain only on success.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
The output queue handler read data from every descriptor without
checking the write-only flag. The driver must not put device-writable
buffers in the transmitq. Skip them with a warning.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
The input queue handler wrote data to every descriptor without
checking the write-only flag. The device must not write to
device-readable buffers. Skip them with a warning.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Remove UnknownRequestType (no longer returned after responding with
ERROR), EventFdWriteFail, EventFdTryCloneFail, MpscRecvFail, and
NotActivatedByGuest which have no call sites. Drop the now-unused
mpsc import.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
A malformed descriptor chain — wrong read/write flags, missing
descriptors, or undersized buffers — caused Request::parse to return
an error that killed the device thread. Log a warning, report the
chain as used with zero length, and continue processing.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
is_range_state() and set_range() took nb_blocks as u16 to match the
wire format, but unplug_all() computed the total block count from
region_size / block_size and cast to u16, silently truncating for
regions larger than 128 GiB. Widen the internal parameter to usize
so unplug_all() resets the full bitmap.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
When is_valid_range() returns false, the handler still computed
offset = addr - config.addr which can underflow if addr is below the
region base, then queried the bitmap at a meaningless index. Return
(ERROR, 0) immediately so no arithmetic runs on invalid input.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
An unknown request type caused process_queue to return an error that
killed the device thread. The request/response descriptors are already
parsed at this point, so respond with VIRTIO_MEM_RESP_ERROR and
continue processing.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Address translation or memory release errors on the free page
reporting queue propagated up and killed the balloon device thread.
Log a warning and skip the offending descriptor so the device
continues operating.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
A single malformed descriptor — device-writable where device-readable
is expected, non-aligned length, overflowed address, unmapped PFN, or
a failed fallocate/madvise — propagated an error that killed the
balloon device thread. Skip bad descriptors and PFNs with a warning
so the device keeps operating.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Only the first descriptor in each chain was processed. If the PFN
array spanned multiple chained descriptors the remaining PFNs were
silently dropped. Iterate the full chain so every descriptor is
validated and its PFNs are acted on.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
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>
