Fix portSWITCH_TO_USER_MODE() on Armv7-M MPU ports (FreeRTOS#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.
n9wxu · Sep 26, 2023 · 84bdb05 · 84bdb05
1 parent ac5deb1
commit 84bdb05
Show file tree

Hide file tree

Showing 8 changed files with 131 additions and 45 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 );