mpu fixes for v1.8-branch #655
Merged
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
The NXP MPU requires special handling of region descriptor 0 to guarantee that the debugger has access to the entire address space. It does not allow writes from the core to affect the start or end addresses, or the permissions associated with the debugger. The original implementation of this driver attempted to work around region descriptor 0, resulting in an off-by-1 error caught by Coverity. Instead, define region descriptor 0 explicitly in the mpu_regions array, and add some asserts to ensure that one doesn't try to change its start or end addresses. This has an added benefit such that more permissions can be enabled in region 0 if desired, whereas the previous implementation always forced all writable permissions to be cleared. Coverity-CID: 170473 Jira: ZEP-2258 Signed-off-by: Maureen Helm <[email protected]>
Both the ARM and NXP MPU drivers incorrectly calculated the region index by assuming the region type (e.g., THREAD_STACK_GUARD_REGION) was zero-indexed, when in reality it is one-indexed. This had the effect of wasting one region. Signed-off-by: Maureen Helm <[email protected]>
Clearing fields in the region descriptor attributes doesn't always have the expected effect of revoking permissions. In the case of bus master supervisor mode fields (MxSM), setting to zero actually enables read, write, and execute access. When we reworked handling of region descriptor 0, we inadvertently enabled execution from RAM by clearing the MxSM fields and enabling the descriptor. This caused samples/mpu_test run to throw a usage fault instead of an MPU-triggered bus fault. Fix this by setting all the MxSM fields to 2'b11, which gives supervisor mode the same access as user mode. Signed-off-by: Maureen Helm <[email protected]>
ztest provides a ztest_test_fail() interface to fail the currently running test, but does not provide an equivalent ztest_test_pass(). Normally a test passes just by returning without an assertion failure or other call to ztest_test_fail(). However, if the correct behavior for a test is to trigger a fatal fault (as with tests/kernel/fatal or protection or MPU tests), then we need a way for the test to pass the currently running test before aborting the current thread. Otherwise, ztest hangs forever in run_test() on the k_sem_take(&test_end_signal, K_FOREVER) call. Add a ztest_test_pass() interface and implement it for kernel and userspace variants of ztest. This interface will be used in the protection tests. Signed-off-by: Stephen Smalley <[email protected]>
Add a self-protection test suite with a set of tests to check whether one can overwrite read-only data and text, and whether one can execute from data, stack, or heap buffers. These tests are modeled after a subset of the lkdtm tests in the Linux kernel. These tests have twice caught bugs in the Zephyr NXP MPU driver, once during initial testing/review of the code (in its earliest forms on gerrit, reported to the original author there) and most recently the regression introduced by commit bacbea6 ("arm: nxp: mpu: Rework handling of region descriptor 0"), which was fixed by commit a8aa9d4 ("arm: nxp: mpu: Fix region descriptor 0 attributes") after being reported. This is intended to be a testsuite of self-protection features rather than just a test of MPU functionality. It is envisioned that these tests will be expanded to cover a wider range of protection features beyond just memory protection, and the current tests are independent of any particular enforcement mechanism (e.g. MPU, MMU, or other). The tests are intended to be cross-platform, and have been built and run on both x86- and ARM-based boards. The tests currently fail on x86-based boards, but this is an accurate reflection of current protections and should change as MMU support arrives. The tests leverage the ztest framework, making them suitable for incorporation into automated regression testing for Zephyr. Signed-off-by: Stephen Smalley <[email protected]>
dbkinder
approved these changes
Jul 5, 2017
|
@nashif Do you have any idea why this is failing checkpatch now? How did it get through before? |
|
FWIW, the offending line was lifted verbatim from tests/kernel/fatal/src/main.c. Initially, I encountered this same error from running checkpatch locally, and consequently inserted a space to avoid it (even though I think checkpatch is wrong here). But then when I submitted the pull request, one of the shippable runs failed complaining that the space shouldn't be there! So I removed it again and shippable was happy with it. I've seen very unstable behavior from shippable runs, not sure why. |
nagineni
pushed a commit
to nagineni/zephyr
that referenced
this pull request
Nov 20, 2017
…yrproject-rtos#655) Also, remove the entire JerryScript build directory which got dropped somehow. (This cleans builds for all boards at once.) Signed-off-by: Geoff Gustafson <[email protected]>
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.
Several mpu issues were found and fixed very close to the 1.8 release, but were not included in the release do to the risk of breaking other things (see #441). Now that these mpu fixes have been in master for a while, cherry-pick them to the 1.8-branch so they can be included in a future 1.8.1 release. Also cherry-pick the new test from @stephensmalley that caught one of the mpu issues.