summary refs log tree commit diff
path: root/x86_64
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 /x86_64
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 'x86_64')
-rw-r--r--x86_64/Cargo.toml1
-rw-r--r--x86_64/src/regs.rs28
2 files changed, 19 insertions, 10 deletions
diff --git a/x86_64/Cargo.toml b/x86_64/Cargo.toml
index 4aa97a9..58d688b 100644
--- a/x86_64/Cargo.toml
+++ b/x86_64/Cargo.toml
@@ -7,6 +7,7 @@ build = "build.rs"
 
 [dependencies]
 arch = { path = "../arch" }
+assertions = { path = "../assertions" }
 byteorder = "*"
 data_model = { path = "../data_model" }
 devices = { path = "../devices" }
diff --git a/x86_64/src/regs.rs b/x86_64/src/regs.rs
index e1bd1f3..8879a0c 100644
--- a/x86_64/src/regs.rs
+++ b/x86_64/src/regs.rs
@@ -2,17 +2,18 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+use std::alloc::Layout;
 use std::fmt::{self, Display};
 use std::{mem, result};
 
+use assertions::const_assert;
 use kvm;
 use kvm_sys::kvm_fpu;
 use kvm_sys::kvm_msr_entry;
 use kvm_sys::kvm_msrs;
 use kvm_sys::kvm_regs;
 use kvm_sys::kvm_sregs;
-use sys_util;
-use sys_util::{GuestAddress, GuestMemory};
+use sys_util::{self, GuestAddress, GuestMemory, LayoutAllocation};
 
 use crate::gdt;
 
@@ -129,15 +130,20 @@ fn create_msr_entries() -> Vec<kvm_msr_entry> {
 ///
 /// * `vcpu` - Structure for the vcpu that holds the vcpu fd.
 pub fn setup_msrs(vcpu: &kvm::Vcpu) -> Result<()> {
+    const SIZE_OF_MSRS: usize = mem::size_of::<kvm_msrs>();
+    const SIZE_OF_ENTRY: usize = mem::size_of::<kvm_msr_entry>();
+    const ALIGN_OF_MSRS: usize = mem::align_of::<kvm_msrs>();
+    const ALIGN_OF_ENTRY: usize = mem::align_of::<kvm_msr_entry>();
+    const_assert!(ALIGN_OF_MSRS >= ALIGN_OF_ENTRY);
+
     let entry_vec = create_msr_entries();
-    let vec_size_bytes =
-        mem::size_of::<kvm_msrs>() + (entry_vec.len() * mem::size_of::<kvm_msr_entry>());
-    let vec: Vec<u8> = Vec::with_capacity(vec_size_bytes);
-    let msrs: &mut kvm_msrs = unsafe {
-        // Converting the vector's memory to a struct is unsafe.  Carefully using the read-only
-        // vector to size and set the members ensures no out-of-bounds erros below.
-        &mut *(vec.as_ptr() as *mut kvm_msrs)
-    };
+    let size = SIZE_OF_MSRS + entry_vec.len() * SIZE_OF_ENTRY;
+    let layout = Layout::from_size_align(size, ALIGN_OF_MSRS).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 msrs = unsafe { allocation.as_mut::<kvm_msrs>() };
 
     unsafe {
         // Mapping the unsized array to a slice is unsafe becase the length isn't known.  Providing
@@ -150,6 +156,8 @@ pub fn setup_msrs(vcpu: &kvm::Vcpu) -> Result<()> {
     vcpu.set_msrs(msrs).map_err(Error::MsrIoctlFailed)?;
 
     Ok(())
+
+    // msrs allocation is deallocated.
 }
 
 /// Configure FPU registers for x86