diff options
author | David Tolnay <dtolnay@chromium.org> | 2019-04-12 16:57:48 -0700 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2019-04-18 19:51:29 -0700 |
commit | 4b292afafcd44ca3fc34f483a8edb455a3212cb5 (patch) | |
tree | 868bdb3122e088e33836cd48608d23518ee5a1d0 /vhost | |
parent | dc4effa72b214bc3bd14ca2f7772ab1b728aef5b (diff) | |
download | crosvm-4b292afafcd44ca3fc34f483a8edb455a3212cb5.tar crosvm-4b292afafcd44ca3fc34f483a8edb455a3212cb5.tar.gz crosvm-4b292afafcd44ca3fc34f483a8edb455a3212cb5.tar.bz2 crosvm-4b292afafcd44ca3fc34f483a8edb455a3212cb5.tar.lz crosvm-4b292afafcd44ca3fc34f483a8edb455a3212cb5.tar.xz crosvm-4b292afafcd44ca3fc34f483a8edb455a3212cb5.tar.zst crosvm-4b292afafcd44ca3fc34f483a8edb455a3212cb5.zip |
clippy: Resolve cast_ptr_alignment
This CL fixes four cases of what I believe are undefined behavior: - In vhost where the original code allocates a Vec<u8> with 1-byte alignment and casts the Vec's data pointer to a &mut vhost_memory which is required to be 8-byte aligned. Underaligned references of type &T or &mut T are always undefined behavior in Rust. - Same pattern in x86_64. - Same pattern in plugin::vcpu. - Code in crosvm_plugin that dereferences a potentially underaligned pointer. This is always undefined behavior in Rust. TEST=bin/clippy TEST=cargo test sys_util Change-Id: I926f17b1fe022a798f69d738f9990d548f40c59b Reviewed-on: https://chromium-review.googlesource.com/1566736 Commit-Ready: David Tolnay <dtolnay@chromium.org> Tested-by: David Tolnay <dtolnay@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: David Tolnay <dtolnay@chromium.org>
Diffstat (limited to 'vhost')
-rw-r--r-- | vhost/Cargo.toml | 1 | ||||
-rw-r--r-- | vhost/src/lib.rs | 31 |
2 files changed, 22 insertions, 10 deletions
diff --git a/vhost/Cargo.toml b/vhost/Cargo.toml index 21ebe50..567e880 100644 --- a/vhost/Cargo.toml +++ b/vhost/Cargo.toml @@ -5,6 +5,7 @@ authors = ["The Chromium OS Authors"] edition = "2018" [dependencies] +assertions = { path = "../assertions" } libc = "*" net_util = { path = "../net_util" } sys_util = { path = "../sys_util" } diff --git a/vhost/src/lib.rs b/vhost/src/lib.rs index c1fb4cd..04ba655 100644 --- a/vhost/src/lib.rs +++ b/vhost/src/lib.rs @@ -9,14 +9,16 @@ pub use crate::net::Net; pub use crate::net::NetT; pub use crate::vsock::Vsock; +use std::alloc::Layout; use std::fmt::{self, Display}; use std::io::Error as IoError; use std::mem; use std::os::unix::io::AsRawFd; use std::ptr::null; +use assertions::const_assert; use sys_util::{ioctl, ioctl_with_mut_ref, ioctl_with_ptr, ioctl_with_ref}; -use sys_util::{EventFd, GuestAddress, GuestMemory, GuestMemoryError}; +use sys_util::{EventFd, GuestAddress, GuestMemory, GuestMemoryError, LayoutAllocation}; #[derive(Debug)] pub enum Error { @@ -108,14 +110,21 @@ pub trait Vhost: AsRawFd + std::marker::Sized { /// Set the guest memory mappings for vhost to use. fn set_mem_table(&self) -> Result<()> { + const SIZE_OF_MEMORY: usize = mem::size_of::<virtio_sys::vhost_memory>(); + const SIZE_OF_REGION: usize = mem::size_of::<virtio_sys::vhost_memory_region>(); + const ALIGN_OF_MEMORY: usize = mem::align_of::<virtio_sys::vhost_memory>(); + const ALIGN_OF_REGION: usize = mem::align_of::<virtio_sys::vhost_memory_region>(); + const_assert!(ALIGN_OF_MEMORY >= ALIGN_OF_REGION); + let num_regions = self.mem().num_regions() as usize; - let vec_size_bytes = mem::size_of::<virtio_sys::vhost_memory>() - + (num_regions * mem::size_of::<virtio_sys::vhost_memory_region>()); - let mut bytes: Vec<u8> = vec![0; vec_size_bytes]; - // Convert bytes pointer to a vhost_memory mut ref. The vector has been - // sized correctly to ensure it can hold vhost_memory and N regions. - let vhost_memory: &mut virtio_sys::vhost_memory = - unsafe { &mut *(bytes.as_mut_ptr() as *mut virtio_sys::vhost_memory) }; + let size = SIZE_OF_MEMORY + num_regions * SIZE_OF_REGION; + let layout = Layout::from_size_align(size, ALIGN_OF_MEMORY).expect("impossible layout"); + let mut allocation = LayoutAllocation::zeroed(layout); + + // Safe to obtain an exclusive reference because there are no other + // references to the allocation yet and all-zero is a valid bit pattern. + let vhost_memory = unsafe { allocation.as_mut::<virtio_sys::vhost_memory>() }; + vhost_memory.nregions = num_regions as u32; // regions is a zero-length array, so taking a mut slice requires that // we correctly specify the size to match the amount of backing memory. @@ -136,12 +145,14 @@ pub trait Vhost: AsRawFd + std::marker::Sized { // 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, virtio_sys::VHOST_SET_MEM_TABLE(), bytes.as_ptr()) }; + let ret = unsafe { ioctl_with_ptr(self, virtio_sys::VHOST_SET_MEM_TABLE(), vhost_memory) }; if ret < 0 { return ioctl_result(); } + Ok(()) + + // vhost_memory allocation is deallocated. } /// Set the number of descriptors in the vring. |