From fa413a3064290cf3d0ba5cd016967f8a060d237f Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Mon, 15 Jul 2024 15:52:37 +0200 Subject: [PATCH] fix(freertos-smp): Added check for empty pxEventList in xTaskRemoveFromEventList() In SMP, a task running on the other core or an ISR can remove a task from the event list. There could be situation wherein a task contesting for the kernel lock may try to remove a task from the event list which has been emptied out by another task or ISR. --- tasks.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/tasks.c b/tasks.c index 47eb7570fbd..f693196baba 100644 --- a/tasks.c +++ b/tasks.c @@ -5482,7 +5482,12 @@ BaseType_t xTaskRemoveFromEventList( const List_t * const pxEventList ) traceENTER_xTaskRemoveFromEventList( pxEventList ); - #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) + #if ( ! ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) ) + + /* THIS FUNCTION MUST BE CALLED FROM A CRITICAL SECTION. It can also be + * called from a critical section within an ISR. */ + + #else /* #if ( ! ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) ) */ /* Lock the kernel data group as we are about to access its members */ UBaseType_t uxSavedInterruptStatus; @@ -5495,11 +5500,13 @@ BaseType_t xTaskRemoveFromEventList( const List_t * const pxEventList ) uxSavedInterruptStatus = 0; taskLOCK_DATA_GROUP( &xTaskSpinlock, &xISRSpinlock ); } - #else /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */ - /* THIS FUNCTION MUST BE CALLED FROM A CRITICAL SECTION. It can also be - * called from a critical section within an ISR. */ - #endif /* #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) */ + /* Before taking the kernel lock, another task/ISR could have already + * emptied the pxEventList. So we insert a check here to see if + * pxEventList is empty before attempting to remove an item from it. */ + if( listLIST_IS_EMPTY( pxEventList ) == pdFALSE ) + { + #endif /* #if ( ! ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) ) */ /* The event list is sorted in priority order, so the first in the list can * be removed as it is known to be the highest priority. Remove the TCB from @@ -5580,6 +5587,14 @@ BaseType_t xTaskRemoveFromEventList( const List_t * const pxEventList ) #endif /* #if ( configNUMBER_OF_CORES == 1 ) */ #if ( ( portUSING_GRANULAR_LOCKS == 1 ) && ( configNUMBER_OF_CORES > 1 ) ) + } + else + { + /* The pxEventList was emptied before we entered the critical + * section, Nothing to do except return pdFALSE. */ + xReturn = pdFALSE; + } + /* We are done accessing the kernel data group. Unlock it. */ if( portCHECK_IF_IN_ISR() == pdTRUE ) {