Skip to content

Commit

Permalink
Use highest numbered MPU regions for kernel
Browse files Browse the repository at this point in the history 
ARMv7-M allows overlapping MPU regions. When 2 MPU regions overlap, the
MPU configuration of the higher numbered MPU region is applied. For
example, if a memory area is covered by 2 MPU regions 0 and 1, the
memory permissions for MPU region 1 are applied.

We use 5 MPU regions for kernel code and kernel data protections and
leave the remaining for the application writer. We were using lowest
numbered MPU regions (0-4) for kernel protections and leaving the
remaining for the application writer. The application writer could
configure those higher numbered MPU regions to override kernel
protections.

This commit changes the code to use highest numbered MPU regions for
kernel protections and leave the remaining for the application writer.
This ensures that the application writer cannot override kernel
protections.

We thank the SecLab team at Northeastern University for reporting this
issue.

Signed-off-by: Gaurav Aggarwal <[email protected]>
  • Loading branch information
@aggarg
aggarg committed Sep 16, 2022
1 parent ca099b9 commit ea9c26f
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 134 deletions.
27 changes: 7 additions & 20 deletions portable/GCC/ARM_CM3_MPU/port.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -756,31 +756,18 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,
xMPUSettings->xRegion[ 0 ].ulRegionBaseAddress =
( ( uint32_t ) __SRAM_segment_start__ ) | /* Base address. */
( portMPU_REGION_VALID ) |
( portSTACK_REGION );
( portSTACK_REGION ); /* Region number. */

xMPUSettings->xRegion[ 0 ].ulRegionAttribute =
( portMPU_REGION_READ_WRITE ) |
( portMPU_REGION_CACHEABLE_BUFFERABLE ) |
( prvGetMPURegionSizeSetting( ( uint32_t ) __SRAM_segment_end__ - ( uint32_t ) __SRAM_segment_start__ ) ) |
( portMPU_REGION_ENABLE );

/* Re-instate the privileged only RAM region as xRegion[ 0 ] will have
* just removed the privileged only parameters. */
xMPUSettings->xRegion[ 1 ].ulRegionBaseAddress =
( ( uint32_t ) __privileged_data_start__ ) | /* Base address. */
( portMPU_REGION_VALID ) |
( portSTACK_REGION + 1 );

xMPUSettings->xRegion[ 1 ].ulRegionAttribute =
( portMPU_REGION_PRIVILEGED_READ_WRITE ) |
( portMPU_REGION_CACHEABLE_BUFFERABLE ) |
prvGetMPURegionSizeSetting( ( uint32_t ) __privileged_data_end__ - ( uint32_t ) __privileged_data_start__ ) |
( portMPU_REGION_ENABLE );

