From 1f437df4935ab2167719770f8f01da9bccdbab17 Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Fri, 11 Oct 2024 11:49:50 +0200 Subject: [PATCH] fix(freertos): Fixed SMP race condition in stream_buffers.c This commit fixes a race condition in dual-core SMP mode where in the xStreamBufferReceive() makes the xTaskWaitingToReceive NULL but it may have already been evaluated to not be NULL by xStreamBufferSend() running on another core and eventually leading to a crash in tasks.c. --- .../freertos/FreeRTOS-Kernel/idf_changes.md | 2 ++ .../freertos/FreeRTOS-Kernel/stream_buffer.c | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/components/freertos/FreeRTOS-Kernel/idf_changes.md b/components/freertos/FreeRTOS-Kernel/idf_changes.md index 28385a40c5..1180ee2f26 100644 --- a/components/freertos/FreeRTOS-Kernel/idf_changes.md +++ b/components/freertos/FreeRTOS-Kernel/idf_changes.md @@ -123,6 +123,8 @@ The following functions were modified to accommodate SMP behavior: - In SMP, the function now disables interrupts to ensure that the calling task does not switch cores while checking its own copy of `uxSchedulerSuspended`. - `prvAddCurrentTaskToDelayedList()` - Added extra check to see if current blocking task has already been deleted by the other core. +- `xStreamBufferReceive()` + - Added a critical section for setting `xTaskWaitingToReceive` to `NULL` so that the write is SMP safe. ### Critical Section Changes diff --git a/components/freertos/FreeRTOS-Kernel/stream_buffer.c b/components/freertos/FreeRTOS-Kernel/stream_buffer.c index f474580ebc..9982b3caa3 100644 --- a/components/freertos/FreeRTOS-Kernel/stream_buffer.c +++ b/components/freertos/FreeRTOS-Kernel/stream_buffer.c @@ -6,7 +6,7 @@ * * SPDX-License-Identifier: MIT * - * SPDX-FileContributor: 2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileContributor: 2023-2024 Espressif Systems (Shanghai) CO LTD * * Permission is hereby granted, free of charge, to any person obtaining a copy of * this software and associated documentation files (the "Software"), to deal in @@ -976,7 +976,19 @@ size_t xStreamBufferReceive( StreamBufferHandle_t xStreamBuffer, /* Wait for data to be available. */ traceBLOCKING_ON_STREAM_BUFFER_RECEIVE( xStreamBuffer ); ( void ) xTaskNotifyWait( ( uint32_t ) 0, ( uint32_t ) 0, NULL, xTicksToWait ); - pxStreamBuffer->xTaskWaitingToReceive = NULL; + + /* In SMP mode, the task may have been woken and scheduled on + * another core. Hence, we must clear the xTaskWaitingToReceive + * handle in a critical section. */ + #if ( configNUMBER_OF_CORES > 1 ) + taskENTER_CRITICAL( &( pxStreamBuffer->xStreamBufferLock ) ); + #endif /* configNUMBER_OF_CORES > 1 */ + { + pxStreamBuffer->xTaskWaitingToReceive = NULL; + } + #if ( configNUMBER_OF_CORES > 1 ) + taskEXIT_CRITICAL( &( pxStreamBuffer->xStreamBufferLock ) ); + #endif /* configNUMBER_OF_CORES > 1 */ /* Recheck the data available after blocking. */ xBytesAvailable = prvBytesInBuffer( pxStreamBuffer );