Arch arm mpu assert additions and fixes #8028
Conversation
| @@ -273,8 +273,7 @@ void arm_core_mpu_configure(u8_t type, u32_t base, u32_t size) | |||
| u32_t region_index = _get_region_index_by_type(type); | |||
| u32_t region_attr = _get_region_attr_by_type(type, size); | |||
|
|
|||
| /* ARM MPU supports up to 16 Regions */ | |||
| if (region_index > _get_num_regions()) { | |||
There was a problem hiding this comment.
This looks like an out-of-bounds sanity-check bug, @agross-linaro, am I correct?
Codecov Report
@@ Coverage Diff @@
## master #8028 +/- ##
==========================================
+ Coverage 64.54% 64.54% +<.01%
==========================================
Files 420 420
Lines 40131 40131
Branches 6759 6759
==========================================
+ Hits 25902 25903 +1
Misses 11106 11106
+ Partials 3123 3122 -1
Continue to review full report at Codecov.
|
b406067 to
cfb5167
Compare
|
FYI @andrewboie @agross-linaro @carlescufi . I think that this PR overlaps partially with #7902 . |
There was a problem hiding this comment.
I think this code does too much.
It's sufficient to just make assertion checks, not try to prevent the system from crashing if assertions are off and people feed garbage data to the MPU driver. We only do this kind of rigorous runtime checking for drivers that we expose to user mode.
arch/arm/core/cortex_m/mpu/arm_mpu.c
Outdated
| if (size > 0) | ||
| _region_init(index, (u32_t)&__app_ram_start, region_attr); | ||
| #endif | ||
| if (index < _get_num_regions()) |
cfb5167 to
12a3fb4
Compare
arch/arm/core/cortex_m/mpu/arm_mpu.c
Outdated
| @@ -143,7 +143,7 @@ static inline u32_t _get_region_index_by_type(u32_t type) | |||
| } | |||
|
|
|||
| /** | |||
| * This internal function check if region is enabled or not | |||
| * This internal function checks if region is enabled or not. | |||
There was a problem hiding this comment.
Can you please make the same comment fixes in the nxp mpu?
This commit fixes some minor issues with coding style and comment syntax in arm_mpu.c Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
This commit enhaces the documentation of internal functions in arm_mpu.c by explicitly stating that the caller needs to ensure the validity of the supplied MPU region index. The warning is required as these functions modify the ARM MPU_RNR register, without checking themselves the validity of the provided region number. Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
This commit adds an internal function to disable an MPU region. The function includes an assert that the requested MPU region number is a valid one. arm_mpu.c is refactor to use this function in all cases where an MPU region needs to be disabled. Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
This commit inserts sanity checks every time we are performing a (re)-configuration of one or multiple MPU regions, ensuring that we do not attempt to configure an invalid region number. Particulary for arm_mpu_config(), called during pre-kernel initialization phase, we add a system ASSERT if we attempt to initialize more regions that what is supported by hardware. We do this to ensure the misconfiguration is detected early and the system boot is aborted. Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
This commit fixes some minor function documentation issues and comments' style in the NXP_MPU driver. Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
12a3fb4 to
221c446
Compare
|
@MaureenHelm I pushed an additional commit that implements the comment&style fixes in NXP_MPU. |
Addresses #7384.
Addresses an additional bug related to region out-of-bounds configuration (see relevant comment).
Adds some minor fixes to the code and comment enhancements.