kernel/process_{standard,loading}: don't create &mut ref to app memory #4714
+91
−39
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 reference _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.
lschuermann
force-pushed
the
dev/process-standard-create-app-memory-raw-slice-ptr
branch
from
8a32455 to
15c17a9
Compare
This was referenced Jan 8, 2026
jrvanwhy
reviewed
Jan 9, 2026
This changes PR #4714 to avoid `as` casting pointers. Instead, this uses methods to perform the casts (mostly `cast` and `addr`), which IMO makes it easier to understand what the code is doing. This also changes `ProcessStandard::create` to derive the provenance of pointers from `remaining_memory` rather than picking it up from an exposed provenance. I *think* this is a bugfix.
2 tasks
Remove pointer `as` casts from PR #4714.
Also reported upstream as rust-lang/rust#150995.
jrvanwhy
approved these changes
Jan 12, 2026
bradjc
reviewed
Jan 13, 2026
Comment on lines
709
to
718
| let load_result = load_process( | ||
| self.kernel, | ||
| self.chip, | ||
| process_binary, | ||
| self.app_memory.take(), | ||
| self.app_memory.get(), | ||
| short_app_id, | ||
| index, | ||
| self.fault_policy, | ||
| self.storage_policy, | ||
| ); |
Contributor
There was a problem hiding this comment.
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.
Member
Author
There was a problem hiding this comment.
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?
| return Err((ProcessLoadError::NotEnoughMemory, remaining_memory)); | ||
| } | ||
| }; | ||
|
|
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Overview
This changes
ProcessStandard::createand 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 inno updates are required./docs, orFormatting
make prepush.