kernel/process_{standard,loading}: don't create &mut ref to app memory

[lschuermann]

Pull Request Overview

This changes ProcessStandard::create and its users in process loading to operate on app memory referenced by a raw slice pointer, instead of a unique (mutable) Rust reference.

Currently, during process loading, Tock creates and stores Rust multiple shared and unique Rust references pointing into process memory. This is dangerous, for multiple reasons: first, the pointee may not be properly initialized memory, violating a basic Rust invariant [1]. Furthermore, the memory may be concurrently accessed and modified by the process, either directly (in the case of write-acessible RAM), or indirectly through system calls to a flash controller.

We should avoid creating Rust references into process memory, except for rare, carefully reasoned about cases. This includes the process buffer infrastructure, to give capsules ephemeral access to select regions in process memory, and the kernel's grant region.

Even if we would only use Rust references during process loading, and only on memory that was previously initialized, this is still hard to reason about. For instance, we would have to ensure that no process runs after all processes have been loaded, and all Rust references to process memory have gone out of scope. Additions like dynamic process loading make establishing and reasoning about these guarantees difficult. Instead, using raw slice pointers (*mut [u8]) is a more obviously safer choice, onto which Rust places much fewer safety requirements.

Testing Strategy

Compiling and careful review?

TODO or Help Wanted

N/A

Documentation Updated

• Updated the relevant files in /docs, or no updates are required.

Formatting

• Ran make prepush.

[bradjc]

Why this change? Semantically, we are passing ownership of the remaining app memory to the load process function and then getting back the updated memory region when the function returns.

[lschuermann]

Because *mut [u8] cannot not implement Default (it's missing pointer metadata) whereas &'static mut [u8] does (where the default value is an empty slice).

app_memory is a Cell<>, not an OptionalCell<> or TakeCell<>. I agree that what we're doing conceptually is taking and then replacing, but what the previous code did with take() is immediately replace app_memory with the empty slice. Which is probably actually not what we wanted to do.

I can change this to an OptionalCell if you prefer, and then restore the .take() semantics?

[bradjc]

I do think it's worth changing, but shouldn't it be a TakeCell? I know that isn't what we normally use, but typically things which are memory are typically not copyable.

[lschuermann]

So TakeCell must hold a reference with a given lifetime, but this is now a raw pointer. So it wouldn't work. OptionalCell would, which I can change this to.

Which raises an orthogonal question I've had for a long time: why do we have TakeCell<'a, T> when you can use OptionalCell<&'a T>MapCell<&'a T>? It seems like ~~OptionalCell~~MapCellsupports a strict superset of the types thatTakeCelldoes, with almost the same APIs. Shouldn'tTakeCell` be removed?

[jrvanwhy]

This is totally outside the scope of this PR, but IMO OptionalCell should be deprecated. Nowadays Cell<Option<T>> has almost all the functionality we want. If we still want the remaining utilities (is_some, is_none, contains, etc) then an extension trait seems like a better place for them.

[lschuermann]

I agree, and I actually meant to say MapCell above, which works well for objects that aren't Copy such as unique references.

[bradjc]

I'm confused. Can we use MapCell then? Something with take() semantics. It seems too easy to lose track of who owns the pointer when not using slices.

[lschuermann]

Sure, I'll change it to use MapCell.

[lschuermann]

Now changed to use MapCell.