Fix CMSIS timer callback use-after-free on delete

CMSIS/RTOS2/FreeRTOS/Source/cmsis_os2.c

@@ -97,6 +97,13 @@ typedef struct {
   void         *arg;
 } TimerCallback_t;
 
+#if ((configUSE_OS2_TIMER == 1) && (configSUPPORT_DYNAMIC_ALLOCATION == 1)) && !defined(USE_FreeRTOS_HEAP_1)
+static void TimerCallback       (TimerHandle_t hTimer);
+static void TimerDeleteCallback (TimerHandle_t hTimer,
+                                 TimerCallbackFunction_t callback,
+                                 void *timer_id);
+#endif
+
 /* Kernel initialization state */
 static osKernelState_t KernelState = osKernelInactive;
 
@@ -222,6 +229,11 @@ osStatus_t osKernelInitialize (void) {
         /* Initialize the memory regions when using heap_5 variant */
         vPortDefineHeapRegions (configHEAP_5_REGIONS);
       #endif
+      #if ((configUSE_OS2_TIMER == 1) && (configSUPPORT_DYNAMIC_ALLOCATION == 1)) && !defined(USE_FreeRTOS_HEAP_1)
+        /* Release dynamically allocated CMSIS timer callback context only when */
+        /* the timer daemon has processed the asynchronous delete command.      */
+        vTimerDeleteCallbackRegister (TimerDeleteCallback);
+      #endif
       KernelState = osKernelReady;
       stat = osOK;
     } else {
@@ -1224,6 +1236,23 @@ static void TimerCallback (TimerHandle_t hTimer) {
   }
 }
 
+#if (configSUPPORT_DYNAMIC_ALLOCATION == 1) && !defined(USE_FreeRTOS_HEAP_1)
+static void TimerDeleteCallback (TimerHandle_t hTimer,
+                                 TimerCallbackFunction_t callback,
+                                 void *timer_id) {
+  TimerCallback_t *callb;
+
+  (void)hTimer;
+
+  /* Only CMSIS timers own a dynamically allocated TimerCallback_t. */
+  if ((callback == TimerCallback) && (((uint32_t)timer_id & 1U) != 0U)) {
+    /* Remove dynamic allocation flag and return memory to dynamic pool. */
+    callb = (TimerCallback_t *)((uint32_t)timer_id & ~1U);
+    vPortFree (callb);
+  }
+}
+#endif
+
 /*
   Create and Initialize a timer.
 */
@@ -1432,9 +1461,6 @@ osStatus_t osTimerDelete (osTimerId_t timer_id) {
 
 #ifndef USE_FreeRTOS_HEAP_1
   TimerHandle_t hTimer = (TimerHandle_t)timer_id;
-  #if (configSUPPORT_DYNAMIC_ALLOCATION == 1)
-  TimerCallback_t *callb;
-  #endif
 
   if (IRQ_Context() != 0U) {
     stat = osErrorISR;
@@ -1443,20 +1469,7 @@ osStatus_t osTimerDelete (osTimerId_t timer_id) {
     stat = osErrorParameter;
   }
   else {
-    #if (configSUPPORT_DYNAMIC_ALLOCATION == 1)
-    callb = (TimerCallback_t *)pvTimerGetTimerID (hTimer);
-    #endif
-
     if (xTimerDelete (hTimer, 0) == pdPASS) {
-      #if (configSUPPORT_DYNAMIC_ALLOCATION == 1)
-        if ((uint32_t)callb & 1U) {
-          /* Callback memory was allocated from dynamic pool, clear flag */
-          callb = (TimerCallback_t *)((uint32_t)callb & ~1U);
-
-          /* Return allocated memory to dynamic pool */
-          vPortFree (callb);
-        }
-      #endif
       stat = osOK;
     } else {
       stat = osErrorResource;

Source/include/timers.h

@@ -82,6 +82,16 @@ typedef struct tmrTimerControl * TimerHandle_t;
  */
 typedef void (* TimerCallbackFunction_t)( TimerHandle_t xTimer );
 
+/*
+ * Defines the prototype for a callback invoked by the timer daemon when it
+ * processes a timer delete command. The timer is still valid when the callback
+ * runs, so wrapper-owned resources referenced by the timer can be released
+ * before the timer control block itself is freed.
+ */
+typedef void (* TimerDeleteCallbackFunction_t)( TimerHandle_t xTimer,
+                                                TimerCallbackFunction_t pxCallbackFunction,
+                                                void * pvTimerID );
+
 /*
  * Defines the prototype to which functions used with the
  * xTimerPendFunctionCallFromISR() function must conform.
@@ -1288,6 +1298,18 @@ void vTimerSetReloadMode( TimerHandle_t xTimer,
  */
 BaseType_t xTimerGetReloadMode( TimerHandle_t xTimer ) PRIVILEGED_FUNCTION;
 
+/**
+ * void vTimerDeleteCallbackRegister( TimerDeleteCallbackFunction_t pxDeleteCallback );
+ *
+ * Registers a callback that the timer daemon invokes immediately before it
+ * deletes a timer. This allows an API wrapper to release resources whose
+ * lifetime must extend until an asynchronous xTimerDelete() command is
+ * processed.
+ *
+ * @param pxDeleteCallback The callback to invoke, or NULL to unregister it.
+ */
+void vTimerDeleteCallbackRegister( TimerDeleteCallbackFunction_t pxDeleteCallback ) PRIVILEGED_FUNCTION;
+
 /**
  * UBaseType_t uxTimerGetReloadMode( TimerHandle_t xTimer );
  *

Source/timers.c

@@ -148,6 +148,7 @@
 /* A queue that is used to send commands to the timer service task. */
     PRIVILEGED_DATA static QueueHandle_t xTimerQueue = NULL;
     PRIVILEGED_DATA static TaskHandle_t xTimerTaskHandle = NULL;
+    PRIVILEGED_DATA static TimerDeleteCallbackFunction_t pxTimerDeleteCallback = NULL;
 
 /*-----------------------------------------------------------*/
 
@@ -1047,6 +1048,15 @@
                             break;
 
                         case tmrCOMMAND_DELETE:
+                            /* Notify wrappers while the timer control block and
+                             * its callback context are still valid. */
+                            if( pxTimerDeleteCallback != NULL )
+                            {
+                                pxTimerDeleteCallback( ( TimerHandle_t ) pxTimer,
+                                                       pxTimer->pxCallbackFunction,
+                                                       pxTimer->pvTimerID );
+                            }
+
                             #if ( configSUPPORT_DYNAMIC_ALLOCATION == 1 )
                             {
                                 /* The timer has already been removed from the active list,
@@ -1191,6 +1201,12 @@
     }
 /*-----------------------------------------------------------*/
 
+    void vTimerDeleteCallbackRegister( TimerDeleteCallbackFunction_t pxDeleteCallback )
+    {
+        pxTimerDeleteCallback = pxDeleteCallback;
+    }
+/*-----------------------------------------------------------*/
+
     void * pvTimerGetTimerID( const TimerHandle_t xTimer )
     {
         Timer_t * const pxTimer = xTimer;