diff --git a/vmm/src/coredump.rs b/vmm/src/coredump.rs index 99647c180..2e0cbf403 100644 --- a/vmm/src/coredump.rs +++ b/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 { diff --git a/vmm/src/cpu.rs b/vmm/src/cpu.rs index 0728eb2af..1524abe7b 100644 --- a/vmm/src/cpu.rs +++ b/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 = 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 diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 7165f5801..abb207c8d 100644 --- a/vmm/src/device_manager.rs +++ b/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(); diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index 112be7e25..aab5a01ff 100644 --- a/vmm/src/lib.rs +++ b/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( diff --git a/vmm/src/memory_manager.rs b/vmm/src/memory_manager.rs index c814ff33f..c3fd98578 100644 --- a/vmm/src/memory_manager.rs +++ b/vmm/src/memory_manager.rs @@ -1188,6 +1188,7 @@ impl MemoryManager { } fn memfd_create(name: &ffi::CStr, flags: u32) -> Result { + // 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::::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 } } diff --git a/vmm/src/serial_manager.rs b/vmm/src/serial_manager.rs index d5be4c725..4ea0fe26f 100644 --- a/vmm/src/serial_manager.rs +++ b/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 { diff --git a/vmm/src/sigwinch_listener.rs b/vmm/src/sigwinch_listener.rs index 4b9cac650..e5e1dfe06 100644 --- a/vmm/src/sigwinch_listener.rs +++ b/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 { 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::()) } { -1 => return Err(io::Error::last_os_error()), 0 => { diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 3172d805d..f4907def1 100644 --- a/vmm/src/vm.rs +++ b/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")]