vhost_user: enforce ByteValued for recv-d structs

Converting arbitrary bytes into an arbitrary Rust value is unsafe.
For example, it's unsafe to create a String that isn't valid UTF-8.
But the various internal recv* functions didn't restrict their return
types enough to enforce this invariant, making them unsafe without
being properly marked.

To fix this, we tighten up the bounds of the functions to enforce that
their return types are ByteValued, meaning that they can only be used
to create types that are safe to initialize with arbitrary data such
as might be received over a socket.

It's worth asking how these functions could have been unsafe in the
first place, since they don't contain any unsafe blocks themselves.
The answer is that the functions that recv into iovecs are also unsafe
but not correctly marked.  I'm preparing further patches to fix that
up, but it's a lot of work so I've separated out this change in the
hope of getting it in first and making the diff for the next one
smaller.

This internal tightening shouldn't result in any public API changes.

Signed-off-by: Alyssa Ross <hi@alyssa.is>

Cargo.toml

@@ -13,7 +13,7 @@ edition = "2018"
 [features]
 default = []
 vhost-vsock = []
-vhost-kern = ["vm-memory"]
+vhost-kern = []
 vhost-user = []
 vhost-user-master = ["vhost-user"]
 vhost-user-slave = ["vhost-user"]
@@ -23,7 +23,7 @@ bitflags = ">=1.0.1"
 libc = ">=0.2.39"
 
 vmm-sys-util = ">=0.3.1"
-vm-memory = { version = "0.6", optional = true }
+vm-memory = "0.6"
 
 [dev-dependencies]
 tempfile = ">=3.2.0"

src/vhost_user/connection.rs

@@ -14,6 +14,7 @@ use std::path::{Path, PathBuf};
 use std::{mem, slice};
 
 use libc::{c_void, iovec};
+use vm_memory::ByteValued;
 use vmm_sys_util::sock_ctrl_msg::ScmSocket;
 
 use super::message::*;
