dsg/cloud-hypervisor
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

virtio-devices: copy VSock header from guest

Browse source 
VsockPacket::hdr holds a raw pointer to the address of the VSock packet
header, which is in guest memory. It opens the door to double-fetch
(or TOCTOU) race conditions. Therefore, VSockPacket::hdr content can't
be trusted since it can be arbitrarily changed by the guest, at any
time.

To mitigate this, we can copy the header content to an array in VMM's
memory that the guest can't modify.

Signed-off-by: Thomas Leroy <thomas.leroy.mp@gmail.com>
This commit is contained in:
Thomas Leroy 2025-12-01 15:22:59 +00:00 • committed by Rob Bradford
parent 87e8ac3f1f
commit 929df76e1a
2 changed files with 65 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

11
virtio-devices/src/vsock/device.rs
View file

@ -130,7 +130,16 @@ where
) {
Ok(mut pkt) => {
if self.backend.write().unwrap().recv_pkt(&mut pkt).is_ok() {
pkt.hdr().len() as u32 + pkt.len()
match pkt.commit_hdr(&*self.mem.memory()) {
Ok(()) => pkt.hdr().len() as u32 + pkt.len(),
Err(err) => {
warn!(
"vsock: Error writing packet header to guest memory: \
{err:?}. Discarding the package."
);
0
}
}
} else {
// We are using a consuming iterator over the virtio buffers, so, if we can't
// fill in this buffer, we'll need to undo the last iterator step.

84
virtio-devices/src/vsock/packet.rs
View file

@ -20,7 +20,7 @@ use std::ops::Deref;
use byteorder::{ByteOrder, LittleEndian};
use virtio_queue::DescriptorChain;
use vm_memory::{Address, GuestMemory};
use vm_memory::{Address, Bytes, GuestAddress, GuestMemory};
use vm_virtio::{AccessPlatform, Translatable};
use super::{Result, VsockError, defs};
@ -96,7 +96,10 @@ const HDROFF_FWD_CNT: usize = 40;
/// - (an optional) data/buffer descriptor, only present for data packets (VSOCK_OP_RW).
///
pub struct VsockPacket {
hdr: *mut u8,
// We still hold the header address in guest memory. We need to write back the modified
// header in RX buffers.
guest_hdr_addr: GuestAddress,
hdr: [u8; VSOCK_PKT_HDR_SIZE],
buf: Option<*mut u8>,
buf_size: usize,
}
@ -129,14 +132,23 @@ impl VsockPacket {
return Err(VsockError::HdrDescTooSmall(head.len()));
}
let guest_hdr_addr = head
.addr()
.translate_gva(access_platform, VSOCK_PKT_HDR_SIZE);
// To avoid TOCTOU issues when reading/writing the VSock packet header in guest memory,
// we need to copy the content of the header in the VMM's memory.
// After the copy, the hdr content can be trusted since the guest can't change its
// content anymore.
let mut hdr = [0u8; VSOCK_PKT_HDR_SIZE];
desc_chain
.memory()
.read_slice(hdr.as_mut_slice(), guest_hdr_addr)
.map_err(|_| VsockError::GuestMemory)?;
let mut pkt = Self {
hdr: get_host_address_range(
desc_chain.memory(),
head.addr()
.translate_gva(access_platform, VSOCK_PKT_HDR_SIZE),
VSOCK_PKT_HDR_SIZE,
)
.ok_or(VsockError::GuestMemory)?,
guest_hdr_addr,
hdr,
buf: None,
buf_size: 0,
};
@ -221,19 +233,28 @@ impl VsockPacket {
return Err(VsockError::HdrDescTooSmall(head.len()));
}
let guest_hdr_addr = head
.addr()
.translate_gva(access_platform, VSOCK_PKT_HDR_SIZE);
// To avoid TOCTOU issues when reading/writing the VSock packet header in guest memory,
// we need to copy the content of the header in the VMM's memory.
// After the copy, the hdr content can be trusted since the guest can't change its
// content anymore.
let mut hdr = [0u8; VSOCK_PKT_HDR_SIZE];
desc_chain
.memory()
.read_slice(hdr.as_mut_slice(), guest_hdr_addr)
.map_err(|_| VsockError::GuestMemory)?;
// Prior to Linux v6.3 there are two descriptors
if head.has_next() {
let buf_desc = desc_chain.next().ok_or(VsockError::BufDescMissing)?;
let buf_size = buf_desc.len() as usize;
Ok(Self {
hdr: get_host_address_range(
desc_chain.memory(),
head.addr()
.translate_gva(access_platform, VSOCK_PKT_HDR_SIZE),
VSOCK_PKT_HDR_SIZE,
)
.ok_or(VsockError::GuestMemory)?,
guest_hdr_addr,
hdr,
buf: Some(
get_host_address_range(
desc_chain.memory(),
@ -247,13 +268,8 @@ impl VsockPacket {
} else {
let buf_size: usize = head.len() as usize - VSOCK_PKT_HDR_SIZE;
Ok(Self {
hdr: get_host_address_range(
desc_chain.memory(),
head.addr()
.translate_gva(access_platform, VSOCK_PKT_HDR_SIZE),
VSOCK_PKT_HDR_SIZE,
)
.ok_or(VsockError::GuestMemory)?,
guest_hdr_addr,
hdr,
buf: Some(
get_host_address_range(
desc_chain.memory(),
@ -273,17 +289,27 @@ impl VsockPacket {
/// Provides in-place, byte-slice, access to the vsock packet header.
///
pub fn hdr(&self) -> &[u8] {
// SAFETY: bound checks have already been performed when creating the packet
// from the virtq descriptor.
unsafe { std::slice::from_raw_parts(self.hdr as *const u8, VSOCK_PKT_HDR_SIZE) }
self.hdr.as_slice()
}
/// Provides in-place, byte-slice, mutable access to the vsock packet header.
///
pub fn hdr_mut(&mut self) -> &mut [u8] {
// SAFETY: bound checks have already been performed when creating the packet
// from the virtq descriptor.
unsafe { std::slice::from_raw_parts_mut(self.hdr, VSOCK_PKT_HDR_SIZE) }
self.hdr.as_mut_slice()
}
/// Writes the local copy of the packet header to the guest memory.
///
pub fn commit_hdr<M: GuestMemory>(&mut self, guest_mem: &M) -> Result<()> {
if self.len() as usize > defs::MAX_PKT_BUF_SIZE {
return Err(VsockError::InvalidPktLen(self.len()));
}
guest_mem
.write(self.hdr(), self.guest_hdr_addr)
.map_err(|_| VsockError::GuestMemory)?;
Ok(())
}
/// Provides in-place, byte-slice access to the vsock packet data buffer.