virtio-devices: modify or provide safety comments

Signed-off-by: Wei Liu <liuwe@microsoft.com>

virtio-devices/src/balloon.rs

@@ -128,8 +128,8 @@ impl BalloonEpollHandler {
         let hva = memory
             .get_host_address(range_base)
             .map_err(Error::GuestMemory)?;
-        // Need unsafe to do syscall madvise
         let res =
+            // SAFETY: Need unsafe to do syscall madvise
             unsafe { libc::madvise(hva as *mut libc::c_void, range_len as libc::size_t, advice) };
         if res != 0 {
             return Err(Error::MadviseFail(io::Error::last_os_error()));
@@ -147,6 +147,7 @@ impl BalloonEpollHandler {
         ))?;
         if let Some(f_off) = region.file_offset() {
             let offset = range_base.0 - region.start_addr().0;
+            // SAFETY: FFI call with valid arguments
             let res = unsafe {
                 libc::fallocate64(
                     f_off.file().as_raw_fd(),

virtio-devices/src/console.rs

@@ -607,6 +607,7 @@ fn get_win_size(tty: &dyn AsRawFd) -> (u16, u16) {
     }
     let ws: WindowSize = WindowSize::default();
 
+    // SAFETY: FFI call with correct arguments
     unsafe {
         libc::ioctl(tty.as_raw_fd(), TIOCGWINSZ, &ws);
     }

virtio-devices/src/epoll_helper.rs

@@ -87,6 +87,7 @@ impl EpollHelper {
         // Create the epoll file descriptor
         let epoll_fd = epoll::create(true).map_err(EpollHelperError::CreateFd)?;
         // Use 'File' to enforce closing on 'epoll_fd'
+        // SAFETY: epoll_fd is a valid fd
         let epoll_file = unsafe { File::from_raw_fd(epoll_fd) };
 
         let mut helper = Self {

virtio-devices/src/iommu.rs

@@ -267,19 +267,31 @@ struct VirtioIommuFault {
     address: u64,
 }
 
-// SAFETY: these data structures only contain integers and have no implicit padding
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuRange32 {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuRange64 {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuConfig {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuReqHead {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuReqTail {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuReqAttach {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuReqDetach {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuReqMap {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuReqUnmap {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuReqProbe {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuProbeProperty {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuProbeResvMem {}
+// SAFETY: data structure only contain integers and have no implicit padding
 unsafe impl ByteValued for VirtioIommuFault {}
 
 #[derive(Error, Debug)]

virtio-devices/src/mem.rs

@@ -417,6 +417,7 @@ impl MemEpollHandler {
     fn discard_memory_range(&self, offset: u64, size: u64) -> Result<(), Error> {
         // Use fallocate if the memory region is backed by a file.
         if let Some(fd) = self.host_fd {
+            // SAFETY: FFI call with valid arguments
             let res = unsafe {
                 libc::fallocate64(
                     fd,
@@ -435,6 +436,7 @@ impl MemEpollHandler {
         // Only use madvise if the memory region is not allocated with
         // hugepages.
         if !self.hugepages {
+            // SAFETY: FFI call with valid arguments
             let res = unsafe {
                 libc::madvise(
                     (self.host_addr + offset) as *mut libc::c_void,

virtio-devices/src/transport/pci_device.rs

@@ -666,8 +666,8 @@ impl VirtioPciDevice {
                     .unwrap();
             }
         } else {
-            // Safe since we know self.cap_pci_cfg_info.cap.cap.offset is 32bits long.
             let bar_offset: u32 =
+                // SAFETY: we know self.cap_pci_cfg_info.cap.cap.offset is 32bits long.
                 unsafe { std::mem::transmute(self.cap_pci_cfg_info.cap.cap.offset) };
             self.read_bar(0, bar_offset as u64, data)
         }
@@ -687,8 +687,8 @@ impl VirtioPciDevice {
             right[..data_len].copy_from_slice(data);
             None
         } else {
-            // Safe since we know self.cap_pci_cfg_info.cap.cap.offset is 32bits long.
             let bar_offset: u32 =
+                // SAFETY: we know self.cap_pci_cfg_info.cap.cap.offset is 32bits long.
                 unsafe { std::mem::transmute(self.cap_pci_cfg_info.cap.cap.offset) };
             self.write_bar(0, bar_offset as u64, data)
         }

virtio-devices/src/vhost_user/fs.rs

@@ -95,6 +95,7 @@ impl VhostUserMasterReqHandler for SlaveReqHandler {
 
             let addr = self.mmap_cache_addr + offset;
             let flags = fs.flags[i];
+            // SAFETY: FFI call with valid arguments
             let ret = unsafe {
                 libc::mmap(
                     addr as *mut libc::c_void,
@@ -136,6 +137,7 @@ impl VhostUserMasterReqHandler for SlaveReqHandler {
             }
 
             let addr = self.mmap_cache_addr + offset;
+            // SAFETY: FFI call with valid arguments
             let ret = unsafe {
                 libc::mmap(
                     addr as *mut libc::c_void,
@@ -172,6 +174,7 @@ impl VhostUserMasterReqHandler for SlaveReqHandler {
 
             let addr = self.mmap_cache_addr + offset;
             let ret =
+                // SAFETY: FFI call with valid arguments
                 unsafe { libc::msync(addr as *mut libc::c_void, len as usize, libc::MS_SYNC) };
             if ret == -1 {
                 return Err(io::Error::last_os_error());
@@ -228,6 +231,7 @@ impl VhostUserMasterReqHandler for SlaveReqHandler {
                     == VhostUserFSSlaveMsgFlags::MAP_W
                 {
                     debug!("write: foffset={}, len={}", foffset, len);
+                    // SAFETY: FFI call with valid arguments
                     unsafe {
                         pwrite64(
                             fd.as_raw_fd(),
@@ -238,6 +242,7 @@ impl VhostUserMasterReqHandler for SlaveReqHandler {
                     }
                 } else {
                     debug!("read: foffset={}, len={}", foffset, len);
+                    // SAFETY: FFI call with valid arguments
                     unsafe { pread64(fd.as_raw_fd(), ptr as *mut c_void, len, foffset as off64_t) }
                 };
 
@@ -279,6 +284,7 @@ impl Default for VirtioFsConfig {
     }
 }
 
+// SAFETY: only a series of integers
 unsafe impl ByteValued for VirtioFsConfig {}
 
 pub struct Fs {

virtio-devices/src/vhost_user/mod.rs

@@ -375,6 +375,7 @@ impl VhostUserCommon {
 
     pub fn shutdown(&mut self) {
         if let Some(vu) = &self.vu {
+            // SAFETY: trivially safe
             let _ = unsafe { libc::close(vu.lock().unwrap().socket_handle().as_raw_fd()) };
         }
 

virtio-devices/src/vhost_user/vu_common_ctrl.rs

@@ -451,7 +451,7 @@ impl VhostUserHandle {
         )
         .map_err(Error::MemfdCreate)?;
 
-        // Safe because we checked the file descriptor is valid
+        // SAFETY: we checked the file descriptor is valid
         let file = unsafe { File::from_raw_fd(fd) };
         // The size of the memory mapping corresponds to the size of a bitmap
         // covering all guest pages for addresses from 0 to the last physical
@@ -465,6 +465,7 @@ impl VhostUserHandle {
         file.set_len(mmap_size).map_err(Error::SetFileSize)?;
 
         // Set the seals
+        // SAFETY: FFI call with valid arguments
         let res = unsafe {
             libc::fcntl(
                 file.as_raw_fd(),
@@ -569,6 +570,7 @@ impl VhostUserHandle {
             // Be careful with the size, as it was based on u8, meaning we must
             // divide it by 8.
             let len = region.size() / 8;
+            // SAFETY: region is of size len
             let bitmap = unsafe {
                 // Cast the pointer to u64
                 let ptr = region.as_ptr() as *const u64;
@@ -582,6 +584,7 @@ impl VhostUserHandle {
 }
 
 fn memfd_create(name: &ffi::CStr, flags: u32) -> std::result::Result<RawFd, std::io::Error> {
+    // SAFETY: FFI call with valid arguments
     let res = unsafe { libc::syscall(libc::SYS_memfd_create, name.as_ptr(), flags) };
 
     if res < 0 {

virtio-devices/src/vsock/packet.rs

@@ -239,7 +239,7 @@ impl VsockPacket {
     /// Provides in-place, byte-slice, access to the vsock packet header.
     ///
     pub fn hdr(&self) -> &[u8] {
-        // This is safe since bound checks have already been performed when creating the packet
+        // 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) }
     }
@@ -247,7 +247,7 @@ impl VsockPacket {
     /// Provides in-place, byte-slice, mutable access to the vsock packet header.
     ///
     pub fn hdr_mut(&mut self) -> &mut [u8] {
-        // This is safe since bound checks have already been performed when creating the packet
+        // 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) }
     }
@@ -261,7 +261,7 @@ impl VsockPacket {
     ///            is stored in the packet header, and accessible via `VsockPacket::len()`.
     pub fn buf(&self) -> Option<&[u8]> {
         self.buf.map(|ptr| {
-            // This is safe since bound checks have already been performed when creating the packet
+            // SAFETY: bound checks have already been performed when creating the packet
             // from the virtq descriptor.
             unsafe { std::slice::from_raw_parts(ptr as *const u8, self.buf_size) }
         })
@@ -276,7 +276,7 @@ impl VsockPacket {
     ///            is stored in the packet header, and accessible via `VsockPacket::len()`.
     pub fn buf_mut(&mut self) -> Option<&mut [u8]> {
         self.buf.map(|ptr| {
-            // This is safe since bound checks have already been performed when creating the packet
+            // SAFETY: bound checks have already been performed when creating the packet
             // from the virtq descriptor.
             unsafe { std::slice::from_raw_parts_mut(ptr, self.buf_size) }
         })
@@ -383,6 +383,7 @@ impl VsockPacket {
 }
 
 #[cfg(test)]
+#[allow(clippy::undocumented_unsafe_blocks)]
 mod tests {
     use super::super::tests::TestContext;
     use super::*;

virtio-devices/src/vsock/unix/muxer.rs

@@ -334,6 +334,7 @@ impl VsockMuxer {
         // device activation time.
         let epoll_fd = epoll::create(true).map_err(Error::EpollFdCreate)?;
         // Use 'File' to enforce closing on 'epoll_fd'
+        // SAFETY: epoll_fd is a valid fd
         let epoll_file = unsafe { File::from_raw_fd(epoll_fd) };
 
         // Open/bind/listen on the host Unix socket, so we can accept host-initiated

virtio-devices/src/watchdog.rs

@@ -235,6 +235,7 @@ impl Watchdog {
             error!("Failed to create timer fd {}", e);
             e
         })?;
+        // SAFETY: timer_fd is a valid fd
         let timer = unsafe { File::from_raw_fd(timer_fd) };
 
         Ok(Watchdog {
@@ -280,6 +281,7 @@ impl Drop for Watchdog {
 }
 
 fn timerfd_create() -> Result<RawFd, io::Error> {
+    // SAFETY: FFI call, trivially safe
     let res = unsafe { libc::timerfd_create(libc::CLOCK_MONOTONIC, 0) };
     if res < 0 {
         Err(io::Error::last_os_error())
@@ -301,6 +303,7 @@ fn timerfd_setup(timer: &File, secs: i64) -> Result<(), io::Error> {
     };
 
     let res =
+        // SAFETY: FFI call with correct arguments
         unsafe { libc::timerfd_settime(timer.as_raw_fd(), 0, &periodic, std::ptr::null_mut()) };
 
     if res < 0 {