summary refs log tree commit diff
path: root/vhost
diff options
context:
space:
mode:
authorDavid Tolnay <dtolnay@chromium.org>2019-04-12 16:57:48 -0700
committerchrome-bot <chrome-bot@chromium.org>2019-04-18 19:51:29 -0700
commit4b292afafcd44ca3fc34f483a8edb455a3212cb5 (patch)
tree868bdb3122e088e33836cd48608d23518ee5a1d0 /vhost
parentdc4effa72b214bc3bd14ca2f7772ab1b728aef5b (diff)
downloadcrosvm-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.toml1
-rw-r--r--vhost/src/lib.rs31
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.