From 5b83e820d25893d479b8dceb0bd66571437fc688 Mon Sep 17 00:00:00 2001 From: Chen Wu Date: Mon, 6 Nov 2023 11:31:09 +0800 Subject: [PATCH] fix(ESPCS-924): Fixed a potential freertos crash - Reason: A task and B interrupt indirectly access the shared resource pxDelayedTaskList without proper security protection, leading to further crash. A task uses xEventGroupSetBits() to access the pxDelayedTaskList resource: xEventGroupSetBits() -> vTaskRemoveFromUnorderedEventList() -> uxListRemove() -> pxList, where pxList is the pxDelayedTaskList. At this point, another B interrupt is triggered (xEventGroupSetBits only suspends task scheduling and does not disable interrupts) and also accesses the pxDelayedTaskList resource: MacIsrSigPostDefHdl() -> __wifi_queue_send_from_isr() -> xQueueGenericSendFromISR() -> xTaskRemoveFromEventList() -> prvResetNextTaskUnblockTime() -> pxDelayedTaskList. This leads to an unsafe access to the pxDelayedTaskList resource by two entities, causing subsequent crash exceptions. - Fix: Modify the timing of the call to prvResetNextTaskUnblockTime() within xTaskRemoveFromEventList from unconditional execution to only execute when task scheduling is enabled. This way, when the B interrupt reaches xTaskRemoveFromEventList, it will not call prvResetNextTaskUnblockTime to access the pxDelayedTaskList resource (due to task scheduling being disabled). After the B interrupt execution is complete and control returns to A task, xTaskResumeAll() will be called, and then prvResetNextTaskUnblockTime() will update the pxDelayedTaskList resource again. --- components/freertos/freertos/tasks.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/components/freertos/freertos/tasks.c b/components/freertos/freertos/tasks.c index c747cd74..61e10e50 100644 --- a/components/freertos/freertos/tasks.c +++ b/components/freertos/freertos/tasks.c @@ -3044,6 +3044,19 @@ BaseType_t xReturn; { ( void ) uxListRemove( &( pxUnblockedTCB->xStateListItem ) ); prvAddTaskToReadyList( pxUnblockedTCB ); + #if( configUSE_TICKLESS_IDLE != 0 ) + { + /* If a task is blocked on a kernel object then xNextTaskUnblockTime + might be set to the blocked task's time out time. If the task is + unblocked for a reason other than a timeout xNextTaskUnblockTime is + normally left unchanged, because it is automatically reset to a new + value when the tick count equals xNextTaskUnblockTime. However if + tickless idling is used it might be more important to enter sleep mode + at the earliest possible time - so reset xNextTaskUnblockTime here to + ensure it is updated at the earliest possible time. */ + prvResetNextTaskUnblockTime(); + } + #endif } else { @@ -3068,20 +3081,6 @@ BaseType_t xReturn; xReturn = pdFALSE; } - #if( configUSE_TICKLESS_IDLE != 0 ) - { - /* If a task is blocked on a kernel object then xNextTaskUnblockTime - might be set to the blocked task's time out time. If the task is - unblocked for a reason other than a timeout xNextTaskUnblockTime is - normally left unchanged, because it is automatically reset to a new - value when the tick count equals xNextTaskUnblockTime. However if - tickless idling is used it might be more important to enter sleep mode - at the earliest possible time - so reset xNextTaskUnblockTime here to - ensure it is updated at the earliest possible time. */ - prvResetNextTaskUnblockTime(); - } - #endif - return xReturn; } /*-----------------------------------------------------------*/