diff --git a/lib/propolis/src/hw/nvme/bits.rs b/lib/propolis/src/hw/nvme/bits.rs index 0ae31b2fb..7407e06da 100644 --- a/lib/propolis/src/hw/nvme/bits.rs +++ b/lib/propolis/src/hw/nvme/bits.rs @@ -5,11 +5,12 @@ #![allow(dead_code)] use bitstruct::bitstruct; +use zerocopy::{FromBytes, FromZeroes}; /// A Submission Queue Entry as represented in memory. /// /// See NVMe 1.0e Section 4.2 Submission Queue Entry - Command Format -#[derive(Debug, Default, Copy, Clone)] +#[derive(Debug, Default, Copy, Clone, FromBytes, FromZeroes)] #[repr(C, packed(1))] pub struct SubmissionQueueEntry { /// Command Dword 0 (CDW0) diff --git a/lib/propolis/src/hw/qemu/fwcfg.rs b/lib/propolis/src/hw/qemu/fwcfg.rs index d3493ea83..2fb8b635f 100644 --- a/lib/propolis/src/hw/qemu/fwcfg.rs +++ b/lib/propolis/src/hw/qemu/fwcfg.rs @@ -815,7 +815,7 @@ mod bits { use zerocopy::byteorder::big_endian::{ U16 as BE16, U32 as BE32, U64 as BE64, }; - use zerocopy::AsBytes; + use zerocopy::{AsBytes, FromBytes, FromZeroes}; pub const FW_CFG_IOP_SELECTOR: u16 = 0x0510; pub const FW_CFG_IOP_DATA: u16 = 0x0511; @@ -867,7 +867,7 @@ mod bits { } } - #[derive(AsBytes, Default, Copy, Clone, Debug)] + #[derive(AsBytes, Default, Copy, Clone, Debug, FromBytes, FromZeroes)] #[repr(C)] pub struct FwCfgDmaAccess { pub ctrl: BE32, diff --git a/lib/propolis/src/hw/virtio/queue.rs b/lib/propolis/src/hw/virtio/queue.rs index fb841a16d..fdaa88d57 100644 --- a/lib/propolis/src/hw/virtio/queue.rs +++ b/lib/propolis/src/hw/virtio/queue.rs @@ -17,8 +17,10 @@ use crate::common::*; use crate::migrate::MigrateStateError; use crate::vmm::MemCtx; +use zerocopy::{FromBytes, FromZeroes}; + #[repr(C)] -#[derive(Copy, Clone)] +#[derive(Copy, Clone, FromBytes, FromZeroes)] struct VqdDesc { addr: u64, len: u32, diff --git a/lib/propolis/src/vmm/mem.rs b/lib/propolis/src/vmm/mem.rs index f6185ceb7..c54245e20 100644 --- a/lib/propolis/src/vmm/mem.rs +++ b/lib/propolis/src/vmm/mem.rs @@ -19,6 +19,8 @@ use crate::common::{GuestAddr, GuestRegion}; use crate::util::aspace::ASpace; use crate::vmm::VmmHdl; +use zerocopy::FromBytes; + bitflags! { /// Bitflags representing memory protections. #[derive(Debug, Copy, Clone)] @@ -480,7 +482,7 @@ impl<'a> SubMapping<'a> { } /// Reads a `T` object from the mapping. - pub fn read(&self) -> Result { + pub fn read(&self) -> Result { self.check_read_access()?; let typed = self.ptr.as_ptr() as *const T; if self.len < std::mem::size_of::() { @@ -488,13 +490,19 @@ impl<'a> SubMapping<'a> { } // Safety: - // - typed must be valid for reads - // - typed must point to a properly initialized value of T + // - typed must be valid for reads: `check_read_access()` succeeded + // - typed must point to a properly initialized value of T: always true + // because we require `T: FromBytes`. `zerocopy::FromBytes` happens + // to have the same concerns as us - that T is valid for all bit + // patterns. Ok(unsafe { typed.read_unaligned() }) } /// Read `values` from the mapping. - pub fn read_many(&self, values: &mut [T]) -> Result<()> { + pub fn read_many( + &self, + values: &mut [T], + ) -> Result<()> { self.check_read_access()?; let copy_len = size_of_val(values); if self.len < copy_len { @@ -508,11 +516,13 @@ impl<'a> SubMapping<'a> { // not guaranteed for the source pointer. Cast it down to a u8, which // will appease those alignment concerns let src = self.ptr.as_ptr() as *const u8; + // We know reinterpreting `*mut T` as `*mut u8` and writing to it cannot + // result in invalid `T` because `T: FromBytes` let dst = values.as_mut_ptr() as *mut u8; // Safety // - `src` is valid for read for the `copy_len` as checked above - // - `src` is valid for writes for its entire length, since it is from a + // - `dst` is valid for writes for its entire length, since it is from a // valid mutable reference passed in to us // - both are aligned for a `u8` copy // - `dst` cannot be overlapped by `src`, since the former came from a @@ -788,7 +798,7 @@ pub struct MemCtx { } impl MemCtx { /// Reads a generic value from a specified guest address. - pub fn read(&self, addr: GuestAddr) -> Option { + pub fn read(&self, addr: GuestAddr) -> Option { if let Some(mapping) = self.region_covered(addr, size_of::(), Prot::READ) { @@ -828,7 +838,7 @@ impl MemCtx { } /// Reads multiple objects from a guest address. - pub fn read_many( + pub fn read_many( &self, base: GuestAddr, count: usize, @@ -1015,7 +1025,7 @@ pub struct MemMany<'a, T: Copy> { pos: usize, phantom: PhantomData, } -impl<'a, T: Copy> MemMany<'a, T> { +impl<'a, T: Copy + FromBytes> MemMany<'a, T> { /// Gets the object at position `pos` within the memory region. /// /// Returns [`Option::None`] if out of range. @@ -1028,7 +1038,7 @@ impl<'a, T: Copy> MemMany<'a, T> { } } } -impl<'a, T: Copy> Iterator for MemMany<'a, T> { +impl<'a, T: Copy + FromBytes> Iterator for MemMany<'a, T> { type Item = T; fn next(&mut self) -> Option {