From 4b292afafcd44ca3fc34f483a8edb455a3212cb5 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 12 Apr 2019 16:57:48 -0700 Subject: 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 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 Tested-by: David Tolnay Tested-by: kokoro Reviewed-by: David Tolnay --- x86_64/Cargo.toml | 1 + x86_64/src/regs.rs | 28 ++++++++++++++++++---------- 2 files changed, 19 insertions(+), 10 deletions(-) (limited to 'x86_64') 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 { /// /// * `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::(); + const SIZE_OF_ENTRY: usize = mem::size_of::(); + const ALIGN_OF_MSRS: usize = mem::align_of::(); + const ALIGN_OF_ENTRY: usize = mem::align_of::(); + const_assert!(ALIGN_OF_MSRS >= ALIGN_OF_ENTRY); + let entry_vec = create_msr_entries(); - let vec_size_bytes = - mem::size_of::() + (entry_vec.len() * mem::size_of::()); - let vec: Vec = 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::() }; 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 -- cgit 1.4.1