diff --git a/core/platform/src/allows/allow_readable.rs b/core/platform/src/allows/allow_readable.rs new file mode 100644 index 00000000..c35701bf --- /dev/null +++ b/core/platform/src/allows/allow_readable.rs @@ -0,0 +1,18 @@ +/// Because the kernel may modify shared memory and place any bit pattern into +/// that memory, we can only read types out of shared memory if every bit +/// pattern is a valid value of that type. `AllowReadable` types are safe to +/// read out of a shared buffer. +pub unsafe trait AllowReadable {} + +unsafe impl AllowReadable for i8 {} +unsafe impl AllowReadable for i16 {} +unsafe impl AllowReadable for i32 {} +unsafe impl AllowReadable for i64 {} +unsafe impl AllowReadable for i128 {} +unsafe impl AllowReadable for isize {} +unsafe impl AllowReadable for u8 {} +unsafe impl AllowReadable for u16 {} +unsafe impl AllowReadable for u32 {} +unsafe impl AllowReadable for u64 {} +unsafe impl AllowReadable for u128 {} +unsafe impl AllowReadable for usize {} diff --git a/core/platform/src/allows/allowed.rs b/core/platform/src/allows/allowed.rs new file mode 100644 index 00000000..4d66c2b2 --- /dev/null +++ b/core/platform/src/allows/allowed.rs @@ -0,0 +1,85 @@ +/// An individual value that has been shared with the kernel using the `allow` +/// system call. +// Allowed's implementation does not directly use the 'b lifetime. Platform uses +// 'b to prevent the Allowed from accessing the buffer after the buffer becomes +// invalid. +// Allowed requires T to be Copy due to concerns about the semantics of +// non-copyable types in shared memory as well as concerns about unexpected +// behavior with Drop types. See the following PR discussion for more +// information: https://github.com/tock/libtock-rs/pull/222 +pub struct Allowed<'b, T: Copy + 'b> { + // Safety properties: + // 1. `buffer` remains valid and usable for the lifetime of this Allowed + // instance. + // 2. Read and write accesses of `buffer`'s pointee must be performed as a + // volatile operation, as the kernel may mutate buffer's pointee at any + // time. + // 3. The value `buffer` points to may have an arbitrary bit pattern in + // it, so reading from `buffer` is only safe if the type contained + // within is AllowReadable. + buffer: core::ptr::NonNull, + + // We need to use the 'b lifetime in Allowed to prevent an "unused lifetime" + // compiler error. We use it here. Note that the phantom data must be an + // &mut rather than a shared reference in order to make Allowed invariant + // with respect to T. Invariance is required because Allowed allows us to + // mutate the value in buffer, and invariance is the required property to do + // so without allowing callers to produce dangling references. This is + // documented at https://doc.rust-lang.org/nomicon/subtyping.html. + _phantom: core::marker::PhantomData<&'b mut T>, +} + +// Allowed's API is based on that of core::cell::Cell, but removes some methods +// that are not safe for use with shared memory. +// +// Internally, Allowed performs accesses to the shared memory using volatile +// reads and writes through raw pointers. We avoid constructing references to +// shared memory because that leads to undefined behavior (there is some +// background on this in the following discussion: +// https://github.com/rust-lang/unsafe-code-guidelines/issues/33). Tock runs on +// single-threaded platforms, some of which lack atomic instructions, so we only +// need to be able to deconflict races between the kernel (which will never +// interrupt an instruction's execution) and this process. Therefore volatile +// accesses are sufficient to deconflict races. +impl<'b, T: Copy + 'b> Allowed<'b, T> { + // Allowed can only be constructed by the Platform. It is constructed after + // the `allow` system call, and as such must accept a raw pointer rather + // than a reference. The caller must make sure the following are true: + // 1. buffer points to a valid instance of type T + // 2. There are no references to buffer's pointee + // 3. buffer remains usable until the Allowed's lifetime has ended. + #[allow(dead_code)] // TODO: Remove when Platform is implemented + pub(crate) unsafe fn new(buffer: core::ptr::NonNull) -> Allowed<'b, T> { + Allowed { + buffer, + _phantom: core::marker::PhantomData, + } + } + + // Sets the value in the buffer. + pub fn set(&self, value: T) { + unsafe { + core::ptr::write_volatile(self.buffer.as_ptr(), value); + } + } +} + +impl<'b, T: crate::AllowReadable + Copy + 'b> Allowed<'b, T> { + pub fn replace(&self, value: T) -> T { + let current = unsafe { core::ptr::read_volatile(self.buffer.as_ptr()) }; + unsafe { + core::ptr::write_volatile(self.buffer.as_ptr(), value); + } + current + } + + pub fn get(&self) -> T { + unsafe { core::ptr::read_volatile(self.buffer.as_ptr()) } + } +} + +impl<'b, T: crate::AllowReadable + Copy + Default + 'b> Allowed<'b, T> { + pub fn take(&self) -> T { + self.replace(T::default()) + } +} diff --git a/core/platform/src/allows/allowed_tests.rs b/core/platform/src/allows/allowed_tests.rs new file mode 100644 index 00000000..f1f0c083 --- /dev/null +++ b/core/platform/src/allows/allowed_tests.rs @@ -0,0 +1,124 @@ +use crate::Allowed; +use core::marker::PhantomData; +use core::ptr::NonNull; + +// How do we simulate accesses to the shared buffer by the kernel? +// +// Well, a naive way would be to mutate the `buffer` variable directly. Because +// Allowed accesses the memory through a *mut pointer, such a test would compile +// and run fine with the current Rust compiler. As far as I can tell, it would +// not hit any behavior documented as undefined at +// https://doc.rust-lang.org/stable/reference/behavior-considered-undefined.html, +// nor would it cause rustc to generate LLVM bitcode that encounters undefined +// behavior. +// +// However, the naive approach will throw an "undefined behavior" error when run +// under Miri (e.g. with `cargo miri test`), which uses the stacked borrows +// model [1]. In particular, accessing the `buffer` variable directly pops the +// SharedRW off buffer's borrow stack, which prevents Allowed from using its +// *mut pointer to access `buffer` afterwards. It is likely that Rust will adopt +// the stacked borrows model as its formal model for borrow validity, and in +// that case accessing `buffer` in that manner will become undefined behavior. +// In addition, running these tests under Miri is highly valuable, as this is +// tricky code to get correct and an unsound API may be hard to fix. +// +// Instead, we explicitly refer to buffer through use of a KernelPtr that +// simulates the pointer that `allow()` would hand to the Tock kernel. As far as +// the stacked borrows model is concerned, accesses through the KernelPtr +// variable behave identically to mutations performed by the kernel. This +// pattern allows us to use `cargo miri test` to not only execute the unit +// tests, but to test whether Allowed would encounter undefined behavior when +// interacting with a real Tock kernel. +// +// [1] https://plv.mpi-sws.org/rustbelt/stacked-borrows/paper.pdf +struct KernelPtr<'b, T: Copy + 'b> { + ptr: NonNull, + + // We need to consume the 'b lifetime. This is very similar to Allowed's + // implementation. + _phantom: PhantomData<&'b mut T>, +} + +impl<'b, T: Copy + 'b> KernelPtr<'b, T> { + // The constructor for KernelPtr; simulates allow(). Returns both the + // Allowed instance the Platform would return and a KernelPtr the test can + // use to simulate a kernel. + pub fn allow(buffer: &'b mut T) -> (Allowed<'b, T>, KernelPtr<'b, T>) { + let ptr = NonNull::new(buffer).unwrap(); + // Discard buffer *without* creating a reference to it, as would be done + // if we called drop(). + let _ = buffer; + // All 3 preconditions of Allowed::new are satisfied by the fact that + // `buffer` is directly derived from a &'b mut T. + let allowed = unsafe { Allowed::new(ptr) }; + let kernel_ptr = KernelPtr { + ptr, + _phantom: PhantomData, + }; + (allowed, kernel_ptr) + } + + // Replaces the value in the buffer with a new one. + pub fn set(&self, value: T) { + unsafe { + core::ptr::write(self.ptr.as_ptr(), value); + } + } + + // Copies the contained value out of the buffer. + pub fn get(&self) -> T { + unsafe { core::ptr::read(self.ptr.as_ptr()) } + } +} + +#[test] +fn set() { + let mut buffer = 1; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.get(), 1); + + // Simulate the kernel replacing the value in buffer. + kernel_ptr.set(2); + allowed.set(3); + assert_eq!(kernel_ptr.get(), 3); +} + +#[test] +fn replace() { + let mut buffer = 1; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.get(), 1); + + // Simulate the kernel replacing the value in buffer. + kernel_ptr.set(2); + let returned = allowed.replace(3); + assert_eq!(returned, 2); + assert_eq!(kernel_ptr.get(), 3); +} + +#[test] +fn get() { + let mut buffer = 1; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.get(), 1); + + assert_eq!(allowed.get(), 1); + assert_eq!(kernel_ptr.get(), 1); + + kernel_ptr.set(2); + assert_eq!(allowed.get(), 2); + assert_eq!(kernel_ptr.get(), 2); +} + +#[test] +fn take() { + let mut buffer = 1; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.get(), 1); + + // Simulate the kernel replacing the value in buffer. + kernel_ptr.set(2); + let returned = allowed.take(); + assert_eq!(returned, 2); + assert_eq!(kernel_ptr.get(), 0); +} diff --git a/core/platform/src/allows/mod.rs b/core/platform/src/allows/mod.rs new file mode 100644 index 00000000..8cb21581 --- /dev/null +++ b/core/platform/src/allows/mod.rs @@ -0,0 +1,8 @@ +mod allow_readable; +mod allowed; + +pub use allow_readable::AllowReadable; +pub use allowed::Allowed; + +#[cfg(test)] +mod allowed_tests; diff --git a/core/platform/src/lib.rs b/core/platform/src/lib.rs index 2e3dec64..7237a1d3 100644 --- a/core/platform/src/lib.rs +++ b/core/platform/src/lib.rs @@ -8,8 +8,10 @@ // 3. A system call trait so that Platform works in both real Tock apps and // unit test environments. [DONE] +mod allows; mod error_code; mod syscalls; +pub use allows::{AllowReadable, Allowed}; pub use error_code::ErrorCode; pub use syscalls::{MemopNoArg, MemopWithArg, Syscalls};