vmm: modify or provide safety comments

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

vmm/src/coredump.rs

@@ -74,6 +74,7 @@ pub struct X86_64UserRegs {
     pub gs: u64,
 }
 
+// SAFETY: This is just a series of bytes
 unsafe impl ByteValued for X86_64UserRegs {}
 
 #[repr(C)]
@@ -169,6 +170,7 @@ pub struct CpuState {
     pub kernel_gs_base: u64,
 }
 
+// SAFETY: This is just a series of bytes
 unsafe impl ByteValued for CpuState {}
 
 pub enum NoteDescType {

vmm/src/cpu.rs

@@ -562,6 +562,7 @@ impl VcpuState {
     fn signal_thread(&self) {
         if let Some(handle) = self.handle.as_ref() {
             loop {
+                // SAFETY: FFI call with correct arguments
                 unsafe {
                     libc::pthread_kill(handle.as_pthread_t() as _, SIGRTMIN());
                 }
@@ -644,7 +645,7 @@ impl CpuManager {
             const XFEATURE_XTILEDATA: usize = 18;
             const XFEATURE_XTILEDATA_MASK: usize = 1 << XFEATURE_XTILEDATA;
 
-            // This is safe as the syscall is only modifing kernel internal
+            // SAFETY: the syscall is only modifing kernel internal
             // data structures that the kernel is itself expected to safeguard.
             let amx_tile = unsafe {
                 libc::syscall(
@@ -657,9 +658,9 @@ impl CpuManager {
             if amx_tile != 0 {
                 return Err(Error::AmxEnable(anyhow!("Guest AMX usage not supported")));
             } else {
-                // This is safe as the mask being modified (not marked mutable as it is
-                // modified in unsafe only which is permitted) isn't in use elsewhere.
                 let mask: usize = 0;
+                // SAFETY: the mask being modified (not marked mutable as it is
+                // modified in unsafe only which is permitted) isn't in use elsewhere.
                 let result = unsafe {
                     libc::syscall(libc::SYS_arch_prctl, ARCH_GET_XCOMP_GUEST_PERM, &mask)
                 };
@@ -851,9 +852,12 @@ impl CpuManager {
 
         // Prepare the CPU set the current vCPU is expected to run onto.
         let cpuset = self.affinity.get(&vcpu_id).map(|host_cpus| {
+            // SAFETY: all zeros is a valid pattern
             let mut cpuset: libc::cpu_set_t = unsafe { std::mem::zeroed() };
+            // SAFETY: FFI call, trivially safe
             unsafe { libc::CPU_ZERO(&mut cpuset) };
             for host_cpu in host_cpus {
+                // SAFETY: FFI call, trivially safe
                 unsafe { libc::CPU_SET(*host_cpu as usize, &mut cpuset) };
             }
             cpuset
@@ -875,6 +879,7 @@ impl CpuManager {
                 .spawn(move || {
                     // Schedule the thread to run on the expected CPU set
                     if let Some(cpuset) = cpuset.as_ref() {
+                        // SAFETY: FFI call with correct arguments
                         let ret = unsafe {
                             libc::sched_setaffinity(
                                 0,
@@ -1698,6 +1703,7 @@ impl Cpu {
 
         let mut mat_data: Vec<u8> = Vec::new();
         mat_data.resize(std::mem::size_of_val(&lapic), 0);
+        // SAFETY: mat_data is large enough to hold lapic
         unsafe { *(mat_data.as_mut_ptr() as *mut LocalApic) = lapic };
 
         mat_data

vmm/src/device_manager.rs

@@ -1784,6 +1784,7 @@ impl DeviceManager {
         // SAFETY: The following pair are safe because termios gets totally overwritten by tcgetattr
         // and we check the return result.
         let mut termios: termios = unsafe { zeroed() };
+        // SAFETY: see above
         let ret = unsafe { tcgetattr(fd, &mut termios as *mut _) };
         if ret < 0 {
             return vmm_sys_util::errno::errno_result();

vmm/src/lib.rs

@@ -424,6 +424,7 @@ impl Vmm {
             Ok(signals) => {
                 self.signals = Some(signals.handle());
                 let exit_evt = self.exit_evt.try_clone().map_err(Error::EventFdClone)?;
+                // SAFETY: trivially safe
                 let on_tty = unsafe { libc::isatty(libc::STDIN_FILENO) } != 0;
 
                 let signal_handler_seccomp_filter = get_seccomp_filter(

vmm/src/memory_manager.rs

@@ -1188,6 +1188,7 @@ impl MemoryManager {
     }
 
     fn memfd_create(name: &ffi::CStr, flags: u32) -> Result<RawFd, io::Error> {
+        // SAFETY: FFI call with correct arguments
         let res = unsafe { libc::syscall(libc::SYS_memfd_create, name.as_ptr(), flags) };
 
         if res < 0 {
@@ -1205,6 +1206,7 @@ impl MemoryManager {
         maxnode: u64,
         flags: u32,
     ) -> Result<(), io::Error> {
+        // SAFETY: FFI call with correct arguments
         let res = unsafe {
             libc::syscall(
                 libc::SYS_mbind,
@@ -1256,6 +1258,7 @@ impl MemoryManager {
         )
         .map_err(Error::SharedFileCreate)?;
 
+        // SAFETY: fd is valid
         let f = unsafe { File::from_raw_fd(fd) };
         f.set_len(size as u64).map_err(Error::SharedFileSetLen)?;
 
@@ -1277,8 +1280,14 @@ impl MemoryManager {
             let fs = ffi::CString::new(fs_str).unwrap();
             let mut path = fs.as_bytes_with_nul().to_owned();
             let path_ptr = path.as_mut_ptr() as *mut _;
+            // SAFETY: FFI call
             let fd = unsafe { libc::mkstemp(path_ptr) };
+            if fd == -1 {
+                return Err(Error::SharedFileCreate(std::io::Error::last_os_error()));
+            }
+            // SAFETY: FFI call
             unsafe { libc::unlink(path_ptr) };
+            // SAFETY: fd is valid
             let f = unsafe { File::from_raw_fd(fd) };
             f.set_len(size as u64).map_err(Error::SharedFileSetLen)?;
 
@@ -1351,6 +1360,7 @@ impl MemoryManager {
                 region.as_ptr() as u64,
                 size
             );
+            // SAFETY: FFI call with corect arguments
             let ret = unsafe { libc::madvise(region.as_ptr() as _, size, libc::MADV_HUGEPAGE) };
             if ret != 0 {
                 let e = io::Error::last_os_error();
@@ -1573,7 +1583,7 @@ impl MemoryManager {
 
         // Mark the pages as mergeable if explicitly asked for.
         if mergeable {
-            // Safe because the address and size are valid since the
+            // SAFETY: the address and size are valid since the
             // mmap succeeded.
             let ret = unsafe {
                 libc::madvise(
@@ -1628,7 +1638,7 @@ impl MemoryManager {
         // Mark the pages as unmergeable if there were previously marked as
         // mergeable.
         if mergeable {
-            // Safe because the address and size are valid as the region was
+            // SAFETY: the address and size are valid as the region was
             // previously advised.
             let ret = unsafe {
                 libc::madvise(
@@ -1797,6 +1807,7 @@ impl MemoryManager {
             // device does not work that way, it provides a file descriptor
             // which is not matching the mapping size, as it's a just a way to
             // let KVM know that an EPC section is being created for the guest.
+            // SAFETY: FFI call with correct arguments
             let host_addr = unsafe {
                 libc::mmap(
                     std::ptr::null_mut(),
@@ -1845,12 +1856,14 @@ impl MemoryManager {
 
     pub fn is_hardlink(f: &File) -> bool {
         let mut stat = std::mem::MaybeUninit::<libc::stat>::uninit();
+        // SAFETY: FFI call with correct arguments
         let ret = unsafe { libc::fstat(f.as_raw_fd(), stat.as_mut_ptr()) };
         if ret != 0 {
             error!("Couldn't fstat the backing file");
             return false;
         }
 
+        // SAFETY: stat is valid
         unsafe { (*stat.as_ptr()).st_nlink as usize > 0 }
     }
 

vmm/src/serial_manager.rs

@@ -110,8 +110,11 @@ impl SerialManager {
             }
             ConsoleOutputMode::Tty => {
                 // If running on an interactive TTY then accept input
+                // SAFETY: trivially safe
                 if unsafe { libc::isatty(libc::STDIN_FILENO) == 1 } {
+                    // SAFETY: STDIN_FILENO is a valid fd
                     let stdin_clone = unsafe { File::from_raw_fd(libc::dup(libc::STDIN_FILENO)) };
+                    // SAFETY: FFI calls with correct arguments
                     let ret = unsafe {
                         let mut flags = libc::fcntl(stdin_clone.as_raw_fd(), libc::F_GETFL);
                         flags |= libc::O_NONBLOCK;
@@ -159,6 +162,7 @@ impl SerialManager {
         }
 
         // Use 'File' to enforce closing on 'epoll_fd'
+        // SAFETY: epoll_fd is valid
         let epoll_file = unsafe { File::from_raw_fd(epoll_fd) };
 
         Ok(Some(SerialManager {

vmm/src/sigwinch_listener.rs

@@ -45,11 +45,14 @@ extern "C" fn sigwinch_handler(_signo: c_int, _info: *mut siginfo_t, _unused: *m
 
 fn unblock_all_signals() -> io::Result<()> {
     let mut set = MaybeUninit::uninit();
+    // SAFETY: set is a correct structure for sigemptyset
     if unsafe { sigemptyset(set.as_mut_ptr()) } == -1 {
         return Err(io::Error::last_os_error());
     }
+    // SAFETY: set is initialized above
     let set = unsafe { set.assume_init() };
 
+    // SAFETY: all arguments are correct
     if unsafe { sigprocmask(SIG_SETMASK, &set, null_mut()) } == -1 {
         return Err(io::Error::last_os_error());
     }
@@ -62,6 +65,7 @@ fn sigwinch_listener_main(seccomp_filter: BpfProgram, tx: File, pty: File) -> !
 
     let pty_fd = pty.into_raw_fd();
 
+    // SAFETY: FFI calls
     unsafe {
         close(STDIN_FILENO);
         close(STDOUT_FILENO);
@@ -75,6 +79,7 @@ fn sigwinch_listener_main(seccomp_filter: BpfProgram, tx: File, pty: File) -> !
 
     register_signal_handler(SIGWINCH, sigwinch_handler).unwrap();
 
+    // SAFETY: FFI calls
     unsafe {
         // Create a new session (and therefore a new process group).
         assert_ne!(setsid(), -1);
@@ -99,6 +104,7 @@ fn sigwinch_listener_main(seccomp_filter: BpfProgram, tx: File, pty: File) -> !
             revents: 0,
         };
 
+        // SAFETY: FFI call with valid arguments
         while unsafe { poll(&mut pollfd, 1, -1) } == -1 {
             let e = io::Error::last_os_error();
             assert!(
@@ -120,16 +126,20 @@ pub fn start_sigwinch_listener(
     pty_sub: File,
 ) -> io::Result<File> {
     let mut pipe = [-1; 2];
+    // SAFETY: FFI call with valid arguments
     if unsafe { pipe2(pipe.as_mut_ptr(), O_CLOEXEC) } == -1 {
         return Err(io::Error::last_os_error());
     }
 
+    // SAFETY: pipe[0] is valid
     let mut rx = unsafe { File::from_raw_fd(pipe[0]) };
+    // SAFETY: pipe[1] is valid
     let tx = unsafe { File::from_raw_fd(pipe[1]) };
 
     let mut args = clone_args::default();
     args.flags |= CLONE_CLEAR_SIGHAND;
 
+    // SAFETY: FFI call
     match unsafe { clone3(&mut args, size_of::<clone_args>()) } {
         -1 => return Err(io::Error::last_os_error()),
         0 => {

vmm/src/vm.rs

@@ -589,6 +589,7 @@ impl Vm {
         )
         .map_err(Error::CpuManager)?;
 
+        // SAFETY: trivially safe
         let on_tty = unsafe { libc::isatty(libc::STDIN_FILENO) } != 0;
 
         #[cfg(feature = "tdx")]