mirror of
https://github.com/espressif/esp-idf.git
synced 2026-09-22 13:01:16 +03:00
fix(esp_event): prevent UAF race between post and loop delete (SEC-222)
esp_event_post_to() could access loop->queue / loop->mutex after esp_event_loop_delete() freed them when both ran concurrently. Introduce esp_event_loop_state_t with: - posts_in_flight: reference-count incremented atomically (under state.lock spinlock) before touching any loop resources, decremented on every exit path via goto on_err. - deleting: atomic_bool set by esp_event_loop_delete() to block new posts from entering the critical section. esp_event_loop_delete() sets deleting=true, then busy-waits (releasing and re-acquiring loop->mutex each tick) until posts_in_flight reaches zero before proceeding with teardown. esp_event_isr_post_to() performs a lock-free atomic_load of deleting as a best-effort guard; ISR context cannot participate in the spinlock protocol but the window is documented and accepted.
This commit is contained in:
@@ -535,6 +535,15 @@ static esp_err_t find_and_unregister_handler(esp_event_remove_handler_context_t*
|
||||
return esp_event_post_to(ctx->loop, esp_event_handler_cleanup, 0, ctx, sizeof(esp_event_remove_handler_context_t), portMAX_DELAY);
|
||||
}
|
||||
|
||||
static bool event_loop_has_posts_in_flight(esp_event_loop_instance_t* loop)
|
||||
{
|
||||
portENTER_CRITICAL(&loop->state.lock);
|
||||
bool posts_in_flight = loop->state.posts_in_flight > 0;
|
||||
portEXIT_CRITICAL(&loop->state.lock);
|
||||
|
||||
return posts_in_flight;
|
||||
}
|
||||
|
||||
/* ---------------------------- Public API --------------------------------- */
|
||||
|
||||
esp_err_t esp_event_loop_create(const esp_event_loop_args_t* event_loop_args, esp_event_loop_handle_t* event_loop)
|
||||
@@ -570,6 +579,10 @@ esp_err_t esp_event_loop_create(const esp_event_loop_args_t* event_loop_args, es
|
||||
goto on_err;
|
||||
}
|
||||
|
||||
loop->state.lock = (portMUX_TYPE)portMUX_INITIALIZER_UNLOCKED;
|
||||
atomic_init(&loop->state.deleting, false);
|
||||
loop->state.posts_in_flight = 0;
|
||||
|
||||
SLIST_INIT(&(loop->loop_nodes));
|
||||
|
||||
// Create the loop task if requested
|
||||
@@ -743,7 +756,15 @@ esp_err_t esp_event_loop_delete(esp_event_loop_handle_t event_loop)
|
||||
esp_event_loop_instance_t* loop = (esp_event_loop_instance_t*) event_loop;
|
||||
SemaphoreHandle_t loop_mutex = loop->mutex;
|
||||
|
||||
xSemaphoreTakeRecursive(loop->mutex, portMAX_DELAY);
|
||||
xSemaphoreTakeRecursive(loop_mutex, portMAX_DELAY);
|
||||
|
||||
atomic_store(&loop->state.deleting, true);
|
||||
while (event_loop_has_posts_in_flight(loop)) {
|
||||
xSemaphoreGiveRecursive(loop_mutex);
|
||||
// Wait for the posts in flight to finish
|
||||
vTaskDelay(1);
|
||||
xSemaphoreTakeRecursive(loop_mutex, portMAX_DELAY);
|
||||
}
|
||||
|
||||
#ifdef CONFIG_ESP_EVENT_LOOP_PROFILING
|
||||
portENTER_CRITICAL(&s_event_loops_spinlock);
|
||||
@@ -934,15 +955,25 @@ esp_err_t esp_event_post_to(esp_event_loop_handle_t event_loop, esp_event_base_t
|
||||
|
||||
esp_event_loop_instance_t* loop = (esp_event_loop_instance_t*) event_loop;
|
||||
|
||||
portENTER_CRITICAL(&loop->state.lock);
|
||||
if (atomic_load(&loop->state.deleting)) {
|
||||
portEXIT_CRITICAL(&loop->state.lock);
|
||||
return ESP_ERR_INVALID_STATE;
|
||||
}
|
||||
loop->state.posts_in_flight++;
|
||||
portEXIT_CRITICAL(&loop->state.lock);
|
||||
|
||||
esp_event_post_instance_t post;
|
||||
memset((void*)(&post), 0, sizeof(post));
|
||||
esp_err_t err = ESP_OK;
|
||||
|
||||
if (event_data != NULL && event_data_size != 0) {
|
||||
// Make persistent copy of event data on heap.
|
||||
void* event_data_copy = calloc(1, event_data_size);
|
||||
|
||||
if (event_data_copy == NULL) {
|
||||
return ESP_ERR_NO_MEM;
|
||||
err = ESP_ERR_NO_MEM;
|
||||
goto on_err;
|
||||
}
|
||||
|
||||
memcpy(event_data_copy, event_data, event_data_size);
|
||||
@@ -987,14 +1018,20 @@ esp_err_t esp_event_post_to(esp_event_loop_handle_t event_loop, esp_event_base_t
|
||||
#ifdef CONFIG_ESP_EVENT_LOOP_PROFILING
|
||||
atomic_fetch_add(&loop->events_dropped, 1);
|
||||
#endif
|
||||
return ESP_ERR_TIMEOUT;
|
||||
err = ESP_ERR_TIMEOUT;
|
||||
goto on_err;
|
||||
}
|
||||
|
||||
#ifdef CONFIG_ESP_EVENT_LOOP_PROFILING
|
||||
atomic_fetch_add(&loop->events_received, 1);
|
||||
#endif
|
||||
|
||||
return ESP_OK;
|
||||
on_err:
|
||||
portENTER_CRITICAL(&loop->state.lock);
|
||||
loop->state.posts_in_flight--;
|
||||
portEXIT_CRITICAL(&loop->state.lock);
|
||||
|
||||
return err;
|
||||
}
|
||||
|
||||
#if CONFIG_ESP_EVENT_POST_FROM_ISR
|
||||
@@ -1009,6 +1046,10 @@ esp_err_t esp_event_isr_post_to(esp_event_loop_handle_t event_loop, esp_event_ba
|
||||
|
||||
esp_event_loop_instance_t* loop = (esp_event_loop_instance_t*) event_loop;
|
||||
|
||||
if (atomic_load(&loop->state.deleting)) {
|
||||
return ESP_ERR_INVALID_STATE;
|
||||
}
|
||||
|
||||
esp_event_post_instance_t post;
|
||||
memset((void*)(&post), 0, sizeof(post));
|
||||
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
extern "C" {
|
||||
#include "Mocktask.h"
|
||||
#include "Mockqueue.h"
|
||||
#include "Mockportmacro.h"
|
||||
}
|
||||
|
||||
namespace {
|
||||
@@ -102,6 +103,8 @@ TEST_CASE("test esp_event_loop_create no_task(void)")
|
||||
xQueueTakeMutexRecursive_IgnoreAndReturn(0);
|
||||
xQueueGiveMutexRecursive_IgnoreAndReturn(0);
|
||||
xQueueReceive_IgnoreAndReturn(0);
|
||||
vPortEnterCritical_Ignore();
|
||||
vPortExitCritical_Ignore();
|
||||
esp_event_loop_handle_t loop = nullptr;
|
||||
|
||||
esp_event_loop_args_t loop_args = test_event_get_default_loop_args();
|
||||
@@ -115,6 +118,8 @@ TEST_CASE("test esp_event_loop_create no_task(void)")
|
||||
xQueueReceive_StopIgnore();
|
||||
xQueueTakeMutexRecursive_StopIgnore();
|
||||
xQueueGiveMutexRecursive_StopIgnore();
|
||||
vPortEnterCritical_StopIgnore();
|
||||
vPortExitCritical_StopIgnore();
|
||||
}
|
||||
|
||||
TEST_CASE("test esp_event_loop_create with_task(void)")
|
||||
@@ -125,6 +130,8 @@ TEST_CASE("test esp_event_loop_create with_task(void)")
|
||||
xQueueTakeMutexRecursive_IgnoreAndReturn(0);
|
||||
xQueueGiveMutexRecursive_IgnoreAndReturn(0);
|
||||
xQueueReceive_IgnoreAndReturn(0);
|
||||
vPortEnterCritical_Ignore();
|
||||
vPortExitCritical_Ignore();
|
||||
esp_event_loop_handle_t loop = nullptr;
|
||||
|
||||
esp_event_loop_args_t loop_args = test_event_get_default_loop_args();
|
||||
@@ -138,6 +145,8 @@ TEST_CASE("test esp_event_loop_create with_task(void)")
|
||||
xQueueReceive_StopIgnore();
|
||||
xQueueTakeMutexRecursive_StopIgnore();
|
||||
xQueueGiveMutexRecursive_StopIgnore();
|
||||
vPortEnterCritical_StopIgnore();
|
||||
vPortExitCritical_StopIgnore();
|
||||
}
|
||||
|
||||
TEST_CASE("registering with ANY_BASE but specific ID fails")
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD
|
||||
* SPDX-FileCopyrightText: 2018-2026 Espressif Systems (Shanghai) CO LTD
|
||||
*
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
@@ -66,6 +66,12 @@ typedef struct esp_event_loop_node {
|
||||
|
||||
typedef SLIST_HEAD(esp_event_loop_nodes, esp_event_loop_node) esp_event_loop_nodes_t;
|
||||
|
||||
typedef struct esp_event_loop_state {
|
||||
portMUX_TYPE lock; /**< spinlock protecting deleting and posts_in_flight */
|
||||
atomic_bool deleting; /**< true when loop deletion has started; read lock-free from ISR */
|
||||
uint32_t posts_in_flight; /**< task-context posts that passed the post-entry gate */
|
||||
} esp_event_loop_state_t;
|
||||
|
||||
/// Event loop
|
||||
typedef struct esp_event_loop_instance {
|
||||
const char* name; /**< name of this event loop */
|
||||
@@ -76,6 +82,7 @@ typedef struct esp_event_loop_instance {
|
||||
SemaphoreHandle_t mutex; /**< mutex for updating the events linked list */
|
||||
esp_event_loop_nodes_t loop_nodes; /**< set of linked lists containing the
|
||||
registered handlers for the loop */
|
||||
esp_event_loop_state_t state; /**< loop deletion and post-entry state */
|
||||
#ifdef CONFIG_ESP_EVENT_LOOP_PROFILING
|
||||
atomic_uint_least32_t events_received; /**< number of events successfully posted to the loop */
|
||||
atomic_uint_least32_t events_dropped; /**< number of events dropped due to queue being full */
|
||||
|
||||
Reference in New Issue
Block a user