vhost: add safety comments on unsafe blocks

In rust-vmm-ci we are enabling `clippy::undocumented_unsafe_blocks` as
errors, so let's comment all unsafe blocks to avoid failures in the
CI.

Signed-off-by: Stefano Garzarella <[email protected]>

Author: stefano-garzarella

crates/vhost-user-backend/src/vring.rs

@@ -515,6 +515,7 @@ mod tests {
         assert!(vring.get_ref().enabled);
 
         let eventfd = EventFd::new(0).unwrap();
+        // SAFETY: Safe because we panic before if eventfd is not valid.
         let file = unsafe { File::from_raw_fd(eventfd.as_raw_fd()) };
         assert!(vring.get_mut().kick.is_none());
         assert!(vring.read_kick().unwrap());
@@ -527,6 +528,7 @@ mod tests {
         std::mem::forget(eventfd);
 
         let eventfd = EventFd::new(0).unwrap();
+        // SAFETY: Safe because we panic before if eventfd is not valid.
         let file = unsafe { File::from_raw_fd(eventfd.as_raw_fd()) };
         assert!(vring.get_ref().call.is_none());
         vring.set_call(Some(file));
@@ -536,6 +538,7 @@ mod tests {
         std::mem::forget(eventfd);
 
         let eventfd = EventFd::new(0).unwrap();
+        // SAFETY: Safe because we panic before if eventfd is not valid.
         let file = unsafe { File::from_raw_fd(eventfd.as_raw_fd()) };
         assert!(vring.get_ref().err.is_none());
         vring.set_err(Some(file));

crates/vhost-user-backend/tests/vhost-user-server.rs

@@ -153,6 +153,7 @@ fn vhost_user_client(path: &Path, barrier: Arc<Barrier>) {
         nix::sys::memfd::MemFdCreateFlag::empty(),
     )
     .unwrap();
+    // SAFETY: Safe because we panic before if memfd is not valid.
     let file = unsafe { File::from_raw_fd(memfd) };
     file.set_len(0x100000).unwrap();
     let file_offset = FileOffset::new(file, 0);

crates/vhost/src/vhost_kern/mod.rs

@@ -102,7 +102,7 @@ impl<T: VhostKernBackend> VhostBackend for T {
     /// Get a bitmask of supported virtio/vhost features.
     fn get_features(&self) -> Result<u64> {
         let mut avail_features: u64 = 0;
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_mut_ref(self, VHOST_GET_FEATURES(), &mut avail_features) };
         ioctl_result(ret, avail_features)
     }
@@ -113,21 +113,21 @@ impl<T: VhostKernBackend> VhostBackend for T {
     /// # Arguments
     /// * `features` - Bitmask of features to set.
     fn set_features(&self, features: u64) -> Result<()> {
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_FEATURES(), &features) };
         ioctl_result(ret, ())
     }
 
     /// Set the current process as the owner of this file descriptor.
     /// This must be run before any other vhost ioctls.
     fn set_owner(&self) -> Result<()> {
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl(self, VHOST_SET_OWNER()) };
         ioctl_result(ret, ())
     }
 
     fn reset_owner(&self) -> Result<()> {
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl(self, VHOST_RESET_OWNER()) };
         ioctl_result(ret, ())
     }
@@ -151,7 +151,7 @@ impl<T: VhostKernBackend> VhostBackend for T {
             )?;
         }
 
-        // This ioctl is called with a pointer that is valid for the lifetime
+        // SAFETY: This ioctl is called with a pointer that is valid for the lifetime
         // of this function. The kernel will make its own copy of the memory
         // tables. As always, check the return value.
         let ret = unsafe { ioctl_with_ptr(self, VHOST_SET_MEM_TABLE(), vhost_memory.as_ptr()) };
