Import the std modules used across the crate instead of spelling the
full paths at every use site.
Signed-off-by: Henry Hrvoje Tonkovac <htonkovac@gmail.com>
Assisted-by: Claude:Opus-4.8
Several error paths in process_desc_chain returned the error before
calling queue.add_used for the offending descriptor. The affected
variants were DescriptorChainInvalid, DescriptorChainTooShort,
DescriptorInvalidHeader, and the GuestMemory variants raised during
descriptor chain translation, slice retrieval, or the num_buffers
write on the RX side.
Without an entry in the used ring the head descriptor remained owned
by the device. A guest that kept submitting bad chains could deplete
the queue over time.
Mark the head descriptor used with length 0 before propagating the
error to the caller, so the ring stays consistent regardless of how
the device decides to react to a guest induced failure.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
When readv returns fewer bytes than vnet_hdr_len the frame is
truncated. Report the truncated length to the guest in the used
ring instead of returning a fatal InvalidVirtioNetHeader error.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
When readv from the TAP returns EINVAL the guest posted a buffer
too small for the vnet_hdr. Return len 0 to the used ring and
continue instead of killing the worker thread. Also move
go_to_previous_position into the appropriate error branches only.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
When writev returns fewer bytes than vnet_hdr_len the packet is
truncated. Log the error and drop it instead of returning a fatal
InvalidVirtioNetHeader error that would crash the worker thread.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
When writev to the TAP returns EINVAL the guest submitted a
malformed packet. Drop it and continue instead of killing the
worker thread.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The virtio spec says the used-ring length is bytes the device wrote to
device writable descriptors. The net TX descriptors are device readable
only (the device wrote nothing back) so the length needs to be 0. On the
ctrl queue the number of bytes reported was wrongly the size of the
status descriptor not the number of bytes written (the descriptor is
permitted to be larger).
Signed-off-by: Rob Bradford <rbradford@meta.com>
The address that is passed from the guest should be treated as
untrusted. Currently an invalid address will panic the VMM. This only
allows the guest to hurt itself, but we shouldn't have the VMM crashing.
Instead let's return an error if possible or invalidate the queue if it
happen during setup.
The data flow from guest to translate_gva/translate_gpa is:
1. Guest writes a raw u64 address into a virtio descriptor in the
shared descriptor table (guest memory).
2. The virtio-queue crate reads this descriptor via read_obj() and
returns the addr field as-is in a GuestAddress — no validation.
3. Device code calls .translate_gva(access_platform, len) on the
GuestAddress.
4. With IOMMU (access_platform is Some): the address is an IOVA that
must be translated to a GPA via the IOMMU mapping table. If the
guest provides an unmapped IOVA, translation returns Err.
Previously, .unwrap() here panicked the VMM.
5. Without IOMMU (access_platform is None): translate_gva is a no-op
(returns self). The raw address flows to GuestMemory::read_obj()
which validates it — out-of-range addresses return
Err(InvalidGuestAddress), so no host memory corruption is possible.
Signed-off-by: Dylan Reid <dgreid@fb.com>
A buggy or malicious guest may write an inappropriate value into
virtqueue's next_avail field. This will result in an error
when iterating over the queue:
863837ef86/virtio-queue/src/queue.rs (L708)
but this error is (logged and) ignored if pop_descriptor_chain()
is used:
863837ef86/virtio-queue/src/queue.rs (L583)
A reasonable approach, implemented here, is to mark the device as
NEEDS_RESET and ignore further queue events until the guest
reinitializes the device.
How this patch was tested:
Linux kernel was patched to trigger a bad next_avail when the
virtqueue queue counter reaches 5000:
--------------- START OF LINUX KERNEL PATCH ----------
$ git diff
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b784aab668670..989f2a0c64a77 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -15,6 +15,9 @@
#include <linux/spinlock.h>
#include <xen/xen.h>
+
+void virtqueue_kick_always(struct virtqueue *vq);
+
#ifdef DEBUG
/* For development, we want to crash whenever the ring is screwed. */
#define BAD_RING(_vq, fmt, args...) \
@@ -677,6 +680,12 @@ static inline int virtqueue_add_split(
struct virtqueue *_vq,
* new available array entries. */
virtio_wmb(vq->weak_barriers);
vq->split.avail_idx_shadow++;
+ {
+ if ((vq->split.avail_idx_shadow % 100) == 0)
+ printk(KERN_ERR "avail idx: %d",
+ (int)vq->split.avail_idx_shadow);
+ if (vq->split.avail_idx_shadow == 5000)
+ vq->split.avail_idx_shadow = 0;
+ }
vq->split.vring.avail->idx = cpu_to_virtio16(_vq->vdev,
vq->split.avail_idx_shadow);
vq->num_added++;
@@ -689,6 +698,11 @@ static inline int virtqueue_add_split(
struct virtqueue *_vq,
if (unlikely(vq->num_added == (1 << 16) - 1))
virtqueue_kick(_vq);
+ {
+ if (unlikely(vq->split.avail_idx_shadow == 0))
+ virtqueue_kick_always(_vq);
+ }
+
return 0;
unmap_release:
@@ -2515,6 +2529,11 @@ bool virtqueue_kick(struct virtqueue *vq)
}
EXPORT_SYMBOL_GPL(virtqueue_kick);
+void virtqueue_kick_always(struct virtqueue *vq)
+{
+ virtqueue_kick_prepare(vq);
+ virtqueue_notify(vq);
+}
/**
* virtqueue_get_buf_ctx - get the next used buffer
* @_vq: the struct virtqueue we're talking about.
--------------- END OF LINUX KERNEL PATCH ----------
Then the kernel was booted, and the host pinged until the
nic became unresponsive:
ping -i 0.002 192.168.4.1
Device status was confirmed using
cat /sys/class/net/eth0/device/status
(it was 0x4f).
Then the device was re-initialized:
DEV_NAME=$(basename $(readlink -f /sys/class/net/eth0/device))
echo $DEV_NAME | tee /sys/bus/virtio/drivers/virtio_net/unbind
echo $DEV_NAME | tee /sys/bus/virtio/drivers/virtio_net/bind
ip link set eth0 up
At this point networking became healthly again.
Signed-off-by: Peter Oskolkov <posk@google.com>
This should be guaranteed by GuestMemory and GuestMemoryRegion, but
those traits are currently safe, so add checks to guard against
incorrect implementations of them.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
Consuming `&Arc<T>` as argument is almost always an antipattern as it
hides whether the callee is going to take over (shared) ownership
(by .clone()) or not. Instead, it is better to consume `&dyn T` or
`Arc<dyn T>` to be more explicit. This commit cleans up the code.
The change is very mechanic and was very easy to implement across the
code base.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Fix clippy warning `uninlined_format_args` reported by rustc rustc
1.89.0 (29483883e 2025-08-04).
```console
warning: variables can be used directly in the `format!` string
--> block/src/lib.rs:649:17
|
649 | info!("{} failed to create io_uring instance: {}", error_msg, e);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
= note: `#[warn(clippy::uninlined_format_args)]` on by default
help: change this to
|
649 - info!("{} failed to create io_uring instance: {}", error_msg, e);
649 + info!("{error_msg} failed to create io_uring instance: {e}");
|
```
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
This streamlines the code base to follow best practices for
error handling in Rust: Each error struct implements
std::error::Error (most due via thiserror::Error derive macro)
and sets its source accordingly.
This allows future work that nicely prints the error chains,
for example.
So far, the convention is that each error prints its
sub error as part of its Display::fmt() impl.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Replace `map_or()` on false condition with `is_some_and` to provide
better readability, as suggestted by v1.84.0-beta.1 `cargo clippy`.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
Historically the Cloud Hypervisor coding style has been to ensure that
all imports are ordered and placed in a single group. Unfortunately
cargo fmt has no support for ensuring that all imports are in a single
group so if whitespace lines were added as part of the import statements
then they would only be odered correctly in the group.
By adopting "group_imports="StdExternalCrate" we can enforce a style
where imports are placed in at most three groups for std, external
crates and the crate itself. Choosing a style enforceable by the tooling
reduces the reviewer burden.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Replace the specific Bitmap implementation from the type signature used
for functions that take memory. This allows more flexibility when using
these functions in particular when these functions are used by the
vhost-user-net backend. An updated vhost-user-backend crate requires
extra constraints on the Bitmap implementation used (it must support
BitmapReplace which AtomicBitmap does not.)
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Update to the latest vm-memory and all the crates that also depend upon
it.
Fix some deprecation warnings.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
warning: this argument is a mutable reference, but not used mutably
--> net_util/src/queue_pair.rs:167:14
|
167 | tap: &mut Tap,
| ^^^^^^^^ help: consider changing to: `&Tap`
|
= warning: changing this function will impact semver compatibility
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
warning: casting raw pointers to the same type and constness is unnecessary (`*const protocol::MemoryRange` -> `*const protocol::MemoryRange`)
--> vm-migration/src/protocol.rs:280:17
|
280 | self.data.as_ptr() as *const MemoryRange as *const u8,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `self.data.as_ptr()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
= note: `#[warn(clippy::unnecessary_cast)]` on by default
Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
This integer overflow was triggered with fuzzing on the virtio-net
device. The integer overflow is from the wrong assumption that the
packets read from or written to the tap device is always larger than the
size of a virtio-net header.
Signed-off-by: Bo Chen <chen.bo@intel.com>
This panic was triggered with fuzzing on the virtio-net device. This
commits handles the error explicitly to avoid the panic, which also
makes the fuzzer happy (as panic is treated as bugs).
Signed-off-by: Bo Chen <chen.bo@intel.com>
Considering error messages will be mostly nested, ensuring no
punctuation at the end will make the error log more readable.
Signed-off-by: Bo Chen <chen.bo@intel.com>
Now that we rely on pop_descriptor_chain() rather than iter() to iterate
over a queue, there's no more borrow on the queue itself, meaning we can
invoke add_used() directly for the iteration loop. This simplifies the
processing of the queues for each virtio device, and bring some possible
performance improvement given we don't have to iterate twice over the
list of descriptors to invoke add_used().
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
Using pop_descriptor_chain() is much more appropriate than iter() since
it recreates the iterator every time, avoiding the queue to be borrowed
and allowing the virtio-net implementation to match all the other ones.
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
The new virtio-queue version introduced some breaking changes which need
to be addressed so that Cloud Hypervisor can still work with this
version.
The most important change is about removing a handle to the guest memory
from the Queue, meaning the caller has to provide the guest memory
handle for multiple methods from the QueueT trait.
One interesting aspect is that QueueT has been widely extended to
provide every getter and setter we need to access and update the Queue
structure without having direct access to its internal fields.
This patch ports all the virtio and vhost-user devices to this new crate
definition. It also updates both vhost-user-block and vhost-user-net
backends based on the updated vhost-user-backend crate. It also updates
the fuzz directory.
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
Renaming translate() to translate_gva() to clarify we want to translate
a GVA address into a GPA.
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
This indicates if anything has been added to the available queue since
it was last iterated through. If it returns true then it is necessary to
iterate through the queue again otherwise it is appropriate to break out
from the loop.
Signed-off-by: Rob Bradford <robert.bradford@intel.com>
This new trait simplifies the address translation of a GuestAddress by
having GuestAddress implementing it.
The three crates virtio-devices, block_util and net_util have been
updated accordingly to rely on this new trait, helping with code
readability and limiting the amount of duplicated code.
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
Moving the whole codebase to rely on the AccessPlatform definition from
vm-virtio so that we can fully remove it from virtio-queue crate.
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
Since we're trying to move away from the translation happening in the
virtio-queue crate, the device itself is performing the address
translation when needed.
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
Relying on the vm-virtio/virtio-queue crate from rust-vmm which has been
copied inside the Cloud Hypervisor tree, the entire codebase is moved to
the new definition of a Queue and other related structures.
The reason for this move is to follow the upstream until we get some
agreement for the patches that we need on top of that to make it
properly work with Cloud Hypervisor.
Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
The TX rate limiter didn't correctly handle the `-EAGAIN` situation
where it should replenish the rate-limiter tokens. This patch simplifies
the TX rate-limiter code path by following the similar trade-off made in
the TX rate limiter, where we always let the `last` descriptor
go-through (even if it was over the rate limit. We simply stop
processing the oncoming `descriptors` if any.
Fixes: #2817
Signed-off-by: Bo Chen <chen.bo@intel.com>
This warning isn't present on on the read case and we now have better
handling of the -EAGAIN situation including retries.
Signed-off-by: Rob Bradford <robert.bradford@intel.com>
If writing to the TAP returns EAGAIN then listen for the TAP to be
writable. When the TAP becomes writable attempt to process the TX queue
again.
Fixes: #2807
Signed-off-by: Rob Bradford <robert.bradford@intel.com>
Issue from beta verion of clippy:
Error: --> vm-virtio/src/queue.rs:700:59
|
700 | if let Some(used_event) = self.get_used_event(&mem) {
| ^^^^ help: change this to: `mem`
|
= note: `-D clippy::needless-borrow` implied by `-D warnings`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
Signed-off-by: Bo Chen <chen.bo@intel.com>
As the first step to complete live-migration with tracking dirty-pages
written by the VMM, this commit patches the dependent vm-memory crate to
the upstream version with the dirty-page-tracking capability. Most
changes are due to the updated `GuestMemoryMmap`, `GuestRegionMmap`, and
`MmapRegion` structs which are taking an additional generic type
parameter to specify what 'bitmap backend' is used.
The above changes should be transparent to the rest of the code base,
e.g. all unit/integration tests should pass without additional changes.
Signed-off-by: Bo Chen <chen.bo@intel.com>
If the tap file descriptor is not writable then try again later. Update
the RX side to match the test on std::io::ErrorKind::WouldBlock
Fixes: #2517
Signed-off-by: Rob Bradford <robert.bradford@intel.com>
