Jelajahi Sumber

FreeRTOS: Fix xStreamBufferReset function always crashing

This function resets the spinlock given as a parameter after taking it
(when entering the critical section). This then results in a panic once
it tries to exit the same critical section.

* Closes https://github.com/espressif/esp-idf/issues/7725
Omar Chebib 4 tahun lalu
induk
melakukan
8376276b14
1 mengubah file dengan 67 tambahan dan 6 penghapusan
  1. 67 6
      components/freertos/FreeRTOS-Kernel/stream_buffer.c

+ 67 - 6
components/freertos/FreeRTOS-Kernel/stream_buffer.c

@@ -195,7 +195,8 @@ typedef struct StreamBufferDef_t                 /*lint !e9058 Style convention
         UBaseType_t uxStreamBufferNumber; /* Used for tracing purposes. */
     #endif
 #ifdef ESP_PLATFORM
-    portMUX_TYPE xStreamBufferMux;  //Mutex required due to SMP
+    /* Mutex required due to SMP. This field shall be the last one of the structure. */
+    portMUX_TYPE xStreamBufferMux;
 #endif // ESP_PLATFORM
 } StreamBuffer_t;
 
@@ -259,6 +260,17 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer,
                                           size_t xTriggerLevelBytes,
                                           uint8_t ucFlags ) PRIVILEGED_FUNCTION;
 
+#ifdef ESP_PLATFORM
+/**
+ * Called by xStreamBufferReset() to reset the members of the StreamBuffer, excluding
+ * its spinlock.
+ */
+static void prvResetStreamBufferFields( StreamBuffer_t * const pxStreamBuffer,
+                                        uint8_t * const pucBuffer,
+                                        size_t xBufferSizeBytes,
+                                        size_t xTriggerLevelBytes,
+                                        uint8_t ucFlags ) PRIVILEGED_FUNCTION;
+#endif
 /*-----------------------------------------------------------*/
 
 #if ( configSUPPORT_DYNAMIC_ALLOCATION == 1 )
@@ -472,11 +484,23 @@ BaseType_t xStreamBufferReset( StreamBufferHandle_t xStreamBuffer )
         {
             if( pxStreamBuffer->xTaskWaitingToSend == NULL )
             {
-                prvInitialiseNewStreamBuffer( pxStreamBuffer,
-                                              pxStreamBuffer->pucBuffer,
-                                              pxStreamBuffer->xLength,
-                                              pxStreamBuffer->xTriggerLevelBytes,
-                                              pxStreamBuffer->ucFlags );
+                #ifdef ESP_PLATFORM
+                    /* As we just entered a critical section, we must NOT reset the spinlock field.
+                     * Thus, call `prvResetStreamBufferFields` instead of `prvInitialiseNewStreamBuffer`
+                     */
+                    prvResetStreamBufferFields( pxStreamBuffer,
+                                                pxStreamBuffer->pucBuffer,
+                                                pxStreamBuffer->xLength,
+                                                pxStreamBuffer->xTriggerLevelBytes,
+                                                pxStreamBuffer->ucFlags );
+
+                #else  // ESP_PLATFORM
+                    prvInitialiseNewStreamBuffer( pxStreamBuffer,
+                                                  pxStreamBuffer->pucBuffer,
+                                                  pxStreamBuffer->xLength,
+                                                  pxStreamBuffer->xTriggerLevelBytes,
+                                                  pxStreamBuffer->ucFlags );
+                #endif // ESP_PLATFORM
                 xReturn = pdPASS;
 
                 #if ( configUSE_TRACE_FACILITY == 1 )
@@ -1331,6 +1355,43 @@ static void prvInitialiseNewStreamBuffer( StreamBuffer_t * const pxStreamBuffer,
 #endif // ESP_PLATFORM
 }
 
+
+#ifdef ESP_PLATFORM
+
+    /** The goal of this function is to (re)set all the fields of the given StreamBuffer, except
+     * its lock.
+     */
+    static void prvResetStreamBufferFields( StreamBuffer_t * const pxStreamBuffer,
+                                            uint8_t * const pucBuffer,
+                                            size_t xBufferSizeBytes,
+                                            size_t xTriggerLevelBytes,
+                                            uint8_t ucFlags )
+    {
+        #if ( configASSERT_DEFINED == 1 )
+            {
+                /* The value written just has to be identifiable when looking at the
+                * memory.  Don't use 0xA5 as that is the stack fill value and could
+                * result in confusion as to what is actually being observed. */
+                const BaseType_t xWriteValue = 0x55;
+                configASSERT( memset( pucBuffer, ( int ) xWriteValue, xBufferSizeBytes ) == pucBuffer );
+            } /*lint !e529 !e438 xWriteValue is only used if configASSERT() is defined. */
+        #endif
+
+        /* Do not include the spinlock in the part to reset!
+         * Thus, make sure the spinlock is the last field of the structure. */
+        _Static_assert( offsetof(StreamBuffer_t, xStreamBufferMux) == sizeof( StreamBuffer_t ) - sizeof(portMUX_TYPE),
+                        "xStreamBufferMux must be the last field of structure StreamBuffer_t" );
+        const size_t erasable = sizeof( StreamBuffer_t ) - sizeof(portMUX_TYPE);
+        ( void ) memset( ( void * ) pxStreamBuffer, 0x00, erasable ); /*lint !e9087 memset() requires void *. */
+        pxStreamBuffer->pucBuffer = pucBuffer;
+        pxStreamBuffer->xLength = xBufferSizeBytes;
+        pxStreamBuffer->xTriggerLevelBytes = xTriggerLevelBytes;
+        pxStreamBuffer->ucFlags = ucFlags;
+    }
+
+#endif // ESP_PLATFORM
+
+
 #if ( configUSE_TRACE_FACILITY == 1 )
 
     UBaseType_t uxStreamBufferGetStreamBufferNumber( StreamBufferHandle_t xStreamBuffer )