@@ -167,15 +167,15 @@ impl<T: VhostKernBackend> VhostBackend for T {
             return Err(Error::LogAddress);
         }
 
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_LOG_BASE(), &base) };
         ioctl_result(ret, ())
     }
 
     /// Specify an eventfd file descriptor to signal on log write.
     fn set_log_fd(&self, fd: RawFd) -> Result<()> {
-        // This ioctl is called on a valid vhost fd and has its return value checked.
         let val: i32 = fd;
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_LOG_FD(), &val) };
         ioctl_result(ret, ())
     }
@@ -191,7 +191,7 @@ impl<T: VhostKernBackend> VhostBackend for T {
             num: u32::from(num),
         };
 
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_VRING_NUM(), &vring_state) };
         ioctl_result(ret, ())
     }
@@ -210,7 +210,7 @@ impl<T: VhostKernBackend> VhostBackend for T {
         // The addresses are converted into the host address space.
         let vring_addr = config_data.to_vhost_vring_addr(queue_index, self.mem())?;
 
-        // This ioctl is called on a valid vhost fd and has its
+        // SAFETY: This ioctl is called on a valid vhost fd and has its
         // return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_VRING_ADDR(), &vring_addr) };
         ioctl_result(ret, ())
@@ -227,7 +227,7 @@ impl<T: VhostKernBackend> VhostBackend for T {
             num: u32::from(base),
         };
 
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_VRING_BASE(), &vring_state) };
         ioctl_result(ret, ())
     }
@@ -238,7 +238,7 @@ impl<T: VhostKernBackend> VhostBackend for T {
             index: queue_index as u32,
             num: 0,
         };
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_GET_VRING_BASE(), &vring_state) };
         ioctl_result(ret, vring_state.num)
     }
@@ -254,7 +254,7 @@ impl<T: VhostKernBackend> VhostBackend for T {
             fd: fd.as_raw_fd(),
         };
 
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_VRING_CALL(), &vring_file) };
         ioctl_result(ret, ())
     }
@@ -271,7 +271,7 @@ impl<T: VhostKernBackend> VhostBackend for T {
             fd: fd.as_raw_fd(),
         };
 
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_VRING_KICK(), &vring_file) };
         ioctl_result(ret, ())
     }
@@ -287,7 +287,7 @@ impl<T: VhostKernBackend> VhostBackend for T {
             fd: fd.as_raw_fd(),
         };
 
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_VRING_ERR(), &vring_file) };
         ioctl_result(ret, ())
     }
