From 664f7d38e0a81597d83fd7805b752452dc7339c0 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Fri, 7 Feb 2020 08:27:21 +0100 Subject: [PATCH] Add first pass at GuestArray (#5) * Add first pass at GuestArray * Return &'a [T] instead of Vec Also, add a sanity test for `GuestArray`. * Change approach in GuestArray::as_ref The new approach should avoid unnecessary copying (does it?) by iterating through the memory and firstly validating the guest pointers, to then extracting the slice `&'a [T]` using the unsafe `slice::from_raw_parts` fn. * Redo implementation of GuestArray and GuestArrayMut This commit: * redos the impl of `as_ref` and `as_ref_mut` for `GuestArray` and `GuestArrayMut` structs in that they now return dynamically borrow checked `GuestArrayRef` and `GuestArrayRefMut` structs * introduces `GuestArrayRef` and `GuestArrayRefMut` structs which perform dynamic borrow-checking of memory region at runtime, and can be derefed to `&[T]` and `&mut [T]` * adds a few sanity checks for the introduced types * Rename r#ref to ref_ * Add constructors for GuestArray and GuestArrayMut This commit: * adds constructors for `GuestArray` and `GuestArrayMut` both of which can now *only* be constructed from `GuestPtr::array` and `GuestPtrMut::array_mut` respectively * changes `Region::extend` to extend the region by adding to the current `len` (avoids problem of asserting for > 0) * implements `fmt::Debug` for most of memory types for easier testing (implementation is *not* enforced on the generic parameter `T` in the struct; rather, if `T` impls `fmt::Debug` then so does the memory type such as `GuestPtr<'_, T>`) --- crates/memory/src/borrow.rs | 1 + crates/memory/src/error.rs | 2 +- crates/memory/src/memory.rs | 400 ++++++++++++++++++++++++++++++++++++ crates/memory/src/region.rs | 7 + 4 files changed, 409 insertions(+), 1 deletion(-) diff --git a/crates/memory/src/borrow.rs b/crates/memory/src/borrow.rs index c691888db9..bd1d077e17 100644 --- a/crates/memory/src/borrow.rs +++ b/crates/memory/src/borrow.rs @@ -5,6 +5,7 @@ use crate::region::Region; #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct BorrowHandle(usize); +#[derive(Debug)] pub struct GuestBorrows { immutable: HashMap, mutable: HashMap, diff --git a/crates/memory/src/error.rs b/crates/memory/src/error.rs index 657faa3384..cbd4bbb61a 100644 --- a/crates/memory/src/error.rs +++ b/crates/memory/src/error.rs @@ -1,7 +1,7 @@ use crate::Region; use thiserror::Error; -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq, Eq)] pub enum GuestError { #[error("Invalid enum value {0}")] InvalidEnumValue(&'static str), diff --git a/crates/memory/src/memory.rs b/crates/memory/src/memory.rs index a8857f893d..bf3001b357 100644 --- a/crates/memory/src/memory.rs +++ b/crates/memory/src/memory.rs @@ -1,4 +1,5 @@ use std::cell::RefCell; +use std::fmt; use std::marker::PhantomData; use std::rc::Rc; @@ -12,6 +13,16 @@ pub struct GuestMemory<'a> { borrows: Rc>, } +impl<'a> fmt::Debug for GuestMemory<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "GuestMemory {{ ptr: {:?}, len: {:?}, borrows: {:?} }}", + self.ptr, self.len, self.borrows + ) + } +} + impl<'a> GuestMemory<'a> { pub fn new(ptr: *mut u8, len: u32) -> GuestMemory<'a> { assert_eq!(ptr as usize % 4096, 0, "GuestMemory must be page-aligned"); @@ -63,6 +74,16 @@ pub struct GuestPtr<'a, T> { type_: PhantomData, } +impl<'a, T: fmt::Debug> fmt::Debug for GuestPtr<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "GuestPtr {{ mem: {:?}, region: {:?} }}", + self.mem, self.region + ) + } +} + impl<'a, T: GuestType> GuestPtr<'a, T> { pub fn as_raw(&self) -> *const u8 { (self.mem.ptr as usize + self.region.start as usize) as *const u8 @@ -76,6 +97,20 @@ impl<'a, T: GuestType> GuestPtr<'a, T> { pub fn cast(&self, offset: u32) -> Result, GuestError> { self.mem.ptr(self.region.start + offset) } + + pub fn array(&self, num_elems: u32) -> Result, GuestError> { + let region = self.region.extend((num_elems - 1) * T::size()); + if self.mem.contains(region) { + let ptr = GuestPtr { + mem: self.mem, + region: self.region, + type_: self.type_, + }; + Ok(GuestArray { ptr, num_elems }) + } else { + Err(GuestError::PtrOutOfBounds(region)) + } + } } impl<'a, T: GuestTypeCopy> GuestPtr<'a, T> { @@ -158,6 +193,16 @@ pub struct GuestPtrMut<'a, T> { type_: PhantomData, } +impl<'a, T: fmt::Debug> fmt::Debug for GuestPtrMut<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "GuestPtrMut {{ mem: {:?}, region: {:?} }}", + self.mem, self.region + ) + } +} + impl<'a, T: GuestType> GuestPtrMut<'a, T> { pub fn as_immut(&self) -> GuestPtr<'a, T> { GuestPtr { @@ -170,6 +215,7 @@ impl<'a, T: GuestType> GuestPtrMut<'a, T> { pub fn as_raw(&self) -> *const u8 { self.as_immut().as_raw() } + pub fn elem(&self, elements: i32) -> Result, GuestError> { self.mem .ptr_mut(self.region.start + (elements * self.region.len as i32) as u32) @@ -178,6 +224,20 @@ impl<'a, T: GuestType> GuestPtrMut<'a, T> { pub fn cast(&self, offset: u32) -> Result, GuestError> { self.mem.ptr_mut(self.region.start + offset) } + + pub fn array_mut(&self, num_elems: u32) -> Result, GuestError> { + let region = self.region.extend((num_elems - 1) * T::size()); + if self.mem.contains(region) { + let ptr = GuestPtrMut { + mem: self.mem, + region: self.region, + type_: self.type_, + }; + Ok(GuestArrayMut { ptr, num_elems }) + } else { + Err(GuestError::PtrOutOfBounds(region)) + } + } } impl<'a, T: GuestTypeCopy> GuestPtrMut<'a, T> { @@ -271,6 +331,16 @@ pub struct GuestRef<'a, T> { type_: PhantomData, } +impl<'a, T: fmt::Debug> fmt::Debug for GuestRef<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "GuestRef {{ mem: {:?}, region: {:?}, handle: {:?} }}", + self.mem, self.region, self.handle + ) + } +} + impl<'a, T> GuestRef<'a, T> { pub fn as_ptr(&self) -> GuestPtr<'a, T> { GuestPtr { @@ -309,6 +379,16 @@ pub struct GuestRefMut<'a, T> { type_: PhantomData, } +impl<'a, T: fmt::Debug> fmt::Debug for GuestRefMut<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "GuestRefMut {{ mem: {:?}, region: {:?}, handle: {:?} }}", + self.mem, self.region, self.handle + ) + } +} + impl<'a, T> GuestRefMut<'a, T> { pub fn as_ptr(&self) -> GuestPtr<'a, T> { GuestPtr { @@ -359,3 +439,323 @@ impl<'a, T> Drop for GuestRefMut<'a, T> { borrows.unborrow_mut(self.handle); } } + +pub struct GuestArray<'a, T> +where + T: GuestType, +{ + ptr: GuestPtr<'a, T>, + num_elems: u32, +} + +impl<'a, T: GuestType + fmt::Debug> fmt::Debug for GuestArray<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "GuestArray {{ ptr: {:?}, num_elems: {:?} }}", + self.ptr, self.num_elems + ) + } +} + +impl<'a, T> GuestArray<'a, T> +where + T: GuestTypeCopy, +{ + pub fn as_ref(&self) -> Result, GuestError> { + let mut ptr = self.ptr.clone(); + for _ in 0..self.num_elems { + ptr = ptr.elem(1)?; + T::validate(&ptr)?; + } + let region = self.ptr.region.extend((self.num_elems - 1) * T::size()); + let handle = { + let mut borrows = self.ptr.mem.borrows.borrow_mut(); + borrows + .borrow_immut(region) + .ok_or_else(|| GuestError::PtrBorrowed(region))? + }; + let ref_ = GuestRef { + mem: self.ptr.mem, + region, + handle, + type_: self.ptr.type_, + }; + Ok(GuestArrayRef { + ref_, + num_elems: self.num_elems, + }) + } +} + +pub struct GuestArrayRef<'a, T> +where + T: GuestType, +{ + ref_: GuestRef<'a, T>, + num_elems: u32, +} + +impl<'a, T> fmt::Debug for GuestArrayRef<'a, T> +where + T: GuestType + fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "GuestArrayRef {{ ref_: {:?}, num_elems: {:?} }}", + self.ref_, self.num_elems + ) + } +} + +impl<'a, T> ::std::ops::Deref for GuestArrayRef<'a, T> +where + T: GuestTypeCopy, +{ + type Target = [T]; + + fn deref(&self) -> &Self::Target { + unsafe { + std::slice::from_raw_parts( + self.ref_.as_ptr().as_raw() as *const T, + self.num_elems as usize, + ) + } + } +} + +pub struct GuestArrayMut<'a, T> +where + T: GuestType, +{ + ptr: GuestPtrMut<'a, T>, + num_elems: u32, +} + +impl<'a, T: GuestType + fmt::Debug> fmt::Debug for GuestArrayMut<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "GuestArrayMut {{ ptr: {:?}, num_elems: {:?} }}", + self.ptr, self.num_elems + ) + } +} + +impl<'a, T> GuestArrayMut<'a, T> +where + T: GuestTypeCopy, +{ + pub fn as_ref(&self) -> Result, GuestError> { + let arr = GuestArray { + ptr: self.ptr.as_immut(), + num_elems: self.num_elems, + }; + arr.as_ref() + } + + pub fn as_ref_mut(&self) -> Result, GuestError> { + let mut ptr = self.ptr.as_immut(); + for _ in 0..self.num_elems { + ptr = ptr.elem(1)?; + T::validate(&ptr)?; + } + let region = self.ptr.region.extend((self.num_elems - 1) * T::size()); + let handle = { + let mut borrows = self.ptr.mem.borrows.borrow_mut(); + borrows + .borrow_mut(region) + .ok_or_else(|| GuestError::PtrBorrowed(region))? + }; + let ref_mut = GuestRefMut { + mem: self.ptr.mem, + region, + handle, + type_: self.ptr.type_, + }; + Ok(GuestArrayRefMut { + ref_mut, + num_elems: self.num_elems, + }) + } +} + +pub struct GuestArrayRefMut<'a, T> +where + T: GuestType, +{ + ref_mut: GuestRefMut<'a, T>, + num_elems: u32, +} + +impl<'a, T> fmt::Debug for GuestArrayRefMut<'a, T> +where + T: GuestType + fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "GuestArrayRefMut {{ ref_mut: {:?}, num_elems: {:?} }}", + self.ref_mut, self.num_elems + ) + } +} + +impl<'a, T> ::std::ops::Deref for GuestArrayRefMut<'a, T> +where + T: GuestTypeCopy, +{ + type Target = [T]; + + fn deref(&self) -> &Self::Target { + unsafe { + std::slice::from_raw_parts( + self.ref_mut.as_ptr().as_raw() as *const T, + self.num_elems as usize, + ) + } + } +} + +impl<'a, T> ::std::ops::DerefMut for GuestArrayRefMut<'a, T> +where + T: GuestTypeCopy, +{ + fn deref_mut(&mut self) -> &mut Self::Target { + unsafe { + std::slice::from_raw_parts_mut( + self.ref_mut.as_ptr_mut().as_raw() as *mut T, + self.num_elems as usize, + ) + } + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[repr(align(4096))] + struct HostMemory { + buffer: [u8; 4096], + } + + impl HostMemory { + pub fn new() -> Self { + Self { buffer: [0; 4096] } + } + pub fn as_mut_ptr(&mut self) -> *mut u8 { + self.buffer.as_mut_ptr() + } + pub fn len(&self) -> usize { + self.buffer.len() + } + } + + #[test] + fn guest_array_out_of_bounds() { + let mut host_memory = HostMemory::new(); + let guest_memory = GuestMemory::new(host_memory.as_mut_ptr(), host_memory.len() as u32); + // try extracting an immutable array out of memory bounds + let ptr: GuestPtr = guest_memory.ptr(4092).expect("ptr to last i32 el"); + let err = ptr.array(2).expect_err("out of bounds ptr error"); + assert_eq!(err, GuestError::PtrOutOfBounds(Region::new(4092, 8))); + // try extracting an mutable array out of memory bounds + let ptr: GuestPtrMut = guest_memory.ptr_mut(4092).expect("ptr mut to last i32 el"); + let err = ptr.array_mut(2).expect_err("out of bounds ptr error"); + assert_eq!(err, GuestError::PtrOutOfBounds(Region::new(4092, 8))); + } + + #[test] + fn guest_array() { + let mut host_memory = HostMemory::new(); + let guest_memory = GuestMemory::new(host_memory.as_mut_ptr(), host_memory.len() as u32); + // write a simple array into memory + { + let ptr: GuestPtrMut = guest_memory.ptr_mut(0).expect("ptr mut to first el"); + let mut el = ptr.as_ref_mut().expect("ref mut to first el"); + *el = 1; + let ptr: GuestPtrMut = guest_memory.ptr_mut(4).expect("ptr mut to second el"); + let mut el = ptr.as_ref_mut().expect("ref mu to second el"); + *el = 2; + let ptr: GuestPtrMut = guest_memory.ptr_mut(8).expect("ptr mut to third el"); + let mut el = ptr.as_ref_mut().expect("ref mut to third el"); + *el = 3; + } + // extract as array + let ptr: GuestPtr = guest_memory.ptr(0).expect("ptr to first el"); + let arr = ptr.array(3).expect("convert ptr to array"); + let as_ref = arr.as_ref().expect("array borrowed immutably"); + assert_eq!(&*as_ref, &[1, 2, 3]); + // borrowing again should be fine + let as_ref2 = arr.as_ref().expect("array borrowed immutably again"); + assert_eq!(&*as_ref2, &*as_ref); + } + + #[test] + fn guest_array_mut() { + let mut host_memory = HostMemory::new(); + let guest_memory = GuestMemory::new(host_memory.as_mut_ptr(), host_memory.len() as u32); + // set elems of array to zero + { + let ptr: GuestPtrMut = guest_memory.ptr_mut(0).expect("ptr mut to first el"); + let mut el = ptr.as_ref_mut().expect("ref mut to first el"); + *el = 0; + let ptr: GuestPtrMut = guest_memory.ptr_mut(4).expect("ptr mut to second el"); + let mut el = ptr.as_ref_mut().expect("ref mu to second el"); + *el = 0; + let ptr: GuestPtrMut = guest_memory.ptr_mut(8).expect("ptr mut to third el"); + let mut el = ptr.as_ref_mut().expect("ref mut to third el"); + *el = 0; + } + // extract as array and verify all is zero + let ptr: GuestPtrMut = guest_memory.ptr_mut(0).expect("ptr mut to first el"); + let arr = ptr.array_mut(3).expect("convert ptr mut to array mut"); + assert_eq!(&*arr.as_ref().expect("array borrowed immutably"), &[0; 3]); + // populate the array and re-verify + for el in &mut *arr.as_ref_mut().expect("array borrowed mutably") { + *el = 10; + } + // re-validate + assert_eq!(&*arr.as_ref().expect("array borrowed immutably"), &[10; 3]); + } + + #[test] + #[should_panic( + expected = "array borrowed immutably while borrowed mutably: PtrBorrowed(Region { start: 0, len: 12 })" + )] + fn guest_array_mut_borrow_checker_1() { + let mut host_memory = HostMemory::new(); + let guest_memory = GuestMemory::new(host_memory.as_mut_ptr(), host_memory.len() as u32); + let ptr: GuestPtrMut = guest_memory.ptr_mut(0).expect("ptr mut to first el"); + let arr = GuestArrayMut { ptr, num_elems: 3 }; + // borrow mutably + let _as_mut = arr + .as_ref_mut() + .expect("array borrowed mutably for the first time"); + // borrow immutably should be fine + let _as_ref = arr + .as_ref() + .expect("array borrowed immutably while borrowed mutably"); + } + + #[test] + #[should_panic( + expected = "array borrowed mutably while borrowed mutably: PtrBorrowed(Region { start: 0, len: 12 })" + )] + fn guest_array_mut_borrow_checker_2() { + let mut host_memory = HostMemory::new(); + let guest_memory = GuestMemory::new(host_memory.as_mut_ptr(), host_memory.len() as u32); + let ptr: GuestPtrMut = guest_memory.ptr_mut(0).expect("ptr mut to first el"); + let arr = GuestArrayMut { ptr, num_elems: 3 }; + // borrow mutably + let _as_mut = arr + .as_ref_mut() + .expect("array borrowed mutably for the first time"); + // try borrowing mutably again + let _as_mut2 = arr + .as_ref_mut() + .expect("array borrowed mutably while borrowed mutably"); + } +} diff --git a/crates/memory/src/region.rs b/crates/memory/src/region.rs index 7ee542f388..fbbe752c85 100644 --- a/crates/memory/src/region.rs +++ b/crates/memory/src/region.rs @@ -25,6 +25,13 @@ impl Region { rhs_end >= self_start } } + + pub fn extend(&self, new_len: u32) -> Self { + Self { + start: self.start, + len: self.len + new_len, + } + } } #[cfg(test)]