/* Invalidate all other regions. */
for( ul = 2; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
/* Invalidate user configurable regions. */
for( ul = 1UL; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
{
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( portSTACK_REGION + ul ) | portMPU_REGION_VALID;
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( ( ul - 1UL ) | portMPU_REGION_VALID );
xMPUSettings->xRegion[ ul ].ulRegionAttribute = 0UL;
}
}
Expand All @@ -807,7 +794,7 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,

lIndex = 0;

for( ul = 1; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
for( ul = 1UL; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
{
if( ( xRegions[ lIndex ] ).ulLengthInBytes > 0UL )
{
Expand All @@ -817,7 +804,7 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress =
( ( uint32_t ) xRegions[ lIndex ].pvBaseAddress ) |
( portMPU_REGION_VALID ) |
( portSTACK_REGION + ul ); /* Region number. */
( ul - 1UL ); /* Region number. */

xMPUSettings->xRegion[ ul ].ulRegionAttribute =
( prvGetMPURegionSizeSetting( xRegions[ lIndex ].ulLengthInBytes ) ) |
Expand All @@ -827,7 +814,7 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,
else
{
/* Invalidate the region. */
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( portSTACK_REGION + ul ) | portMPU_REGION_VALID;
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( ( ul - 1UL ) | portMPU_REGION_VALID );
xMPUSettings->xRegion[ ul ].ulRegionAttribute = 0UL;
}

Expand Down
14 changes: 7 additions & 7 deletions portable/GCC/ARM_CM3_MPU/portmacro.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@
#define portMPU_REGION_CACHEABLE_BUFFERABLE ( 0x07UL << 16UL )
#define portMPU_REGION_EXECUTE_NEVER ( 0x01UL << 28UL )

#define portUNPRIVILEGED_FLASH_REGION ( 0UL )
#define portPRIVILEGED_FLASH_REGION ( 1UL )
#define portPRIVILEGED_RAM_REGION ( 2UL )
#define portGENERAL_PERIPHERALS_REGION ( 3UL )
#define portSTACK_REGION ( 4UL )
#define portFIRST_CONFIGURABLE_REGION ( 5UL )
#define portLAST_CONFIGURABLE_REGION ( 7UL )
#define portUNPRIVILEGED_FLASH_REGION ( 5UL )
#define portPRIVILEGED_FLASH_REGION ( 6UL )
#define portPRIVILEGED_RAM_REGION ( 7UL )
#define portFIRST_CONFIGURABLE_REGION ( 0UL )
#define portLAST_CONFIGURABLE_REGION ( 2UL )
#define portNUM_CONFIGURABLE_REGIONS ( ( portLAST_CONFIGURABLE_REGION - portFIRST_CONFIGURABLE_REGION ) + 1 )
#define portTOTAL_NUM_REGIONS ( portNUM_CONFIGURABLE_REGIONS + 1 ) /* Plus one to make space for the stack region. */
#define portTOTAL_NUM_REGIONS_IN_TCB ( portNUM_CONFIGURABLE_REGIONS + 1 ) /* Plus one to make space for the stack region. */

#define portSWITCH_TO_USER_MODE() __asm volatile ( " mrs r0, control \n orr r0, #1 \n msr control, r0 " ::: "r0", "memory" )

Expand All @@ -103,7 +103,7 @@
/* Plus 1 to create space for the stack region. */
typedef struct MPU_SETTINGS
{
xMPU_REGION_REGISTERS xRegion[ portTOTAL_NUM_REGIONS ];
xMPU_REGION_REGISTERS xRegion[ portTOTAL_NUM_REGIONS_IN_TCB ];
} xMPU_SETTINGS;

/* Architecture specifics. */
Expand Down
39 changes: 13 additions & 26 deletions portable/GCC/ARM_CM4_MPU/port.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
#define portMPU_REGION_BASE_ADDRESS_REG ( *( ( volatile uint32_t * ) 0xe000ed9C ) )
#define portMPU_REGION_ATTRIBUTE_REG ( *( ( volatile uint32_t * ) 0xe000edA0 ) )
#define portMPU_CTRL_REG ( *( ( volatile uint32_t * ) 0xe000ed94 ) )
#define portEXPECTED_MPU_TYPE_VALUE ( portTOTAL_NUM_REGIONS << 8UL )
#define portEXPECTED_MPU_TYPE_VALUE ( configTOTAL_MPU_REGIONS << 8UL )
#define portMPU_ENABLE ( 0x01UL )
#define portMPU_BACKGROUND_ENABLE ( 1UL << 2UL )
#define portPRIVILEGED_EXECUTION_START_ADDRESS ( 0UL )
Expand Down Expand Up @@ -380,12 +380,12 @@ static void prvRestoreContextOfFirstTask( void )
" ldmia r1!, {r4-r11} \n"/* Read 4 sets of MPU registers [MPU Region # 4 - 7]. */
" stmia r2, {r4-r11} \n"/* Write 4 sets of MPU registers [MPU Region # 4 - 7]. */
" \n"
#if ( portTOTAL_NUM_REGIONS == 16 )
#if ( configTOTAL_MPU_REGIONS == 16 )
" ldmia r1!, {r4-r11} \n"/* Read 4 sets of MPU registers [MPU Region # 8 - 11]. */
" stmia r2, {r4-r11} \n"/* Write 4 sets of MPU registers. [MPU Region # 8 - 11]. */
" ldmia r1!, {r4-r11} \n"/* Read 4 sets of MPU registers [MPU Region # 12 - 15]. */
" stmia r2, {r4-r11} \n"/* Write 4 sets of MPU registers. [MPU Region # 12 - 15]. */
#endif /* portTOTAL_NUM_REGIONS == 16. */
#endif /* configTOTAL_MPU_REGIONS == 16. */
" \n"
" ldr r2, =0xe000ed94 \n"/* MPU_CTRL register. */
" ldr r3, [r2] \n"/* Read the value of MPU_CTRL. */
Expand Down Expand Up @@ -633,12 +633,12 @@ void xPortPendSVHandler( void )
" ldmia r1!, {r4-r11} \n"/* Read 4 sets of MPU registers [MPU Region # 4 - 7]. */
" stmia r2, {r4-r11} \n"/* Write 4 sets of MPU registers [MPU Region # 4 - 7]. */
" \n"
#if ( portTOTAL_NUM_REGIONS == 16 )
#if ( configTOTAL_MPU_REGIONS == 16 )
" ldmia r1!, {r4-r11} \n"/* Read 4 sets of MPU registers [MPU Region # 8 - 11]. */
" stmia r2, {r4-r11} \n"/* Write 4 sets of MPU registers. [MPU Region # 8 - 11]. */
" ldmia r1!, {r4-r11} \n"/* Read 4 sets of MPU registers [MPU Region # 12 - 15]. */
" stmia r2, {r4-r11} \n"/* Write 4 sets of MPU registers. [MPU Region # 12 - 15]. */
#endif /* portTOTAL_NUM_REGIONS == 16. */
#endif /* configTOTAL_MPU_REGIONS == 16. */
" \n"
" ldr r2, =0xe000ed94 \n"/* MPU_CTRL register. */
" ldr r3, [r2] \n"/* Read the value of MPU_CTRL. */
Expand Down Expand Up @@ -736,7 +736,7 @@ static void prvSetupMPU( void )
#endif /* if defined( __ARMCC_VERSION ) */

/* The only permitted number of regions are 8 or 16. */
configASSERT( ( portTOTAL_NUM_REGIONS == 8 ) || ( portTOTAL_NUM_REGIONS == 16 ) );
configASSERT( ( configTOTAL_MPU_REGIONS == 8 ) || ( configTOTAL_MPU_REGIONS == 16 ) );

/* Ensure that the configTOTAL_MPU_REGIONS is configured correctly. */
configASSERT( portMPU_TYPE_REG == portEXPECTED_MPU_TYPE_VALUE );
Expand Down Expand Up @@ -879,31 +879,18 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,
xMPUSettings->xRegion[ 0 ].ulRegionBaseAddress =
( ( uint32_t ) __SRAM_segment_start__ ) | /* Base address. */
( portMPU_REGION_VALID ) |
( portSTACK_REGION );
( portSTACK_REGION ); /* Region number. */

xMPUSettings->xRegion[ 0 ].ulRegionAttribute =
( portMPU_REGION_READ_WRITE ) |
( ( configTEX_S_C_B_SRAM & portMPU_RASR_TEX_S_C_B_MASK ) << portMPU_RASR_TEX_S_C_B_LOCATION ) |
( prvGetMPURegionSizeSetting( ( uint32_t ) __SRAM_segment_end__ - ( uint32_t ) __SRAM_segment_start__ ) ) |
( portMPU_REGION_ENABLE );

/* Re-instate the privileged only RAM region as xRegion[ 0 ] will have
* just removed the privileged only parameters. */
xMPUSettings->xRegion[ 1 ].ulRegionBaseAddress =
( ( uint32_t ) __privileged_data_start__ ) | /* Base address. */
( portMPU_REGION_VALID ) |
( portSTACK_REGION + 1 );

xMPUSettings->xRegion[ 1 ].ulRegionAttribute =
( portMPU_REGION_PRIVILEGED_READ_WRITE ) |
( ( configTEX_S_C_B_SRAM & portMPU_RASR_TEX_S_C_B_MASK ) << portMPU_RASR_TEX_S_C_B_LOCATION ) |
prvGetMPURegionSizeSetting( ( uint32_t ) __privileged_data_end__ - ( uint32_t ) __privileged_data_start__ ) |
( portMPU_REGION_ENABLE );

/* Invalidate all other regions. */
for( ul = 2; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
/* Invalidate user configurable regions. */
for( ul = 1UL; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
{
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( portSTACK_REGION + ul ) | portMPU_REGION_VALID;
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( ( ul - 1UL ) | portMPU_REGION_VALID );
xMPUSettings->xRegion[ ul ].ulRegionAttribute = 0UL;
}
}
Expand All @@ -930,7 +917,7 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,

lIndex = 0;

for( ul = 1; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
for( ul = 1UL; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
{
if( ( xRegions[ lIndex ] ).ulLengthInBytes > 0UL )
{
Expand All @@ -940,7 +927,7 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress =
( ( uint32_t ) xRegions[ lIndex ].pvBaseAddress ) |
( portMPU_REGION_VALID ) |
( portSTACK_REGION + ul ); /* Region number. */
( ul - 1UL ); /* Region number. */

xMPUSettings->xRegion[ ul ].ulRegionAttribute =
( prvGetMPURegionSizeSetting( xRegions[ lIndex ].ulLengthInBytes ) ) |
Expand All @@ -950,7 +937,7 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,
else
{
/* Invalidate the region. */
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( portSTACK_REGION + ul ) | portMPU_REGION_VALID;
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( ( ul - 1UL ) | portMPU_REGION_VALID );
xMPUSettings->xRegion[ ul ].ulRegionAttribute = 0UL;
}

Expand Down
21 changes: 10 additions & 11 deletions portable/GCC/ARM_CM4_MPU/portmacro.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,15 +173,15 @@ typedef unsigned long UBaseType_t;
#define configTEX_S_C_B_SRAM ( 0x07UL )
#endif

#define portUNPRIVILEGED_FLASH_REGION ( 0UL )
#define portPRIVILEGED_FLASH_REGION ( 1UL )
#define portPRIVILEGED_RAM_REGION ( 2UL )
#define portGENERAL_PERIPHERALS_REGION ( 3UL )
#define portSTACK_REGION ( 4UL )
#define portFIRST_CONFIGURABLE_REGION ( 5UL )
#define portTOTAL_NUM_REGIONS ( configTOTAL_MPU_REGIONS )
#define portNUM_CONFIGURABLE_REGIONS ( portTOTAL_NUM_REGIONS - portFIRST_CONFIGURABLE_REGION )
#define portLAST_CONFIGURABLE_REGION ( portTOTAL_NUM_REGIONS - 1 )
#define portGENERAL_PERIPHERALS_REGION ( configTOTAL_MPU_REGIONS - 5UL )
#define portSTACK_REGION ( configTOTAL_MPU_REGIONS - 4UL )
#define portUNPRIVILEGED_FLASH_REGION ( configTOTAL_MPU_REGIONS - 3UL )
#define portPRIVILEGED_FLASH_REGION ( configTOTAL_MPU_REGIONS - 2UL )
#define portPRIVILEGED_RAM_REGION ( configTOTAL_MPU_REGIONS - 1UL )
#define portFIRST_CONFIGURABLE_REGION ( 0UL )
#define portLAST_CONFIGURABLE_REGION ( configTOTAL_MPU_REGIONS - 6UL )
#define portNUM_CONFIGURABLE_REGIONS ( configTOTAL_MPU_REGIONS - 5UL )
#define portTOTAL_NUM_REGIONS_IN_TCB ( portNUM_CONFIGURABLE_REGIONS + 1 ) /* Plus 1 to create space for the stack region. */

#define portSWITCH_TO_USER_MODE() __asm volatile ( " mrs r0, control \n orr r0, #1 \n msr control, r0 " ::: "r0", "memory" )

Expand All @@ -191,10 +191,9 @@ typedef struct MPU_REGION_REGISTERS
uint32_t ulRegionAttribute;
} xMPU_REGION_REGISTERS;

/* Plus 1 to create space for the stack region. */
typedef struct MPU_SETTINGS
{
xMPU_REGION_REGISTERS xRegion[ portTOTAL_NUM_REGIONS ];
xMPU_REGION_REGISTERS xRegion[ portTOTAL_NUM_REGIONS_IN_TCB ];
} xMPU_SETTINGS;

/* Architecture specifics. */
Expand Down
31 changes: 9 additions & 22 deletions portable/IAR/ARM_CM4F_MPU/port.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
#define portMPU_REGION_BASE_ADDRESS_REG ( *( ( volatile uint32_t * ) 0xe000ed9C ) )
#define portMPU_REGION_ATTRIBUTE_REG ( *( ( volatile uint32_t * ) 0xe000edA0 ) )
#define portMPU_CTRL_REG ( *( ( volatile uint32_t * ) 0xe000ed94 ) )
#define portEXPECTED_MPU_TYPE_VALUE ( portTOTAL_NUM_REGIONS << 8UL )
#define portEXPECTED_MPU_TYPE_VALUE ( configTOTAL_MPU_REGIONS << 8UL )
#define portMPU_ENABLE ( 0x01UL )
#define portMPU_BACKGROUND_ENABLE ( 1UL << 2UL )
#define portPRIVILEGED_EXECUTION_START_ADDRESS ( 0UL )
Expand Down Expand Up @@ -555,7 +555,7 @@ static void prvSetupMPU( void )
extern uint32_t __privileged_data_end__[];

/* The only permitted number of regions are 8 or 16. */
configASSERT( ( portTOTAL_NUM_REGIONS == 8 ) || ( portTOTAL_NUM_REGIONS == 16 ) );
configASSERT( ( configTOTAL_MPU_REGIONS == 8 ) || ( configTOTAL_MPU_REGIONS == 16 ) );

/* Ensure that the configTOTAL_MPU_REGIONS is configured correctly. */
configASSERT( portMPU_TYPE_REG == portEXPECTED_MPU_TYPE_VALUE );
Expand Down Expand Up @@ -656,31 +656,18 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,
xMPUSettings->xRegion[ 0 ].ulRegionBaseAddress =
( ( uint32_t ) __SRAM_segment_start__ ) | /* Base address. */
( portMPU_REGION_VALID ) |
( portSTACK_REGION );
( portSTACK_REGION ); /* Region number. */

xMPUSettings->xRegion[ 0 ].ulRegionAttribute =
( portMPU_REGION_READ_WRITE ) |
( ( configTEX_S_C_B_SRAM & portMPU_RASR_TEX_S_C_B_MASK ) << portMPU_RASR_TEX_S_C_B_LOCATION ) |
( prvGetMPURegionSizeSetting( ( uint32_t ) __SRAM_segment_end__ - ( uint32_t ) __SRAM_segment_start__ ) ) |
( portMPU_REGION_ENABLE );

/* Re-instate the privileged only RAM region as xRegion[ 0 ] will have
* just removed the privileged only parameters. */
xMPUSettings->xRegion[ 1 ].ulRegionBaseAddress =
( ( uint32_t ) __privileged_data_start__ ) | /* Base address. */
( portMPU_REGION_VALID ) |
( portSTACK_REGION + 1 );

xMPUSettings->xRegion[ 1 ].ulRegionAttribute =
( portMPU_REGION_PRIVILEGED_READ_WRITE ) |
( ( configTEX_S_C_B_SRAM & portMPU_RASR_TEX_S_C_B_MASK ) << portMPU_RASR_TEX_S_C_B_LOCATION ) |
prvGetMPURegionSizeSetting( ( uint32_t ) __privileged_data_end__ - ( uint32_t ) __privileged_data_start__ ) |
( portMPU_REGION_ENABLE );

/* Invalidate all other regions. */
for( ul = 2; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
/* Invalidate user configurable regions. */
for( ul = 1UL; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
{
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( portSTACK_REGION + ul ) | portMPU_REGION_VALID;
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( ( ul - 1UL ) | portMPU_REGION_VALID );
xMPUSettings->xRegion[ ul ].ulRegionAttribute = 0UL;
}
}
Expand All @@ -707,7 +694,7 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,

lIndex = 0;

for( ul = 1; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
for( ul = 1UL; ul <= portNUM_CONFIGURABLE_REGIONS; ul++ )
{
if( ( xRegions[ lIndex ] ).ulLengthInBytes > 0UL )
{
Expand All @@ -717,7 +704,7 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress =
( ( uint32_t ) xRegions[ lIndex ].pvBaseAddress ) |
( portMPU_REGION_VALID ) |
( portSTACK_REGION + ul ); /* Region number. */
( ul - 1UL ); /* Region number. */

xMPUSettings->xRegion[ ul ].ulRegionAttribute =
( prvGetMPURegionSizeSetting( xRegions[ lIndex ].ulLengthInBytes ) ) |
Expand All @@ -727,7 +714,7 @@ void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings,
else
{
/* Invalidate the region. */
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( portSTACK_REGION + ul ) | portMPU_REGION_VALID;
xMPUSettings->xRegion[ ul ].ulRegionBaseAddress = ( ( ul - 1UL ) | portMPU_REGION_VALID );
xMPUSettings->xRegion[ ul ].ulRegionAttribute = 0UL;
}

Expand Down
Loading

0 comments on commit ea9c26f

Please sign in to comment.