diff --git a/rust-vmm-ci b/rust-vmm-ci index 5f36cc96..4cb208e7 160000 --- a/rust-vmm-ci +++ b/rust-vmm-ci @@ -1 +1 @@ -Subproject commit 5f36cc9604fbe8beb2dc9cc27d2a551300b741a5 +Subproject commit 4cb208e71e359cb116e85a2efcc578caaa1570a0 diff --git a/src/devices/src/virtio/net/tap.rs b/src/devices/src/virtio/net/tap.rs index 0db62d98..4ea858d6 100644 --- a/src/devices/src/virtio/net/tap.rs +++ b/src/devices/src/virtio/net/tap.rs @@ -85,6 +85,7 @@ impl IfReqBuilder { } pub fn if_name(mut self, if_name: &[u8; IFACE_NAME_MAX_LEN]) -> Self { + // SAFETY: // Since we don't call as_mut on the same union field more than once, this block is safe. let ifrn_name = unsafe { self.0.ifr_ifrn.ifrn_name.as_mut() }; ifrn_name.copy_from_slice(if_name.as_ref()); @@ -93,6 +94,7 @@ impl IfReqBuilder { } pub(crate) fn flags(mut self, flags: i16) -> Self { + // SAFETY: // Since we don't call as_mut on the same union field more than once, this block is safe. let ifru_flags = unsafe { self.0.ifr_ifru.ifru_flags.as_mut() }; *ifru_flags = flags; @@ -101,6 +103,7 @@ impl IfReqBuilder { } pub(crate) fn execute(mut self, socket: &F, ioctl: u64) -> Result { + // SAFETY: // ioctl is safe. Called with a valid socket fd, and we check the return. let ret = unsafe { ioctl_with_mut_ref(socket, ioctl, &mut self.0) }; if ret < 0 { @@ -119,9 +122,10 @@ impl Tap { pub fn open_named(if_name: &str) -> Result { let terminated_if_name = build_terminated_if_name(if_name)?; + // SAFETY: + // Open calls are safe because we give a constant null-terminated + // string and verify the result. let fd = unsafe { - // Open calls are safe because we give a constant null-terminated - // string and verify the result. libc::open( b"/dev/net/tun\0".as_ptr() as *const c_char, libc::O_RDWR | libc::O_NONBLOCK | libc::O_CLOEXEC, @@ -130,6 +134,7 @@ impl Tap { if fd < 0 { return Err(Error::OpenTun(IoError::last_os_error())); } + // SAFETY: // We just checked that the fd is valid. let tuntap = unsafe { File::from_raw_fd(fd) }; @@ -138,9 +143,10 @@ impl Tap { .flags((IFF_TAP | IFF_NO_PI | IFF_VNET_HDR) as i16) .execute(&tuntap, TUNSETIFF())?; - // Safe since only the name is accessed, and it's cloned out. Ok(Tap { tap_file: tuntap, + // SAFETY: + // Safe since only the name is accessed, and it's cloned out. if_name: unsafe { *ifreq.ifr_ifrn.ifrn_name.as_ref() }, }) } @@ -156,6 +162,7 @@ impl Tap { /// Set the offload flags for the tap interface. pub fn set_offload(&self, flags: c_uint) -> Result<()> { + // SAFETY: // ioctl is safe. Called with a valid tap fd, and we check the return. let ret = unsafe { ioctl_with_val(&self.tap_file, TUNSETOFFLOAD(), c_ulong::from(flags)) }; if ret < 0 { @@ -167,6 +174,7 @@ impl Tap { /// Set the size of the vnet hdr. pub fn set_vnet_hdr_size(&self, size: c_int) -> Result<()> { + // SAFETY: // ioctl is safe. Called with a valid tap fd, and we check the return. let ret = unsafe { ioctl_with_ref(&self.tap_file, TUNSETVNETHDRSZ(), &size) }; if ret < 0 { diff --git a/src/vm-vcpu-ref/src/x86_64/gdt.rs b/src/vm-vcpu-ref/src/x86_64/gdt.rs index 47a87c4b..6c8ad31b 100644 --- a/src/vm-vcpu-ref/src/x86_64/gdt.rs +++ b/src/vm-vcpu-ref/src/x86_64/gdt.rs @@ -51,6 +51,7 @@ pub type Result = std::result::Result; /// Descriptors" of the Intel Manual Volume 3a. pub struct SegmentDescriptor(pub u64); +// SAFETY: // Safe because SegmentDescriptor is just a wrapper over u64. unsafe impl ByteValued for SegmentDescriptor {} diff --git a/src/vm-vcpu-ref/src/x86_64/mpspec.rs b/src/vm-vcpu-ref/src/x86_64/mpspec.rs index 4e3a3eb2..96e8e4c6 100644 --- a/src/vm-vcpu-ref/src/x86_64/mpspec.rs +++ b/src/vm-vcpu-ref/src/x86_64/mpspec.rs @@ -9,7 +9,8 @@ non_camel_case_types, non_upper_case_globals, unused, - clippy::redundant_static_lifetimes + clippy::redundant_static_lifetimes, + clippy::undocumented_unsafe_blocks )] #![cfg_attr(test, allow(clippy::zero_ptr))] #![cfg_attr(test, allow(deref_nullptr))] diff --git a/src/vm-vcpu-ref/src/x86_64/mptable.rs b/src/vm-vcpu-ref/src/x86_64/mptable.rs index d8ec2c06..593f6e6c 100644 --- a/src/vm-vcpu-ref/src/x86_64/mptable.rs +++ b/src/vm-vcpu-ref/src/x86_64/mptable.rs @@ -33,14 +33,33 @@ struct MpcLintsrc(mpspec::mpc_lintsrc); #[derive(Copy, Clone, Default)] struct MpfIntel(mpspec::mpf_intel); -// These `mpspec` wrapper types are POD (Plain Old Data), so reading them from -// data a slice of u8 (which is what ByteValued offers) is safe. +// SAFETY: +// This wrapper type is POD (Plain Old Data), so reading them from a slice of +// u8 (which is what ByteValued offers) is safe. unsafe impl ByteValued for MpcBus {} +// SAFETY: +// This wrapper type is POD (Plain Old Data), so reading them from a slice of +// u8 (which is what ByteValued offers) is safe. unsafe impl ByteValued for MpcCpu {} +// SAFETY: +// This wrapper type is POD (Plain Old Data), so reading them from a slice of +// u8 (which is what ByteValued offers) is safe. unsafe impl ByteValued for MpcIntsrc {} +// SAFETY: +// This wrapper type is POD (Plain Old Data), so reading them from a slice of +// u8 (which is what ByteValued offers) is safe. unsafe impl ByteValued for MpcIoapic {} +// SAFETY: +// This wrapper type is POD (Plain Old Data), so reading them from a slice of +// u8 (which is what ByteValued offers) is safe. unsafe impl ByteValued for MpcTable {} +// SAFETY: +// This wrapper type is POD (Plain Old Data), so reading them from a slice of +// u8 (which is what ByteValued offers) is safe. unsafe impl ByteValued for MpcLintsrc {} +// SAFETY: +// This wrapper type is POD (Plain Old Data), so reading them from a slice of +// u8 (which is what ByteValued offers) is safe. unsafe impl ByteValued for MpfIntel {} // MPTABLE, describing VCPUS. @@ -105,6 +124,7 @@ const CPU_FEATURE_APIC: u32 = 0x200; const CPU_FEATURE_FPU: u32 = 0x001; fn compute_checksum(v: &T) -> u8 { + // SAFETY: // Safe because we are only reading the bytes within the size of the `T` reference `v`. let v_slice = unsafe { slice::from_raw_parts(v as *const T as *const u8, mem::size_of::()) }; let mut checksum: u8 = 0; diff --git a/src/vm-vcpu/src/vcpu/mod.rs b/src/vm-vcpu/src/vcpu/mod.rs index 26c15ebc..53b3aae6 100644 --- a/src/vm-vcpu/src/vcpu/mod.rs +++ b/src/vm-vcpu/src/vcpu/mod.rs @@ -654,6 +654,7 @@ impl KvmVcpu { fn set_local_immediate_exit(value: u8) { Self::TLS_VCPU_PTR.with(|v| { if let Some(vcpu) = *v.borrow() { + // SAFETY: // The block below modifies a mmaped memory region (`kvm_run` struct) which is valid // as long as the `VMM` is still in scope. This function is called in response to // SIGRTMIN(), while the vCPU threads are still active. Their termination are diff --git a/src/vm-vcpu/src/vcpu/regs.rs b/src/vm-vcpu/src/vcpu/regs.rs index 226d70d2..5a4fc2d7 100644 --- a/src/vm-vcpu/src/vcpu/regs.rs +++ b/src/vm-vcpu/src/vcpu/regs.rs @@ -18,6 +18,7 @@ macro_rules! arm64_core_reg { macro_rules! offset__of { ($str:ty, $($field:ident)+) => ({ let tmp: std::mem::MaybeUninit<$str> = std::mem::MaybeUninit::uninit(); + // SAFETY: // Safe because we are not using the value of tmp. let tmp = unsafe { tmp.assume_init() }; let base = &tmp as *const _ as usize; diff --git a/src/vm-vcpu/src/vm.rs b/src/vm-vcpu/src/vm.rs index 05514fe6..a435dbef 100644 --- a/src/vm-vcpu/src/vm.rs +++ b/src/vm-vcpu/src/vm.rs @@ -355,6 +355,7 @@ impl KvmVm { flags: 0, }; + // SAFETY: // Safe because: // * userspace_addr is a valid address for a memory region, obtained by calling // get_host_address() on a valid region's start address;