@@ -304,8 +304,9 @@ pub trait VhostKernFeatures: Sized + AsRawFd {
     /// Get a bitmask of supported vhost backend features.
     fn get_backend_features(&self) -> Result<u64> {
         let mut avail_features: u64 = 0;
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+
         let ret =
+            // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
             unsafe { ioctl_with_mut_ref(self, VHOST_GET_BACKEND_FEATURES(), &mut avail_features) };
         ioctl_result(ret, avail_features)
     }
@@ -316,7 +317,7 @@ pub trait VhostKernFeatures: Sized + AsRawFd {
     /// # Arguments
     /// * `features` - Bitmask of features to set.
     fn set_backend_features(&mut self, features: u64) -> Result<()> {
-        // This ioctl is called on a valid vhost fd and has its return value checked.
+        // SAFETY: This ioctl is called on a valid vhost fd and has its return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_BACKEND_FEATURES(), &features) };
 
         if ret >= 0 {
@@ -348,6 +349,8 @@ impl<I: VhostKernBackend + VhostKernFeatures> VhostIotlbBackend for I {
             msg_v2.__bindgen_anon_1.iotlb.perm = msg.perm as u8;
             msg_v2.__bindgen_anon_1.iotlb.type_ = msg.msg_type as u8;
 
+            // SAFETY: This is safe because we are using a valid vhost fd, and
+            // a valid pointer and size to the vhost_msg_v2 structure.
             ret = unsafe {
                 write(
                     self.as_raw_fd(),
@@ -367,6 +370,8 @@ impl<I: VhostKernBackend + VhostKernFeatures> VhostIotlbBackend for I {
             msg_v1.__bindgen_anon_1.iotlb.perm = msg.perm as u8;
             msg_v1.__bindgen_anon_1.iotlb.type_ = msg.msg_type as u8;
 
+            // SAFETY: This is safe because we are using a valid vhost fd, and
+            // a valid pointer and size to the vhost_msg structure.
             ret = unsafe {
                 write(
                     self.as_raw_fd(),
@@ -386,6 +391,9 @@ impl VhostIotlbMsgParser for vhost_msg {
             return Err(Error::InvalidIotlbMsg);
         }
 
+        // SAFETY: We trust the kernel to return a structure with the union
+        // fields properly initialized. We are sure it is a vhost_msg, because
+        // we checked that `self.type_` is VHOST_IOTLB_MSG.
         unsafe {
             if self.__bindgen_anon_1.iotlb.type_ == 0 {
                 return Err(Error::InvalidIotlbMsg);
@@ -408,6 +416,9 @@ impl VhostIotlbMsgParser for vhost_msg_v2 {
             return Err(Error::InvalidIotlbMsg);
         }
 
+        // SAFETY: We trust the kernel to return a structure with the union
+        // fields properly initialized. We are sure it is a vhost_msg_v2, because
+        // we checked that `self.type_` is VHOST_IOTLB_MSG_V2.
         unsafe {
             if self.__bindgen_anon_1.iotlb.type_ == 0 {
                 return Err(Error::InvalidIotlbMsg);

crates/vhost/src/vhost_kern/net.rs

@@ -45,6 +45,7 @@ impl<AS: GuestAddressSpace> VhostNet for Net<AS> {
             fd: fd.map_or(-1, |v| v.as_raw_fd()),
         };
 
+        // SAFETY: Safe because the vhost-net fd is valid and we check the return value
         let ret = unsafe { ioctl_with_ref(self, VHOST_NET_SET_BACKEND(), &vring_file) };
         ioctl_result(ret, ())
     }

crates/vhost/src/vhost_kern/vdpa.rs

@@ -84,7 +84,7 @@ impl<AS: GuestAddressSpace> VhostKernVdpa<AS> {
             log_guest_addr: config_data.get_log_addr(),
         };
 
-        // This ioctl is called on a valid vhost fd and has its
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
         // return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_SET_VRING_ADDR(), &vring_addr) };
         ioctl_result(ret, ())
@@ -94,17 +94,25 @@ impl<AS: GuestAddressSpace> VhostKernVdpa<AS> {
 impl<AS: GuestAddressSpace> VhostVdpa for VhostKernVdpa<AS> {
     fn get_device_id(&self) -> Result<u32> {
         let mut device_id: u32 = 0;
+
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_DEVICE_ID(), &mut device_id) };
         ioctl_result(ret, device_id)
     }
 
     fn get_status(&self) -> Result<u8> {
         let mut status: u8 = 0;
+
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_STATUS(), &mut status) };
         ioctl_result(ret, status)
     }
 
     fn set_status(&self, status: u8) -> Result<()> {
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_VDPA_SET_STATUS(), &status) };
         ioctl_result(ret, ())
     }
@@ -115,6 +123,8 @@ impl<AS: GuestAddressSpace> VhostVdpa for VhostKernVdpa<AS> {
 
         config.as_mut_fam_struct().off = offset;
 
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe {
             ioctl_with_ptr(
                 self,
@@ -136,8 +146,9 @@ impl<AS: GuestAddressSpace> VhostVdpa for VhostKernVdpa<AS> {
         config.as_mut_slice().copy_from_slice(buffer);
 
         let ret =
+            // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+            // return value checked.
             unsafe { ioctl_with_ptr(self, VHOST_VDPA_SET_CONFIG(), config.as_fam_struct_ptr()) };
-
         ioctl_result(ret, ())
     }
 
@@ -147,18 +158,26 @@ impl<AS: GuestAddressSpace> VhostVdpa for VhostKernVdpa<AS> {
             num: enabled as u32,
         };
 
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_VDPA_SET_VRING_ENABLE(), &vring_state) };
         ioctl_result(ret, ())
     }
 
     fn get_vring_num(&self) -> Result<u16> {
         let mut vring_num: u16 = 0;
+
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_VRING_NUM(), &mut vring_num) };
         ioctl_result(ret, vring_num)
     }
 
     fn set_config_call(&self, fd: &EventFd) -> Result<()> {
         let event_fd: ::std::os::raw::c_int = fd.as_raw_fd();
+
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_VDPA_SET_CONFIG_CALL(), &event_fd) };
         ioctl_result(ret, ())
     }
@@ -167,6 +186,8 @@ impl<AS: GuestAddressSpace> VhostVdpa for VhostKernVdpa<AS> {
         let mut low_iova_range = vhost_vdpa_iova_range { first: 0, last: 0 };
 
         let ret =
+            // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+            // return value checked.
             unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_IOVA_RANGE(), &mut low_iova_range) };
 
         let iova_range = VhostVdpaIovaRange {
@@ -179,25 +200,37 @@ impl<AS: GuestAddressSpace> VhostVdpa for VhostKernVdpa<AS> {
 
     fn get_config_size(&self) -> Result<u32> {
         let mut config_size: u32 = 0;
+
         let ret =
+            // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+            // return value checked.
             unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_CONFIG_SIZE(), &mut config_size) };
         ioctl_result(ret, config_size)
     }
 
     fn get_vqs_count(&self) -> Result<u32> {
         let mut vqs_count: u32 = 0;
+
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_VQS_COUNT(), &mut vqs_count) };
         ioctl_result(ret, vqs_count)
     }
 
     fn get_group_num(&self) -> Result<u32> {
         let mut group_num: u32 = 0;
+
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_GROUP_NUM(), &mut group_num) };
         ioctl_result(ret, group_num)
     }
 
     fn get_as_num(&self) -> Result<u32> {
         let mut as_num: u32 = 0;
+
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_AS_NUM(), &mut as_num) };
         ioctl_result(ret, as_num)
     }
