[core] Fix use-after-free when deleting a running StaticTask (#19048)

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Kevin Ahrendt
2026-09-09 09:54:42 +12:00
committed by Jesse Hills
co-authored by Copilot Autofix powered by AI
parent f89b9e704c
commit 9b6facb20d
6 changed files with 50 additions and 20 deletions
@@ -446,9 +446,9 @@ void MicroWakeWord::loop() {
xEventGroupClearBits(this->event_group_, EventGroupBits::TASK_STOPPING);
}
if ((event_group_bits & EventGroupBits::TASK_STOPPED)) {
// Retries on a subsequent loop if the task is still running on the other core
if ((event_group_bits & EventGroupBits::TASK_STOPPED) && this->inference_task_.deallocate()) {
ESP_LOGD(TAG, "Inference task is finished, freeing task resources");
this->inference_task_.deallocate();
xEventGroupClearBits(this->event_group_, ALL_BITS);
xQueueReset(this->detection_queue_);
this->set_state_(State::STOPPED);
@@ -382,8 +382,8 @@ void MixerSpeaker::loop() {
ESP_LOGV(TAG, "Stopping");
xEventGroupClearBits(this->event_group_, MIXER_TASK_STATE_STOPPING);
}
if (event_group_bits & MIXER_TASK_STATE_STOPPED) {
this->task_.deallocate();
// Retries on a subsequent loop if the task is still running on the other core
if ((event_group_bits & MIXER_TASK_STATE_STOPPED) && this->task_.deallocate()) {
ESP_LOGD(TAG, "Stopped");
xEventGroupClearBits(this->event_group_, MIXER_TASK_ALL_BITS);
this->all_stopped_since_ms_ = 0;
@@ -153,8 +153,8 @@ void ResamplerSpeaker::loop() {
ESP_LOGV(TAG, "Stopping");
xEventGroupClearBits(this->event_group_, ResamplingEventGroupBits::STATE_STOPPING);
}
if (event_group_bits & ResamplingEventGroupBits::STATE_STOPPED) {
this->task_.deallocate();
// Retries on a subsequent loop if the task is still running on the other core
if ((event_group_bits & ResamplingEventGroupBits::STATE_STOPPED) && this->task_.deallocate()) {
ESP_LOGD(TAG, "Stopped");
xEventGroupClearBits(this->event_group_, ResamplingEventGroupBits::ALL_BITS);
}
@@ -202,8 +202,15 @@ AudioPipelineState AudioPipeline::process_state() {
if (!this->is_playing_) {
// The tasks have been stopped for two ``process_state`` calls in a row, so delete the tasks
if (this->read_task_.is_created() || this->decode_task_.is_created()) {
this->read_task_.deallocate();
this->decode_task_.deallocate();
// Both are attempted every time; a task that is still running on the other core is freed by a
// subsequent call, and freeing an already freed task succeeds without doing anything
bool read_task_freed = this->read_task_.deallocate();
bool decode_task_freed = this->decode_task_.deallocate();
if (!read_task_freed || !decode_task_freed) {
// A task is still running on the other core, so keep the pipeline in its current state and try
// again on the next call
return AudioPipelineState::PLAYING;
}
if (this->hard_stop_) {
// Stop command was sent, so immediately end the playback
this->speaker_->stop();
+23 -7
View File
@@ -40,16 +40,31 @@ bool StaticTask::create(TaskFunction_t fn, const char *name, uint32_t stack_size
return true;
}
void StaticTask::destroy() {
if (this->handle_ != nullptr) {
TaskHandle_t handle = this->handle_;
this->handle_ = nullptr;
vTaskDelete(handle);
bool StaticTask::destroy() {
if (this->handle_ == nullptr) {
return true;
}
// Suspending takes the task off the ready and event lists, so nothing can schedule it again. It only asks
// the other core to yield though, so the task may still be running on it for a moment.
vTaskSuspend(this->handle_);
if (eTaskGetState(this->handle_) != eSuspended) {
// The task is still running on the other core and using its stack. Deleting it now would only put it on
// the termination list and return, so the caller has to try again once it has been swapped out.
return false;
}
// The task cannot run again, so the delete completes right away instead of being left to the idle task.
TaskHandle_t handle = this->handle_;
this->handle_ = nullptr;
vTaskDelete(handle);
return true;
}
void StaticTask::deallocate() {
this->destroy();
bool StaticTask::deallocate() {
if (!this->destroy()) {
return false;
}
if (this->stack_buffer_ != nullptr) {
RAMAllocator<StackType_t> allocator(this->use_psram_ ? RAMAllocator<StackType_t>::ALLOC_EXTERNAL
: RAMAllocator<StackType_t>::ALLOC_INTERNAL);
@@ -57,6 +72,7 @@ void StaticTask::deallocate() {
this->stack_buffer_ = nullptr;
this->stack_size_ = 0;
}
return true;
}
} // namespace esphome
+12 -5
View File
@@ -11,6 +11,7 @@ namespace esphome {
/** Helper for FreeRTOS static task management.
* Bundles TaskHandle_t, StaticTask_t, and the stack buffer into one object with create/destroy methods.
* Call destroy() and deallocate() from another task: a task cannot free the stack it is still running on.
*/
class StaticTask {
public:
@@ -23,7 +24,7 @@ class StaticTask {
/// @brief Allocate stack and create task.
/// @param fn Task function
/// @param name Task name (for debug)
/// @param stack_size Stack size in StackType_t words
/// @param stack_size Stack size in bytes (StackType_t is a byte on ESP-IDF)
/// @param param Parameter passed to task function
/// @param priority FreeRTOS task priority
/// @param use_psram If true, allocate stack in PSRAM; otherwise internal RAM
@@ -31,11 +32,17 @@ class StaticTask {
bool create(TaskFunction_t fn, const char *name, uint32_t stack_size, void *param, UBaseType_t priority,
bool use_psram);
/// @brief Delete the task but keep the stack buffer allocated for reuse by a subsequent create() call.
void destroy();
/// @brief Delete the task, keeping the stack buffer allocated for reuse by a subsequent create() call.
/// The task must have finished its work and parked itself, either suspended or blocked indefinitely: it is
/// suspended here so that it cannot be scheduled again, and it is given no chance to clean up.
/// @return true if the task was deleted; false if it is still running on another core, in which case the
/// caller should try again later.
bool destroy();
/// @brief Delete the task (if running) and free the stack buffer.
void deallocate();
/// @brief Delete the task (if created) and free the stack buffer.
/// @return true if the stack buffer was freed; false if the task is still running on another core, in
/// which case the caller should try again later.
bool deallocate();
protected:
TaskHandle_t handle_{nullptr};