From 127453d7eccdb6a903d0855fabb8f0935be90882 Mon Sep 17 00:00:00 2001 From: Zach Reizner Date: Thu, 30 May 2019 18:31:02 -0700 Subject: eliminate mut from non-mut references This manifested itself in a couple places that were turning shared memory buffers into slices for the purposes of passing these slices to `Read` and `Write` trait methods. However, this required the removal of the methods that took `Read` and `Write` instances. This was a convenient interface but impossible to implement safely because making slices from raw pointers without enforcing safety guarantees causes undefined behaviour in Rust. It turns out lots of code in crosvm was using these interfaces indirectly, which explains why this CL touches so much. TEST=crosvm run BUG=chromium:938767 Change-Id: I4ff40c98da6ed08a4a42f4c31f0717f81b1c5863 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1636685 Reviewed-by: Zach Reizner Tested-by: Zach Reizner Tested-by: kokoro Commit-Queue: Zach Reizner --- data_model/src/volatile_memory.rs | 149 +++++++------------------------------- 1 file changed, 26 insertions(+), 123 deletions(-) (limited to 'data_model') diff --git a/data_model/src/volatile_memory.rs b/data_model/src/volatile_memory.rs index 9bdc0f4..6ce230b 100644 --- a/data_model/src/volatile_memory.rs +++ b/data_model/src/volatile_memory.rs @@ -21,14 +21,10 @@ use std::cmp::min; use std::fmt::{self, Display}; -use std::io::Result as IoResult; -use std::io::{Read, Write}; use std::marker::PhantomData; use std::mem::size_of; -use std::ptr::copy; -use std::ptr::{null_mut, read_volatile, write_volatile}; +use std::ptr::{copy, null_mut, read_volatile, write_bytes, write_volatile}; use std::result; -use std::slice::{from_raw_parts, from_raw_parts_mut}; use std::{isize, usize}; use crate::DataInit; @@ -178,6 +174,31 @@ impl<'a> VolatileSlice<'a> { unsafe { Ok(VolatileSlice::new(new_addr as *mut u8, new_size)) } } + /// Sets each byte of this slice with the given byte, similar to `memset`. + /// + /// The bytes of this slice are accessed in an arbitray order. + /// + /// # Examples + /// + /// ``` + /// # use data_model::VolatileMemory; + /// # fn test_write_45() -> Result<(), ()> { + /// let mut mem = [0u8; 32]; + /// let mem_ref = &mut mem[..]; + /// let vslice = mem_ref.get_slice(0, 32).map_err(|_| ())?; + /// vslice.write_bytes(45); + /// for &mut v in mem_ref { + /// assert_eq!(v, 45); + /// } + /// # Ok(()) + /// # } + pub fn write_bytes(&self, value: u8) { + // Safe because the memory is valid and needs only byte alignment. + unsafe { + write_bytes(self.as_ptr(), value, self.size as usize); + } + } + /// Copies `self.size()` or `buf.len()` times the size of `T` bytes, whichever is smaller, to /// `buf`. /// @@ -270,124 +291,6 @@ impl<'a> VolatileSlice<'a> { } } } - - /// Attempt to write all data from memory to a writable object and returns how many bytes were - /// actually written on success. - /// - /// # Arguments - /// * `w` - Write from memory to `w`. - /// - /// # Examples - /// - /// * Write some bytes to /dev/null - /// - /// ``` - /// # use std::fs::File; - /// # use std::path::Path; - /// # use data_model::VolatileMemory; - /// # fn test_write_null() -> Result<(), ()> { - /// # let mut mem = [0u8; 32]; - /// # let mem_ref = &mut mem[..]; - /// # let vslice = mem_ref.get_slice(0, 32).map_err(|_| ())?; - /// let mut file = File::open(Path::new("/dev/null")).map_err(|_| ())?; - /// vslice.write_to(&mut file).map_err(|_| ())?; - /// # Ok(()) - /// # } - /// ``` - pub fn write_to(&self, w: &mut T) -> IoResult { - w.write(unsafe { self.as_slice() }) - } - - /// Writes all data from memory to a writable object via `Write::write_all`. - /// - /// # Arguments - /// * `w` - Write from memory to `w`. - /// - /// # Examples - /// - /// * Write some bytes to /dev/null - /// - /// ``` - /// # use std::fs::File; - /// # use std::path::Path; - /// # use data_model::VolatileMemory; - /// # fn test_write_null() -> Result<(), ()> { - /// # let mut mem = [0u8; 32]; - /// # let mem_ref = &mut mem[..]; - /// # let vslice = mem_ref.get_slice(0, 32).map_err(|_| ())?; - /// let mut file = File::open(Path::new("/dev/null")).map_err(|_| ())?; - /// vslice.write_all_to(&mut file).map_err(|_| ())?; - /// # Ok(()) - /// # } - /// ``` - pub fn write_all_to(&self, w: &mut T) -> IoResult<()> { - w.write_all(unsafe { self.as_slice() }) - } - - /// Reads up to this slice's size to memory from a readable object and returns how many bytes - /// were actually read on success. - /// - /// # Arguments - /// * `r` - Read to `r` to memory. - /// - /// # Examples - /// - /// * Read some bytes to /dev/null - /// - /// ``` - /// # use std::fs::File; - /// # use std::path::Path; - /// # use data_model::VolatileMemory; - /// # fn test_write_null() -> Result<(), ()> { - /// # let mut mem = [0u8; 32]; - /// # let mem_ref = &mut mem[..]; - /// # let vslice = mem_ref.get_slice(0, 32).map_err(|_| ())?; - /// let mut file = File::open(Path::new("/dev/null")).map_err(|_| ())?; - /// vslice.read_from(&mut file).map_err(|_| ())?; - /// # Ok(()) - /// # } - /// ``` - pub fn read_from(&self, r: &mut T) -> IoResult { - r.read(unsafe { self.as_mut_slice() }) - } - - /// Read exactly this slice's size into memory from to a readable object via `Read::read_exact`. - /// - /// # Arguments - /// * `r` - Read to `r` to memory. - /// - /// # Examples - /// - /// * Read some bytes to /dev/null - /// - /// ``` - /// # use std::fs::File; - /// # use std::path::Path; - /// # use data_model::VolatileMemory; - /// # fn test_write_null() -> Result<(), ()> { - /// # let mut mem = [0u8; 32]; - /// # let mem_ref = &mut mem[..]; - /// # let vslice = mem_ref.get_slice(0, 32).map_err(|_| ())?; - /// let mut file = File::open(Path::new("/dev/null")).map_err(|_| ())?; - /// vslice.read_from(&mut file).map_err(|_| ())?; - /// # Ok(()) - /// # } - /// ``` - pub fn read_exact_from(&self, r: &mut T) -> IoResult<()> { - r.read_exact(unsafe { self.as_mut_slice() }) - } - - // These function are private and only used for the read/write functions. It is not valid in - // general to take slices of volatile memory. - unsafe fn as_slice(&self) -> &[u8] { - from_raw_parts(self.addr, self.size as usize) - } - - // TODO(zachr) - refactor this so the mut from non-mut isn't necessary (bug: 938767) - #[allow(clippy::mut_from_ref)] - unsafe fn as_mut_slice(&self) -> &mut [u8] { - from_raw_parts_mut(self.addr, self.size as usize) - } } impl<'a> VolatileMemory for VolatileSlice<'a> { -- cgit 1.4.1