User-defined misaligned MPU regions can override kernel-defined regions on ARM_CM3_MPU

While the docs are extensively mentioning that all regions should be aligned to power of two for correct MPU configuration, it is not explicit enough that not aligning (or not sanitizing) a user-defined region or a stack region may override MPU settings defined for higher priority kernel regions.

For example, for the ARM_CM3_MPU port, in port.c at lines 1318 (https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/ce221a8bb468e462ca6b435cef66a9636e00baf4/portable/GCC/ARM_CM3_MPU/port.c#L1318):

xMPUSettings->xRegion[ 0 ].ulRegionBaseAddress =
    ( ( uint32_t ) pxBottomOfStack ) |
    ( portMPU_REGION_VALID ) |
    ( portSTACK_REGION ); /* Region number. */

will obviously escalate the actual region priority from 3 (value of portSTACK_REGION) to 7 (value of portPRIVILEGED_RAM_REGION) if pxBottomOfStack is maliciously or inadvertently shifted by 4 bytes (for example passing 0x20000004 instead of 0x20000000).

Same issue for user-defined regions which has the same bitwise calculation without masking.

This is in particular dangerous for implementers still using MPU wrappers v1 (which allows restricted tasks to create other restricted tasks, allowing to pass arbitrary misaligned stack pointers), and dangerous for implementers using MPU wrappers v2 who are not aware and might blindly trust user defined pointers thinking that the higher priority MPU regions will guard them.

Is this intentional? Why not adding assertions/masking to make sure addresses are correctly aligned? At the very least, maybe the docs and function headers should be updated to notify of the dangers of arbitrary stack and user-defined regions.

Good point - we should mask the bits not meant for address. Would you be willing to raise a PR?

Sure, I created https://github.com/FreeRTOS/FreeRTOS-Kernel/pull/1473 :slight_smile: Note that I only applied the change to the port I’m mentioning. I’ll extend the change to other MPU ports if the idea is approved.