arch/cortex-m3: Allow the kernel to access protected memory #2120
Conversation
This file contains 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
bradjc
previously approved these changes
Sep 22, 2020
|
Ha! I didn't realise they were symlinked, I checked the M4 after I changed the M3. Ignore that part then. |
ppannuto
previously approved these changes
Sep 22, 2020
ppannuto
added
the
release-blocker
hudson-ayers
previously approved these changes
Sep 22, 2020
Commit 128782d "mpu: Change the disable_mpu API" converted the disable_app_mpu() function to not make any changes. This means when we return from an app to the kernel we don't disable the MPU. This resulted in some access failures when the kernel tried to access direct write flash regions (tock#1873 (comment)). We can either disable the MPU when returning to the kernel or change the app MPU configuration permissions to not impact the kernel. This patch converts the Cortex-M3 MPU implementation to always allow the kernel access when configuring the app. This way we can avoid the overhead of disabling the MPU on context switches. There is no security gap here as the kernel could just disable the MPU anyway if it was malicious. Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
alistair23
dismissed stale reviews from hudson-ayers, ppannuto, and bradjc
via
7542a1c
alistair23
force-pushed
the
alistair/arm-mpu-fix
branch
from
9bba3a9
to
7542a1c
Compare
|
Fixed the format failure. |
hudson-ayers
approved these changes
Sep 22, 2020
bradjc
approved these changes
Sep 23, 2020
|
bors r+ |
2 tasks
bors bot
added a commit
that referenced
this pull request
Feb 18, 2021
2433: use const generics to remove duplicate code for cortex-m MPU r=bradjc a=hudson-ayers ### Pull Request Overview This pull request uses const generics to remove duplicate code for the Cortex-M MPU. It also moves mpu.rs into `arch/cortex-m`, rather than having it in `arch/cortex-m3` and using symbolic links elsewhere. Each specific architecture re-exports the generic type as a concrete type with the correct number of regions for that architecture. I think this is more in line with how we generally handle shared code in other places. I realize that now `cortex-m/mpu.rs` would not work for all `cortex-m` platforms (e.g. `cortex-m0`), but architectures for which this does not work should just not re-export the functionality, and instead present their own MPU type. `cortex-m/src/mpu.rs` clearly indicates in the comments that it is only for use on m3/m4/m7. We need generics because the cortex-m7 MPU has 16 regions instead of 8. The cortex-m7 MPU has already fallen out of date with the cortex-m3 MPU implementation because #2120 only updated the m3 implementation, and not the mostly duplicate code used in m7. I used the `pub mod MPU {}` wrapper in each `lib.rs` to avoid having to modify every chip to update the path, but it could be removed. ### Testing Strategy This pull request was tested by compiling and running blink on an Imix. ### TODO or Help Wanted N/A ### Documentation Updated - [x] No updates are required. ### Formatting - [x] Ran `make prepush`. Co-authored-by: Hudson Ayers <hayers@stanford.edu>
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
Commit 128782d "mpu: Change the disable_mpu API" converted the
disable_app_mpu() function to not make any changes. This means when we
return from an app to the kernel we don't disable the MPU.
This resulted in some access failures when the kernel tried to access
direct write flash regions (#1873 (comment)).
We can either disable the MPU when returning to the kernel or change the
app MPU configuration permissions to not impact the kernel.
This patch converts the Cortex-M3 MPU implementation to always allow the
kernel access when configuring the app. This way we can avoid the
overhead of disabling the MPU on context switches. There is no security
gap here as the kernel could just disable the MPU anyway if it was
malicious.
Signed-off-by: Alistair Francis alistair.francis@wdc.com
Testing Strategy
None.
TODO or Help Wanted
Documentation Updated
/docs, or no updates are required.Formatting
make prepush.