Merge branch 'master' into allowed
diff --git a/core/platform/src/allows/allow_readable.rs b/core/platform/src/allows/allow_readable.rs new file mode 100644 index 0000000..c35701b --- /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 0000000..db8ba26 --- /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. +// Design note: 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. +pub struct Allowed<'b, T: '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<T>, + + // 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: '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<T>) -> Allowed<'b, T> { + Allowed { + buffer, + _phantom: core::marker::PhantomData, + } + } + + // Sets the value in the buffer. Note that unlike `core::cell::Cell::set`, + // `Allowed::set` will not drop the existing value. To drop the existing + // value, use `replace` and drop the return value. + pub fn set(&self, value: T) { + unsafe { + core::ptr::write_volatile(self.buffer.as_ptr(), value); + } + } +} + +impl<'b, T: crate::AllowReadable + '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 + } +} + +impl<'b, T: crate::AllowReadable + Copy + 'b> Allowed<'b, T> { + pub fn get(&self) -> T { + unsafe { core::ptr::read_volatile(self.buffer.as_ptr()) } + } +} + +impl<'b, T: crate::AllowReadable + 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 0000000..ec6a358 --- /dev/null +++ b/core/platform/src/allows/allowed_tests.rs
@@ -0,0 +1,184 @@ +use crate::allows::test_util::DropCheck; +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: 'b> { + ptr: NonNull<T>, + + // We need to consume the 'b lifetime. This is very similar to Allowed's + // implementation. + _phantom: PhantomData<&'b mut T>, +} + +impl<'b, T: '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. Does not drop the + // existing value. + pub fn set(&self, value: T) { + unsafe { + core::ptr::write(self.ptr.as_ptr(), value); + } + } +} + +impl<'b, T: Copy + 'b> KernelPtr<'b, T> { + // Copies the contained value out of the buffer. + pub fn get(&self) -> T { + unsafe { core::ptr::read(self.ptr.as_ptr()) } + } +} + +impl<'b, 'f: 'b> KernelPtr<'b, DropCheck<'f>> { + // Retrieves the value of the contained DropCheck. + pub fn value(&self) -> usize { + let drop_check = unsafe { core::ptr::read(self.ptr.as_ptr()) }; + let value = drop_check.value; + core::mem::forget(drop_check); + value + } +} + +#[test] +fn set() { + let dropped1 = core::cell::Cell::new(false); + let dropped2 = core::cell::Cell::new(false); + let dropped3 = core::cell::Cell::new(false); + let mut buffer = DropCheck { + flag: Some(&dropped1), + value: 1, + }; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.value(), 1); + assert_eq!(dropped1.get(), false); + + // Simulate the kernel replacing the value in buffer. We don't drop the + // existing value. + kernel_ptr.set(DropCheck { + flag: Some(&dropped2), + value: 2, + }); + allowed.set(DropCheck { + flag: Some(&dropped3), + value: 3, + }); + assert_eq!(kernel_ptr.value(), 3); + assert_eq!(dropped1.get(), false); + assert_eq!(dropped2.get(), false); + assert_eq!(dropped3.get(), false); +} + +#[test] +fn replace() { + let dropped1 = core::cell::Cell::new(false); + let dropped2 = core::cell::Cell::new(false); + let dropped3 = core::cell::Cell::new(false); + let mut buffer = DropCheck { + flag: Some(&dropped1), + value: 1, + }; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.value(), 1); + assert_eq!(dropped1.get(), false); + + // Simulate the kernel replacing the value in buffer. We don't drop the + // existing value. + kernel_ptr.set(DropCheck { + flag: Some(&dropped2), + value: 2, + }); + let returned = allowed.replace(DropCheck { + flag: Some(&dropped3), + value: 3, + }); + assert_eq!(returned.value, 2); + assert_eq!(kernel_ptr.value(), 3); + assert_eq!(dropped1.get(), false); + assert_eq!(dropped2.get(), false); + assert_eq!(dropped3.get(), false); +} + +#[test] +fn get() { + // We can't use DropCheck because Drop and Copy cannot both be implemented + // on a single type. + let mut buffer = 1; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + 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 dropped1 = core::cell::Cell::new(false); + let dropped2 = core::cell::Cell::new(false); + let dropped3 = core::cell::Cell::new(false); + let mut buffer = DropCheck { + flag: Some(&dropped1), + value: 1, + }; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.value(), 1); + assert_eq!(dropped1.get(), false); + + // Simulate the kernel replacing the value in buffer. We don't drop the + // existing value. + kernel_ptr.set(DropCheck { + flag: Some(&dropped2), + value: 2, + }); + let returned = allowed.take(); + assert_eq!(returned.value, 2); + assert_eq!(kernel_ptr.value(), 0); + assert_eq!(dropped1.get(), false); + assert_eq!(dropped2.get(), false); + assert_eq!(dropped3.get(), false); +}
diff --git a/core/platform/src/allows/mod.rs b/core/platform/src/allows/mod.rs new file mode 100644 index 0000000..410d158 --- /dev/null +++ b/core/platform/src/allows/mod.rs
@@ -0,0 +1,10 @@ +mod allow_readable; +mod allowed; + +pub use allow_readable::AllowReadable; +pub use allowed::Allowed; + +#[cfg(test)] +mod allowed_tests; +#[cfg(test)] +mod test_util;
diff --git a/core/platform/src/allows/test_util.rs b/core/platform/src/allows/test_util.rs new file mode 100644 index 0000000..0f63d9c --- /dev/null +++ b/core/platform/src/allows/test_util.rs
@@ -0,0 +1,37 @@ +//! Contains testing utilities for libtock_platform::allowed. + +// A value that can be placed in an Allowed that checks whether it has been +// dropped. +#[derive(Default)] +pub(crate) struct DropCheck<'f> { + pub flag: Option<&'f core::cell::Cell<bool>>, + pub value: usize, +} + +impl<'f> Drop for DropCheck<'f> { + fn drop(&mut self) { + if let Some(flag) = self.flag { + flag.set(true); + } + } +} + +// Note: DropCheck cannot be safely used in a non-test context, as DropCheck +// does not satisfy AllowReadable's "every bit pattern is valid" requirement. +// However, in the tests we aren't sharing the buffer with a real Tock kernel, +// and instead we guarantee that only valid instances of DropCheck are loaded +// into the buffer. +unsafe impl<'f> crate::AllowReadable for DropCheck<'f> {} + +// Verify that DropCheck works, as none of the tests require it to actually set +// the drop flag. +#[test] +fn drop_check() { + let flag = core::cell::Cell::new(false); + let drop_check = DropCheck { + flag: Some(&flag), + value: 0, + }; + drop(drop_check); + assert_eq!(flag.get(), true); +}
diff --git a/core/platform/src/lib.rs b/core/platform/src/lib.rs index 24c46f5..11543ed 100644 --- a/core/platform/src/lib.rs +++ b/core/platform/src/lib.rs
@@ -8,6 +8,8 @@ // 3. A system call trait so that Platform works in both real Tock apps and // unit test environments. +mod allows; mod error_code; +pub use allows::{AllowReadable, Allowed}; pub use error_code::ErrorCode;