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>
Callers of get_host_address_range() rely on it returning a pointer to at
least size bytes of memory. mem.get_host_address() is an overrideable
method of a safe trait, so it is better for safe code to not rely on its
correctness for safety. Instead, use mem.get_slice(), which returns a
VolatileSlice whose invariants guarantee that it points to a sufficient
amount of memory. If mem.check_range() succeeds but mem.get_slice()
returns a slice that is too small, this means that there is either a
logic error or a situation the code cannot support yet, so panic.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
This API passes a u64 to a kernel API that treats the u64 as a userspace
address. Therefore, it should be marked unsafe, but it currently is not
[1]. Wrap the call in an unsafe block to document that invariants must
be upheld to avoid undefined behavior. This causes a compiler warning,
so suppress the warning with #[allow(unused_unsafe)].
[1]: https://github.com/rust-vmm/vfio/issues/100
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
I believe that its only caller used it safely, but it is still better to
mark the code as unsafe. Also add additional validity checks.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
To ensure that struct sizes are the same on 32-bit and 64-bit, various
kernel APIs use __u64 (Rust u64) to represent userspace pointers.
Userspace is expected to cast pointers to __u64 before passing them to
the kernel, and cast kernel-provided __u64 to a pointer before using
them. However, various safe APIs in Cloud Hypervisor took
caller-provided u64 values and passed them to syscalls that interpret
them as userspace addresses. Therefore, passing bad u64 values would
cause memory disclosure or corruption.
Fix the bug by using usize and pointer types as appropriate. To make
soundness of the code easier to reason about, the PCI code gains a new
MmapRegion abstraction that ensures the validity of pointers. The rest
of the code already has an MmapRegion abstraction it can use. To avoid
having to reason about whether something is keeping the MmapRegion
alive, reference counting is added. MmapRegion cannot hold references
to other objects, so the reference counting cannot introduce cycles.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
This better aligns with the rest of the code and makes it clearer
that these tests can run "as is" in a normal hosted environments
without the special test environment.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
A major improvement to the developer experience of clippy in
Cloud Hypervisor.
1. Make `cargo clippy` just work with the same lints we use in CI
2. Simplify adding new lints
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This would be a good opportunity to optimize another pointless vector
away, but I don't have a good way to test this at the moment. But
maybe someone else gives it a shot.
On-behalf-of: SAP julian.stecklina@sap.com
Signed-off-by: Julian Stecklina <julian.stecklina@cyberus-technology.de>
as of rust 1.90, writes to unix socket streams use send_with_flags
instead of write, so it uses a sendto syscall instead of write.
Signed-off-by: Matt Moriarity <matt@mattmoriarity.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
The virtio spec defines the feature bit range and 4 x 32-bit pages.
However no features are currently defined with bits > 63. However Linux
has now started acking features in those higher pages. Since this is
harmless (we drop the acks to those feature pages) and quietly return 0
for available features in those higher pages this warning can be
removed.
Fixes: #7414
Signed-off-by: Rob Bradford <rbradford@rivosinc.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>
The function `read_local_stream_port` had no proper handling for
unexpected or incomplete input.
When the control socket of the VSOCK device was closed without sending
the expected `CONNECT <PORT>\n` statement completely, the thread
got stuck in an infinite loop as it attempted to read from a closed
socket over and over again which never returned any data.
This resulted in the thread responsible for `epoll` being completely
blocked. New VSOCK connections could not be established and existing
ones became defunct, effectively leading to a Denial of Service of
the entire VSOCK device.
The issue can be reproduced by opening a socket and immediately
closing it.
```
socat - UNIX-CONNECT:/socket.vsock
<Ctrl-C>
```
Instead of applying a quick fix by handling the `EPOLLHUP` event before
reading, the function is refactored to remove the error-prone `while`
loop and multiple `read`s.
Notably, we now check if the number of bytes read is zero, which occurs
when `event_set == EPOLLHUP | EPOLLIN`, indicating that the socket has
been closed by the client.
Additionally, the actual parsing code is now extracted into a dedicated
function that is tested.
Fixes: #6798
Signed-off-by: Maximilian Güntner <code@mguentner.de>
These can differ between platforms, so it's better to use centralized
definitions of them. We can't currently do this for the KVM and VFIO
ioctls, because the corresponding crates don't publicly expose the
ioctl numbers.
Signed-off-by: Alyssa Ross <hi@alyssa.is>
Cache and batch IO requests after parsing all
items in the queue, improving performance—especially
for small block sizes—by reducing per-request overhead.
Introduced two methods in the AsyncIo trait for batch
submission, with implementation in the raw disk backend.
This method should be called during/after parsing all block IO requests
in the available queue. If the batch submission is not enabled, by
default it does the old way of submitting requests.
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Instead of returning boolean return an struct of completion status
so that it can be cached for batch submission.
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Signed-off-by: Muminul Islam <muislam@microsoft.com>
This patch changes the read-only check using acked features bit, which
will help to check more features.
Signed-off-by: Songqian Li <sionli@tencent.com>
Move UserspaceMapping to vm-device to avoid redefinition since
UserspaceMapping is used by both `virtio-devices` and `device`
crate.
Signed-off-by: Songqian Li <sionli@tencent.com>
Many of the workspace members in the Cloud-hypervisor workspace share
common dependencies. Making these workspace dependencies reduces
duplication and improves maintainability.
Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de>
On-behalf-of: SAP oliver.anderson@sap.com
The read_exact() call was introduced in 82ac114b8 ("virtio-devices:
vsock: handle short read in muxer") to solve a crash when a connection
disconnected without sending any data, but it introduced a problem of
its own: because the socket is non-blocking, read_exact() may read
some data, then return ErrorKind::WouldBlock. In that case, the data
it read will be discarded. So for example if it read "CONNECT ",
and then nothing else was available to read yet, "CONNECT " would be
discarded, and so the next time this function was called, when epoll
triggered again for the socket, only the following data would end up
in command.buf, causing an error due to just a port number being an
invalid command.
Contrary to that commit message, this code was actually designed to
handle short reads just fine — in the case of a short read, it stores
the data it has read in command, and returns
Error::UnixRead(ErrorKind::WouldBlock), which is ignored by the
caller, and the function gets called again when there is more data to
read, building up command potentially over the course of several
reads. The only thing it didn't handle correctly, as far as I can
tell, was a 0-byte read, which happens when a client disconnects from
the socket without writing anything. All that's needed to fix this is
to avoid an invalid subtraction in that case, so this change reverts
82ac114b8, fixing the issue with partial commands being discarded, and
instead handles the 0-byte read by using slice::get, and treating an
empty command as an incomplete command, which of course it is.
Fixes: 82ac114b8 ("virtio-devices: vsock: handle short read in muxer")
Signed-off-by: Alyssa Ross <hi@alyssa.is>
This makes it possible to run cargo test just for the virtio-devices
crate (as long as either KVM or MSHV is specified).
Signed-off-by: Alyssa Ross <hi@alyssa.is>
Recently vfio crates have moved to crates.io, thus we should start
consuming the crate from crates.io instead git url.
This results in better versioning instead of tracking some git commit
sha.
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
Instead of exiting on IO errors, report the errors to the guest with
VIRTIO_BLK_S_IOERR. For example, the guest kernel will log something
similar to this if the nbd behind /dev/vdc is unexpectedly disconnected:
[ 166.033957] I/O error, dev vdc, sector 264 op 0x1:(WRITE) flags 0x9800 phys_seg 1 prio class 2
[ 166.035083] Aborting journal on device vdc-8.
[ 166.037307] Buffer I/O error on dev vdc, logical block 9, lost sync page write
[ 166.038471] JBD2: I/O error when updating journal superblock for vdc-8.
[...]
[ 174.234470] EXT4-fs (vdc): I/O error while writing superblock
In case the rootfs is not located on the affected block device, this
will not crash the guest.
Fixes: #6995
Signed-off-by: Gauthier Jolly <contact@gjolly.fr>
This was caught by the nightly compiler during cargo fuzz build.
error: lifetime flowing from input to output with different syntax can be confusing
--> /home/runner/work/cloud-hypervisor/cloud-hypervisor/hypervisor/src/arch/x86/emulator/mod.rs:493:26
|
493 | pub fn new(platform: &mut dyn PlatformEmulator<CpuState = T>) -> Emulator<T> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ----------- the lifetime gets resolved as `'_`
| |
| this lifetime flows to the output
|
= note: `-D mismatched-lifetime-syntaxes` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(mismatched_lifetime_syntaxes)]`
help: one option is to remove the lifetime for references and use the anonymous lifetime for paths
|
493 | pub fn new(platform: &mut dyn PlatformEmulator<CpuState = T>) -> Emulator<'_, T> {
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
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
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
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
Allow tap interfaces to be configured with an IPv6 address. The change
is fairly straightforward: we need to update the API types and CLI
parsing to accept either an IPv6 or IPv4 and then match on the IP
address type when the tap device is configured.
For IPv6 addresses, the netmask (prefix) must be provided at the same
time as the address itself (in the SIOCSIFADDR ioctl). They cannot be
configured separately. So we remove the separate "set_netmask" function
and convert "set_ip_addr" to also accept a netmask. For IPv4 addresses,
the IP address and netmask were already always set together, so this
should have no functional impact for users of IPv4 addresses.
Signed-off-by: Gregory Anders <ganders@cloudflare.com>
# What
This commit introduces file-based advisory locking for the files backing
up the block devices by using the fcntl() syscall with OFD locks. The
per-open-file-descriptor (OFD) locks are more robust than traditional
POSIX locks (F_SETLK) as they are not tied to process IDs and avoid
common issues in multithreaded or multi-fd scenarios [1]. Therefore,
we don't use `std::fs::File::try_lock()`, which is backed by F_SETLKW.
The locking mechanism is aware of the `readonly` property and allows
`n` readers or `1` writer (exclusive mode).
As the locks are advisory, multiple cloud-hypervisor processes can
prevent themselves from writing to the same file. However, this is not
a system-wide file-system level locking mechanism preventing to open()
a file.
The introduced new locking mechanism does not cover vhost-user devices.
# Why
To prevent misconfiguration and improve safety, it is good practice to
protect disk image files with a locking mechanism. Experience and common
best practices suggest that advisory locks are preferable over mandatory
locks due to better compatibility and fewer pitfalls (in fs space).
The introduced functionality is aligned with the approach taken by
QEMU [0], and is also recommended in [1].
# Implementation Details
We need to ensure that not only normal operation keeps working but also
state save/resume and live-migration. Especially for live migration,
it is crucial that the sender VMM releases the locks when the VM stops
so the receiver VMM can acquire them right after that.
Therefore, the locking and releasing happen directly on the block
device struct. The device manager knows all block devices and can
forward requests to these types.
Last but not least, this commit uses on explicit lock acquiring
but implicit lock releasing (FD close). It only explicitly releases
the locks where this integrates more smoothly into the existing
code.
# Testing
I tested
- normal operation
- state save/resume,
- device hot plugging,
- and live-migration
with read/shared and write/exclusive locks.
One can use the `fcntl-tool` to test if locks are actually acquired
or released [2].
# Links
[0] 825b96dbce/util/osdep.c (L266)
[1] https://apenwarr.ca/log/20101213
[2] https://crates.io/crates/fcntl-tool
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
The Memory Space Enable (MSE) bit from the COMMAND register in the
PCI configuration space controls whether a PCI device responds to memory
space accesses, e.g. read and write cycles to the device MMIO regions
defined by its BARs. The MSE bit is used by the device drivers to ensure
the correctness of BAR reprogramming. A common workflow is, the driver
first clears the MSE bit, then writes new values to the BAR registers,
and finally set the MSE bit to finish the BAR reprogramming.
This patch changes how we handle BAR reprogramming for all PCI
devices (e.g. virtio-pci, vfio, vfio-user, etc.), so that we follow the
same convention, e.g. moving PCI BARs when its MSE bit is set.
Note that some device drivers (such as edk2) only clear and set MSE once
while reprogramming multiple BARs of a single device. To support such
behavior, this patch adds support for multiple pending BAR reprogramming.
See: https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7027#issuecomment-2853642959
Signed-off-by: Bo Chen <bchen@crusoe.ai>
A BAR reprogramming of a PCI device will only happen when the (guest)
kernel write to its PCI config space, e.g. the detection of bar
reprogramming (`detect_bar_repgraomming()`) can be embedded to the PCI
config space write (`write_config_register()`). It simplifies APIs
exposed by the `struct PciConfiguration` and `trait PciDevice`. It also
prepares for easier handling of pending bar reprogramming when the MSE
bit of the COMMAND register is not enabled at the time of changing BAR
registers.
See: https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7027#issuecomment-2853642959
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Rust has a new way of constructing other error and clippy complains if
we are still using the older way to construct error message. Thus,
migrate to the new approach suggested by the clippy.
Warning from beta compiler:
error: this can be `std::io::Error::other(_)`
--> block/src/vhdx/mod.rs:142:17
|
| / std::io::Error::new(
| | std::io::ErrorKind::Other,
| | format!("Failed to update VHDx header: {e}"),
| | )
| |_________________^
|
= help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#io_other_error
help: use `std::io::Error::other`
std::io::Error::other(
format!("Failed to update VHDx header: {e}"),
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
`serde_json` crate is referenced by multiple components, centralize it
to workspace to better manage this crate.
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