@@ -209,6 +242,8 @@ impl<AS: GuestAddressSpace> VhostVdpa for VhostKernVdpa<AS> {
         };
 
         let ret =
+            // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+            // return value checked.
             unsafe { ioctl_with_mut_ref(self, VHOST_VDPA_GET_VRING_GROUP(), &mut vring_state) };
         ioctl_result(ret, vring_state.num)
     }
@@ -219,11 +254,15 @@ impl<AS: GuestAddressSpace> VhostVdpa for VhostKernVdpa<AS> {
             num: asid,
         };
 
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl_with_ref(self, VHOST_VDPA_GET_VRING_GROUP(), &vring_state) };
         ioctl_result(ret, ())
     }
 
     fn suspend(&self) -> Result<()> {
+        // SAFETY: This ioctl is called on a valid vhost-vdpa fd and has its
+        // return value checked.
         let ret = unsafe { ioctl(self, VHOST_VDPA_SUSPEND()) };
         ioctl_result(ret, ())
     }
@@ -506,14 +545,16 @@ mod tests {
         vdpa.dma_map(0xFFFF_0000, 0xFFFF, std::ptr::null::<u8>(), false)
             .unwrap_err();
 
-        unsafe {
-            let layout = Layout::from_size_align(0xFFFF, 1).unwrap();
-            let ptr = alloc(layout);
+        let layout = Layout::from_size_align(0xFFFF, 1).unwrap();
 
-            vdpa.dma_map(0xFFFF_0000, 0xFFFF, ptr, false).unwrap();
-            vdpa.dma_unmap(0xFFFF_0000, 0xFFFF).unwrap();
+        // SAFETY: Safe because layout has non-zero size.
+        let ptr = unsafe { alloc(layout) };
 
-            dealloc(ptr, layout);
-        };
+        vdpa.dma_map(0xFFFF_0000, 0xFFFF, ptr, false).unwrap();
+        vdpa.dma_unmap(0xFFFF_0000, 0xFFFF).unwrap();
+
+        // SAFETY: Safe because `ptr` is allocated with the same allocator
+        // using the same `layout`.
+        unsafe { dealloc(ptr, layout) };
     }
 }