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:
Konstantin Kondrashov
2026-07-07 17:24:23 +03:00
parent c0fc78ca26
commit 90bfe49549
3 changed files with 62 additions and 5 deletions

View File

@@ -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));

View File

@@ -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")

View File

@@ -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 */