@@ -467,7 +468,7 @@ impl<R: Req> Endpoint<R> {
     /// * - SocketError: other socket related errors.
     /// * - PartialMessage: received a partial message.
     /// * - InvalidMessage: received a invalid message.
-    pub fn recv_body<T: Sized + Default + VhostUserMsgValidator>(
+    pub fn recv_body<T: ByteValued + Sized + VhostUserMsgValidator>(
         &mut self,
     ) -> Result<(VhostUserMsgHeader<R>, T, Option<Vec<File>>)> {
         let mut hdr = VhostUserMsgHeader::default();
@@ -546,7 +547,7 @@ impl<R: Req> Endpoint<R> {
     /// * - PartialMessage: received a partial message.
     /// * - InvalidMessage: received a invalid message.
     #[cfg_attr(feature = "cargo-clippy", allow(clippy::type_complexity))]
-    pub fn recv_payload_into_buf<T: Sized + Default + VhostUserMsgValidator>(
+    pub fn recv_payload_into_buf<T: ByteValued + Sized + VhostUserMsgValidator>(
         &mut self,
         buf: &mut [u8],
     ) -> Result<(VhostUserMsgHeader<R>, T, usize, Option<Vec<File>>)> {

src/vhost_user/master.rs

@@ -10,6 +10,7 @@ use std::os::unix::net::UnixStream;
 use std::path::Path;
 use std::sync::{Arc, Mutex, MutexGuard};
 
+use vm_memory::ByteValued;
 use vmm_sys_util::eventfd::EventFd;
 
 use super::connection::Endpoint;
@@ -675,7 +676,7 @@ impl MasterInternal {
         Ok(hdr)
     }
 
-    fn recv_reply<T: Sized + Default + VhostUserMsgValidator>(
+    fn recv_reply<T: ByteValued + Sized + VhostUserMsgValidator>(
         &mut self,
         hdr: &VhostUserMsgHeader<MasterReq>,
     ) -> VhostUserResult<T> {
@@ -691,7 +692,7 @@ impl MasterInternal {
         Ok(body)
     }
 
-    fn recv_reply_with_files<T: Sized + Default + VhostUserMsgValidator>(
+    fn recv_reply_with_files<T: ByteValued + Sized + VhostUserMsgValidator>(
         &mut self,
         hdr: &VhostUserMsgHeader<MasterReq>,
     ) -> VhostUserResult<(T, Option<Vec<File>>)> {
@@ -707,7 +708,7 @@ impl MasterInternal {
         Ok((body, files))
     }
 
-    fn recv_reply_with_payload<T: Sized + Default + VhostUserMsgValidator>(
+    fn recv_reply_with_payload<T: ByteValued + Sized + VhostUserMsgValidator>(
         &mut self,
         hdr: &VhostUserMsgHeader<MasterReq>,
     ) -> VhostUserResult<(T, Vec<u8>, Option<Vec<File>>)> {

src/vhost_user/message.rs

@@ -12,6 +12,8 @@
 use std::fmt::Debug;
 use std::marker::PhantomData;
 
+use vm_memory::ByteValued;
+
 use crate::VringConfigData;
 
 /// The vhost-user specification uses a field of u32 to store message length.
@@ -412,7 +414,7 @@ bitflags! {
 
 /// A generic message to encapsulate a 64-bit value.
 #[repr(packed)]
-#[derive(Default)]
+#[derive(Copy, Clone, Default)]
 pub struct VhostUserU64 {
     /// The encapsulated 64-bit common value.
     pub value: u64,
@@ -425,6 +427,8 @@ impl VhostUserU64 {
     }
 }
 
+unsafe impl ByteValued for VhostUserU64 {}
+
 impl VhostUserMsgValidator for VhostUserU64 {}
 
 /// Memory region descriptor for the SET_MEM_TABLE request.
@@ -546,7 +550,7 @@ impl VhostUserMsgValidator for VhostUserSingleMemoryRegion {
 
 /// Vring state descriptor.
 #[repr(packed)]
-#[derive(Default)]
+#[derive(Copy, Clone, Default)]
 pub struct VhostUserVringState {
     /// Vring index.
     pub index: u32,
@@ -561,6 +565,8 @@ impl VhostUserVringState {
     }
 }
 
+unsafe impl ByteValued for VhostUserVringState {}
+
 impl VhostUserMsgValidator for VhostUserVringState {}
 
 // Bit mask for vring address flags.
@@ -655,7 +661,7 @@ bitflags! {
 
 /// Message to read/write device configuration space.
 #[repr(packed)]
-#[derive(Default)]
+#[derive(Copy, Clone, Default)]
 pub struct VhostUserConfig {
     /// Offset of virtio device's configuration space.
     pub offset: u32,
@@ -676,6 +682,8 @@ impl VhostUserConfig {
     }
 }
 
+unsafe impl ByteValued for VhostUserConfig {}
+
 impl VhostUserMsgValidator for VhostUserConfig {
     #[allow(clippy::if_same_then_else)]
     fn is_valid(&self) -> bool {
@@ -698,7 +706,7 @@ pub type VhostUserConfigPayload = Vec<u8>;
 /// Single memory region descriptor as payload for ADD_MEM_REG and REM_MEM_REG
 /// requests.
 #[repr(C)]
-#[derive(Default, Clone)]
+#[derive(Copy, Clone, Default)]
 pub struct VhostUserInflight {
     /// Size of the area to track inflight I/O.
     pub mmap_size: u64,
@@ -722,6 +730,8 @@ impl VhostUserInflight {
     }
 }
 
+unsafe impl ByteValued for VhostUserInflight {}
+
 impl VhostUserMsgValidator for VhostUserInflight {
     fn is_valid(&self) -> bool {
         if self.num_queues == 0 || self.queue_size == 0 {