From 451d3fb2f01b81c99580ee78ff35b376aba88138 Mon Sep 17 00:00:00 2001 From: Alyssa Ross Date: Wed, 10 Jan 2024 16:00:54 +0100 Subject: [PATCH] vmm: limit VSOCK CIDs to 32 bits The VIRTIO specification[1] says: > The upper 32 bits of the CID are reserved and zeroed. We should therefore not allow the user to supply a VSOCK CID with those bits set. To accomplish this, limit the public API of the virtio-vsock device to only accept 32-bit CIDs, while still using 64-bit CIDs internally since that's how virtio-vsock works. [1]: https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-4400004 Signed-off-by: Alyssa Ross --- virtio-devices/src/vsock/device.rs | 4 ++-- virtio-devices/src/vsock/mod.rs | 6 +++--- virtio-devices/src/vsock/unix/muxer.rs | 14 +++++++------- vmm/src/config.rs | 4 ++-- vmm/src/vm_config.rs | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/virtio-devices/src/vsock/device.rs b/virtio-devices/src/vsock/device.rs index fa793295c..8411cb71c 100644 --- a/virtio-devices/src/vsock/device.rs +++ b/virtio-devices/src/vsock/device.rs @@ -340,7 +340,7 @@ where #[allow(clippy::too_many_arguments)] pub fn new( id: String, - cid: u64, + cid: u32, path: PathBuf, backend: B, iommu: bool, @@ -372,7 +372,7 @@ where ..Default::default() }, id, - cid, + cid: cid.into(), backend: Arc::new(RwLock::new(backend)), path, seccomp_action, diff --git a/virtio-devices/src/vsock/mod.rs b/virtio-devices/src/vsock/mod.rs index b5c63a15b..cc77a2fd3 100644 --- a/virtio-devices/src/vsock/mod.rs +++ b/virtio-devices/src/vsock/mod.rs @@ -17,7 +17,7 @@ pub use self::device::Vsock; pub use self::unix::VsockUnixBackend; pub use self::unix::VsockUnixError; -pub use packet::VsockPacket; +use packet::VsockPacket; use std::os::unix::io::RawFd; mod defs { @@ -262,10 +262,10 @@ mod tests { impl TestContext { pub fn new() -> Self { - const CID: u64 = 52; + const CID: u32 = 52; const MEM_SIZE: usize = 1024 * 1024 * 128; Self { - cid: CID, + cid: CID as u64, mem: GuestMemoryMmap::from_ranges(&[(GuestAddress(0), MEM_SIZE)]).unwrap(), mem_size: MEM_SIZE, device: Vsock::new( diff --git a/virtio-devices/src/vsock/unix/muxer.rs b/virtio-devices/src/vsock/unix/muxer.rs index 0768beae9..591944fe3 100644 --- a/virtio-devices/src/vsock/unix/muxer.rs +++ b/virtio-devices/src/vsock/unix/muxer.rs @@ -347,7 +347,7 @@ impl VsockBackend for VsockMuxer {} impl VsockMuxer { /// Muxer constructor. /// - pub fn new(cid: u64, host_sock_path: String) -> Result { + pub fn new(cid: u32, host_sock_path: String) -> Result { // Create the nested epoll FD. This FD will be added to the VMM `EpollContext`, at // device activation time. let epoll_fd = epoll::create(true).map_err(Error::EpollFdCreate)?; @@ -362,7 +362,7 @@ impl VsockMuxer { .map_err(Error::UnixBind)?; let mut muxer = Self { - cid, + cid: cid.into(), host_sock, host_sock_path, epoll_file, @@ -862,7 +862,7 @@ mod tests { use super::super::super::tests::TestContext as VsockTestContext; use super::*; - const PEER_CID: u64 = 3; + const PEER_CID: u32 = 3; const PEER_BUF_ALLOC: u32 = 64 * 1024; struct MuxerTestContext { @@ -906,7 +906,7 @@ mod tests { } self.pkt .set_type(uapi::VSOCK_TYPE_STREAM) - .set_src_cid(PEER_CID) + .set_src_cid(PEER_CID.into()) .set_dst_cid(uapi::VSOCK_HOST_CID) .set_src_port(peer_port) .set_dst_port(local_port) @@ -1060,7 +1060,7 @@ mod tests { ctx.recv(); assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RST); assert_eq!(ctx.pkt.src_cid(), uapi::VSOCK_HOST_CID); - assert_eq!(ctx.pkt.dst_cid(), PEER_CID); + assert_eq!(ctx.pkt.dst_cid(), PEER_CID as u64); assert_eq!(ctx.pkt.src_port(), LOCAL_PORT); assert_eq!(ctx.pkt.dst_port(), PEER_PORT); @@ -1105,7 +1105,7 @@ mod tests { assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RST); assert_eq!(ctx.pkt.len(), 0); assert_eq!(ctx.pkt.src_cid(), uapi::VSOCK_HOST_CID); - assert_eq!(ctx.pkt.dst_cid(), PEER_CID); + assert_eq!(ctx.pkt.dst_cid(), PEER_CID as u64); assert_eq!(ctx.pkt.src_port(), LOCAL_PORT); assert_eq!(ctx.pkt.dst_port(), PEER_PORT); @@ -1119,7 +1119,7 @@ mod tests { assert_eq!(ctx.pkt.op(), uapi::VSOCK_OP_RESPONSE); assert_eq!(ctx.pkt.len(), 0); assert_eq!(ctx.pkt.src_cid(), uapi::VSOCK_HOST_CID); - assert_eq!(ctx.pkt.dst_cid(), PEER_CID); + assert_eq!(ctx.pkt.dst_cid(), PEER_CID as u64); assert_eq!(ctx.pkt.src_port(), LOCAL_PORT); assert_eq!(ctx.pkt.dst_port(), PEER_PORT); let key = ConnMapKey { diff --git a/vmm/src/config.rs b/vmm/src/config.rs index aa6f476a3..81de491eb 100644 --- a/vmm/src/config.rs +++ b/vmm/src/config.rs @@ -156,7 +156,7 @@ pub enum ValidationError { /// Need shared memory for vfio-user UserDevicesRequireSharedMemory, /// VSOCK Context Identifier has a special meaning, unsuitable for a VM. - VsockSpecialCid(u64), + VsockSpecialCid(u32), /// Memory zone is reused across NUMA nodes MemoryZoneReused(String, u32, u32), /// Invalid number of PCI segments @@ -2199,7 +2199,7 @@ impl VmConfig { } if let Some(vsock) = &self.vsock { - if [u32::MAX as u64, 0, 1, 2].contains(&vsock.cid) { + if [!0, 0, 1, 2].contains(&vsock.cid) { return Err(ValidationError::VsockSpecialCid(vsock.cid)); } } diff --git a/vmm/src/vm_config.rs b/vmm/src/vm_config.rs index 453b2e938..1b286845e 100644 --- a/vmm/src/vm_config.rs +++ b/vmm/src/vm_config.rs @@ -521,7 +521,7 @@ pub fn default_vdpaconfig_num_queues() -> usize { #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Default)] pub struct VsockConfig { - pub cid: u64, + pub cid: u32, pub socket: PathBuf, #[serde(default)] pub iommu: bool,