From 84bdb05bd284b9ef4c465331c9921200c5ee7bd6 Mon Sep 17 00:00:00 2001 From: Soren Ptak Date: Tue, 26 Sep 2023 02:06:23 -0700 Subject: [PATCH] Fix portSWITCH_TO_USER_MODE() on Armv7-M MPU ports (#803) A task's privilege level is stored in ulTaskFlag member in the TCB. Current implementation of portSWITCH_TO_USER_MODE() does not update this flag but just lowers the processor's privilege level. This results in many APIs incorrectly determining task's privilege level and access permissions - - xPortIsAuthorizedToAccessBuffer - xPortIsTaskPrivileged - xPortIsAuthorizedToAccessKernelObject This PR fixes the portSWITCH_TO_USER_MODE() implementation to correctly update the ulTaskFlag member in the TCB before lowering the processor's privilege level. --- portable/GCC/ARM_CM3_MPU/port.c | 20 +++++++++++- portable/GCC/ARM_CM3_MPU/portmacro.h | 17 +++++++--- portable/GCC/ARM_CM4_MPU/port.c | 20 +++++++++++- portable/GCC/ARM_CM4_MPU/portmacro.h | 17 +++++++--- portable/IAR/ARM_CM4F_MPU/port.c | 20 +++++++++++- portable/IAR/ARM_CM4F_MPU/portmacro.h | 17 +++++++--- portable/RVDS/ARM_CM4_MPU/port.c | 47 +++++++++++++++------------ portable/RVDS/ARM_CM4_MPU/portmacro.h | 18 ++++++---- 8 files changed, 131 insertions(+), 45 deletions(-) diff --git a/portable/GCC/ARM_CM3_MPU/port.c b/portable/GCC/ARM_CM3_MPU/port.c index 3c46dfde84c..ea2b9355e28 100644 --- a/portable/GCC/ARM_CM3_MPU/port.c +++ b/portable/GCC/ARM_CM3_MPU/port.c @@ -185,6 +185,11 @@ BaseType_t xIsPrivileged( void ) __attribute__( ( naked ) ); */ void vResetPrivilege( void ) __attribute__( ( naked ) ); +/** + * @brief Make a task unprivileged. + */ +void vPortSwitchToUserMode( void ); + /** * @brief Enter critical section. */ @@ -284,7 +289,7 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, } else { - xMPUSettings->ulTaskFlags &= ( ~portTASK_IS_PRIVILEGED_FLAG ); + xMPUSettings->ulTaskFlags &= ( ~( portTASK_IS_PRIVILEGED_FLAG ) ); xMPUSettings->ulContext[ 0 ] = portINITIAL_CONTROL_IF_UNPRIVILEGED; } @@ -1209,6 +1214,19 @@ void vResetPrivilege( void ) /* __attribute__ (( naked )) */ } /*-----------------------------------------------------------*/ +void vPortSwitchToUserMode( void ) +{ + /* Load the current task's MPU settings from its TCB. */ + xMPU_SETTINGS * xTaskMpuSettings = xTaskGetMPUSettings( NULL ); + + /* Mark the task as unprivileged. */ + xTaskMpuSettings->ulTaskFlags &= ( ~( portTASK_IS_PRIVILEGED_FLAG ) ); + + /* Lower the processor's privilege level. */ + vResetPrivilege(); +} +/*-----------------------------------------------------------*/ + void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings, const struct xMEMORY_REGION * const xRegions, StackType_t * pxBottomOfStack, diff --git a/portable/GCC/ARM_CM3_MPU/portmacro.h b/portable/GCC/ARM_CM3_MPU/portmacro.h index a65bb2162ca..d0822e1443f 100644 --- a/portable/GCC/ARM_CM3_MPU/portmacro.h +++ b/portable/GCC/ARM_CM3_MPU/portmacro.h @@ -96,8 +96,6 @@ typedef unsigned long UBaseType_t; #define portNUM_CONFIGURABLE_REGIONS ( ( portLAST_CONFIGURABLE_REGION - portFIRST_CONFIGURABLE_REGION ) + 1 ) #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" ) - typedef struct MPU_REGION_REGISTERS { uint32_t ulRegionBaseAddress; @@ -268,24 +266,33 @@ extern void vPortExitCritical( void ); extern BaseType_t xIsPrivileged( void ); extern void vResetPrivilege( void ); +extern void vPortSwitchToUserMode( void ); /** * @brief Checks whether or not the processor is privileged. * * @return 1 if the processor is already privileged, 0 otherwise. */ -#define portIS_PRIVILEGED() xIsPrivileged() +#define portIS_PRIVILEGED() xIsPrivileged() /** * @brief Raise an SVC request to raise privilege. */ -#define portRAISE_PRIVILEGE() __asm volatile ( "svc %0 \n" ::"i" ( portSVC_RAISE_PRIVILEGE ) : "memory" ); +#define portRAISE_PRIVILEGE() __asm volatile ( "svc %0 \n" ::"i" ( portSVC_RAISE_PRIVILEGE ) : "memory" ); /** * @brief Lowers the privilege level by setting the bit 0 of the CONTROL * register. */ -#define portRESET_PRIVILEGE() vResetPrivilege() +#define portRESET_PRIVILEGE() vResetPrivilege() + +/** + * @brief Make a task unprivileged. + * + * It must be called from privileged tasks only. Calling it from unprivileged + * task will result in a memory protection fault. + */ +#define portSWITCH_TO_USER_MODE() vPortSwitchToUserMode() /*-----------------------------------------------------------*/ extern BaseType_t xPortIsTaskPrivileged( void ); diff --git a/portable/GCC/ARM_CM4_MPU/port.c b/portable/GCC/ARM_CM4_MPU/port.c index 861c400cdf7..e2ef1f054b7 100644 --- a/portable/GCC/ARM_CM4_MPU/port.c +++ b/portable/GCC/ARM_CM4_MPU/port.c @@ -206,6 +206,11 @@ BaseType_t xIsPrivileged( void ) __attribute__( ( naked ) ); */ void vResetPrivilege( void ) __attribute__( ( naked ) ); +/** + * @brief Make a task unprivileged. + */ +void vPortSwitchToUserMode( void ); + /** * @brief Enter critical section. */ @@ -312,7 +317,7 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, } else { - xMPUSettings->ulTaskFlags &= ( ~portTASK_IS_PRIVILEGED_FLAG ); + xMPUSettings->ulTaskFlags &= ( ~( portTASK_IS_PRIVILEGED_FLAG ) ); xMPUSettings->ulContext[ 0 ] = portINITIAL_CONTROL_IF_UNPRIVILEGED; } @@ -1390,6 +1395,19 @@ void vResetPrivilege( void ) /* __attribute__ (( naked )) */ } /*-----------------------------------------------------------*/ +void vPortSwitchToUserMode( void ) +{ + /* Load the current task's MPU settings from its TCB. */ + xMPU_SETTINGS * xTaskMpuSettings = xTaskGetMPUSettings( NULL ); + + /* Mark the task as unprivileged. */ + xTaskMpuSettings->ulTaskFlags &= ( ~( portTASK_IS_PRIVILEGED_FLAG ) ); + + /* Lower the processor's privilege level. */ + vResetPrivilege(); +} +/*-----------------------------------------------------------*/ + void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings, const struct xMEMORY_REGION * const xRegions, StackType_t * pxBottomOfStack, diff --git a/portable/GCC/ARM_CM4_MPU/portmacro.h b/portable/GCC/ARM_CM4_MPU/portmacro.h index 6f8f0c4e9e7..1f62279b040 100644 --- a/portable/GCC/ARM_CM4_MPU/portmacro.h +++ b/portable/GCC/ARM_CM4_MPU/portmacro.h @@ -190,8 +190,6 @@ typedef unsigned long UBaseType_t; #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" ) - typedef struct MPU_REGION_REGISTERS { uint32_t ulRegionBaseAddress; @@ -362,24 +360,33 @@ extern void vPortExitCritical( void ); extern BaseType_t xIsPrivileged( void ); extern void vResetPrivilege( void ); +extern void vPortSwitchToUserMode( void ); /** * @brief Checks whether or not the processor is privileged. * * @return 1 if the processor is already privileged, 0 otherwise. */ -#define portIS_PRIVILEGED() xIsPrivileged() +#define portIS_PRIVILEGED() xIsPrivileged() /** * @brief Raise an SVC request to raise privilege. */ -#define portRAISE_PRIVILEGE() __asm volatile ( "svc %0 \n" ::"i" ( portSVC_RAISE_PRIVILEGE ) : "memory" ); +#define portRAISE_PRIVILEGE() __asm volatile ( "svc %0 \n" ::"i" ( portSVC_RAISE_PRIVILEGE ) : "memory" ); /** * @brief Lowers the privilege level by setting the bit 0 of the CONTROL * register. */ -#define portRESET_PRIVILEGE() vResetPrivilege() +#define portRESET_PRIVILEGE() vResetPrivilege() + +/** + * @brief Make a task unprivileged. + * + * It must be called from privileged tasks only. Calling it from unprivileged + * task will result in a memory protection fault. + */ +#define portSWITCH_TO_USER_MODE() vPortSwitchToUserMode() /*-----------------------------------------------------------*/ extern BaseType_t xPortIsTaskPrivileged( void ); diff --git a/portable/IAR/ARM_CM4F_MPU/port.c b/portable/IAR/ARM_CM4F_MPU/port.c index f7b9f45ce20..6db7bd796a9 100644 --- a/portable/IAR/ARM_CM4F_MPU/port.c +++ b/portable/IAR/ARM_CM4F_MPU/port.c @@ -283,6 +283,11 @@ extern void vPortRestoreContextOfFirstTask( void ) PRIVILEGED_FUNCTION; */ BaseType_t xPortIsTaskPrivileged( void ) PRIVILEGED_FUNCTION; +/** + * @brief Make a task unprivileged. + */ +void vPortSwitchToUserMode( void ); + /*-----------------------------------------------------------*/ /* Each task maintains its own interrupt status in the critical nesting @@ -318,7 +323,7 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, } else { - xMPUSettings->ulTaskFlags &= ( ~portTASK_IS_PRIVILEGED_FLAG ); + xMPUSettings->ulTaskFlags &= ( ~( portTASK_IS_PRIVILEGED_FLAG ) ); xMPUSettings->ulContext[ 0 ] = portINITIAL_CONTROL_IF_UNPRIVILEGED; } @@ -741,6 +746,19 @@ BaseType_t xPortIsTaskPrivileged( void ) /* PRIVILEGED_FUNCTION */ } /*-----------------------------------------------------------*/ +void vPortSwitchToUserMode( void ) +{ + /* Load the current task's MPU settings from its TCB. */ + xMPU_SETTINGS * xTaskMpuSettings = xTaskGetMPUSettings( NULL ); + + /* Mark the task as unprivileged. */ + xTaskMpuSettings->ulTaskFlags &= ( ~( portTASK_IS_PRIVILEGED_FLAG ) ); + + /* Lower the processor's privilege level. */ + vResetPrivilege(); +} +/*-----------------------------------------------------------*/ + /* * See header file for description. */ diff --git a/portable/IAR/ARM_CM4F_MPU/portmacro.h b/portable/IAR/ARM_CM4F_MPU/portmacro.h index 5a102d0ad0b..98b087e11fd 100644 --- a/portable/IAR/ARM_CM4F_MPU/portmacro.h +++ b/portable/IAR/ARM_CM4F_MPU/portmacro.h @@ -192,8 +192,6 @@ typedef unsigned long UBaseType_t; #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, r0, #1 \n msr control, r0 " ::: "r0", "memory" ) - typedef struct MPU_REGION_REGISTERS { uint32_t ulRegionBaseAddress; @@ -390,24 +388,33 @@ portFORCE_INLINE static BaseType_t xPortIsInsideInterrupt( void ) extern BaseType_t xIsPrivileged( void ); extern void vResetPrivilege( void ); +extern void vPortSwitchToUserMode( void ); /** * @brief Checks whether or not the processor is privileged. * * @return 1 if the processor is already privileged, 0 otherwise. */ -#define portIS_PRIVILEGED() xIsPrivileged() +#define portIS_PRIVILEGED() xIsPrivileged() /** * @brief Raise an SVC request to raise privilege. */ -#define portRAISE_PRIVILEGE() __asm volatile ( "svc %0 \n" ::"i" ( portSVC_RAISE_PRIVILEGE ) : "memory" ); +#define portRAISE_PRIVILEGE() __asm volatile ( "svc %0 \n" ::"i" ( portSVC_RAISE_PRIVILEGE ) : "memory" ); /** * @brief Lowers the privilege level by setting the bit 0 of the CONTROL * register. */ -#define portRESET_PRIVILEGE() vResetPrivilege() +#define portRESET_PRIVILEGE() vResetPrivilege() + +/** + * @brief Make a task unprivileged. + * + * It must be called from privileged tasks only. Calling it from unprivileged + * task will result in a memory protection fault. + */ +#define portSWITCH_TO_USER_MODE() vPortSwitchToUserMode() /*-----------------------------------------------------------*/ extern BaseType_t xPortIsTaskPrivileged( void ); diff --git a/portable/RVDS/ARM_CM4_MPU/port.c b/portable/RVDS/ARM_CM4_MPU/port.c index 68562f8eddc..e7e26b9694f 100644 --- a/portable/RVDS/ARM_CM4_MPU/port.c +++ b/portable/RVDS/ARM_CM4_MPU/port.c @@ -219,6 +219,11 @@ BaseType_t xIsPrivileged( void ); */ void vResetPrivilege( void ); +/** + * @brief Make a task unprivileged. + */ +void vPortSwitchToUserMode( void ); + /** * @brief Enter critical section. */ @@ -312,7 +317,7 @@ StackType_t * pxPortInitialiseStack( StackType_t * pxTopOfStack, } else { - xMPUSettings->ulTaskFlags &= ( ~portTASK_IS_PRIVILEGED_FLAG ); + xMPUSettings->ulTaskFlags &= ( ~( portTASK_IS_PRIVILEGED_FLAG ) ); xMPUSettings->ulContext[ 0 ] = portINITIAL_CONTROL_IF_UNPRIVILEGED; } @@ -1219,19 +1224,6 @@ __weak void vSetupTimerInterrupt( void ) } /*-----------------------------------------------------------*/ -__asm void vPortSwitchToUserMode( void ) -{ -/* *INDENT-OFF* */ - PRESERVE8 - - mrs r0, control - orr r0, #1 - msr control, r0 - bx r14 -/* *INDENT-ON* */ -} -/*-----------------------------------------------------------*/ - __asm void vPortEnableVFP( void ) { /* *INDENT-OFF* */ @@ -1349,10 +1341,10 @@ __asm BaseType_t xIsPrivileged( void ) PRESERVE8 mrs r0, control /* r0 = CONTROL. */ - tst r0, #1 /* Perform r0 & 1 (bitwise AND) and update the conditions flag. */ + tst r0, #1 /* Perform r0 & 1 (bitwise AND) and update the conditions flag. */ ite ne - movne r0, #0 /* CONTROL[0]!=0. Return false to indicate that the processor is not privileged. */ - moveq r0, #1 /* CONTROL[0]==0. Return true to indicate that the processor is privileged. */ + movne r0, #0 /* CONTROL[0]!=0. Return false to indicate that the processor is not privileged. */ + moveq r0, #1 /* CONTROL[0]==0. Return true to indicate that the processor is privileged. */ bx lr /* Return. */ /* *INDENT-ON* */ } @@ -1363,14 +1355,27 @@ __asm void vResetPrivilege( void ) /* *INDENT-OFF* */ PRESERVE8 - mrs r0, control /* r0 = CONTROL. */ - orrs r0, #1 /* r0 = r0 | 1. */ - msr control, r0 /* CONTROL = r0. */ - bx lr /* Return. */ + mrs r0, control /* r0 = CONTROL. */ + orrs r0, #1 /* r0 = r0 | 1. */ + msr control, r0 /* CONTROL = r0. */ + bx lr /* Return. */ /* *INDENT-ON* */ } /*-----------------------------------------------------------*/ +void vPortSwitchToUserMode( void ) +{ + /* Load the current task's MPU settings from its TCB. */ + xMPU_SETTINGS * xTaskMpuSettings = xTaskGetMPUSettings( NULL ); + + /* Mark the task as unprivileged. */ + xTaskMpuSettings->ulTaskFlags &= ( ~( portTASK_IS_PRIVILEGED_FLAG ) ); + + /* Lower the processor's privilege level. */ + vResetPrivilege(); +} +/*-----------------------------------------------------------*/ + void vPortStoreTaskMPUSettings( xMPU_SETTINGS * xMPUSettings, const struct xMEMORY_REGION * const xRegions, StackType_t * pxBottomOfStack, diff --git a/portable/RVDS/ARM_CM4_MPU/portmacro.h b/portable/RVDS/ARM_CM4_MPU/portmacro.h index 5e32e4a470b..1d17b4c105f 100644 --- a/portable/RVDS/ARM_CM4_MPU/portmacro.h +++ b/portable/RVDS/ARM_CM4_MPU/portmacro.h @@ -189,9 +189,6 @@ typedef unsigned long UBaseType_t; #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. */ -void vPortSwitchToUserMode( void ); -#define portSWITCH_TO_USER_MODE() vPortSwitchToUserMode() - typedef struct MPU_REGION_REGISTERS { uint32_t ulRegionBaseAddress; @@ -356,24 +353,33 @@ extern void vPortExitCritical( void ); extern BaseType_t xIsPrivileged( void ); extern void vResetPrivilege( void ); +extern void vPortSwitchToUserMode( void ); /** * @brief Checks whether or not the processor is privileged. * * @return 1 if the processor is already privileged, 0 otherwise. */ -#define portIS_PRIVILEGED() xIsPrivileged() +#define portIS_PRIVILEGED() xIsPrivileged() /** * @brief Raise an SVC request to raise privilege. */ -#define portRAISE_PRIVILEGE() __asm { svc portSVC_RAISE_PRIVILEGE } +#define portRAISE_PRIVILEGE() __asm { svc portSVC_RAISE_PRIVILEGE } /** * @brief Lowers the privilege level by setting the bit 0 of the CONTROL * register. */ -#define portRESET_PRIVILEGE() vResetPrivilege() +#define portRESET_PRIVILEGE() vResetPrivilege() + +/** + * @brief Make a task unprivileged. + * + * It must be called from privileged tasks only. Calling it from unprivileged + * task will result in a memory protection fault. + */ +#define portSWITCH_TO_USER_MODE() vPortSwitchToUserMode() /*-----------------------------------------------------------*/ extern BaseType_t xPortIsTaskPrivileged( void );