Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the Allowed type to libtock_platform. #222

Merged
merged 3 commits into from
Aug 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions core/platform/src/allows/allow_readable.rs
Original file line number Diff line number Diff line change
@@ -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 {}
85 changes: 85 additions & 0 deletions core/platform/src/allows/allowed.rs
Original file line number Diff line number Diff line change
@@ -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<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: 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<T>) -> 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())
}
}
124 changes: 124 additions & 0 deletions core/platform/src/allows/allowed_tests.rs
Original file line number Diff line number Diff line change
@@ -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<T>,

// 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);
}

jrvanwhy marked this conversation as resolved.
Show resolved Hide resolved
#[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);
}
8 changes: 8 additions & 0 deletions core/platform/src/allows/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
mod allow_readable;
mod allowed;

pub use allow_readable::AllowReadable;
pub use allowed::Allowed;

#[cfg(test)]
mod allowed_tests;
2 changes: 2 additions & 0 deletions core/platform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;