From 71decf1197d138276827fa258707720112a4a930 Mon Sep 17 00:00:00 2001 From: liqigan Date: Thu, 9 Jul 2026 19:24:04 +0800 Subject: [PATCH] fix(bt/bluedroid): Fixed use after free issue on osi_event_delete --- components/bt/common/btc/core/btc_task.c | 18 + components/bt/common/osi/include/osi/thread.h | 3 + components/bt/common/osi/thread.c | 398 ++++++++++++++++-- 3 files changed, 375 insertions(+), 44 deletions(-) diff --git a/components/bt/common/btc/core/btc_task.c b/components/bt/common/btc/core/btc_task.c index 35840a1d414..02d3ef214ee 100644 --- a/components/bt/common/btc/core/btc_task.c +++ b/components/bt/common/btc/core/btc_task.c @@ -542,14 +542,26 @@ error_exit:; bt_status_t btc_init(void) { const size_t workqueue_len[] = {BTC_TASK_WORKQUEUE0_LEN, BTC_TASK_WORKQUEUE1_LEN}; + + /* The osi_event subsystem must be ready before any osi_event_create() + * (e.g. btc_gap_ble_init() below). It cannot live in osi_init(), which + * runs later in the BTC task via bte_main_boot_entry(). */ + if (osi_thread_event_init() != 0) { + return BT_STATUS_NOMEM; + } + btc_thread = osi_thread_create(BTC_TASK_NAME, BTC_TASK_STACK_SIZE, BTC_TASK_PRIO, BTC_TASK_PINNED_TO_CORE, BTC_TASK_WORKQUEUE_NUM, workqueue_len, false); if (btc_thread == NULL) { + osi_thread_event_deinit(); return BT_STATUS_NOMEM; } #if BTC_DYNAMIC_MEMORY if (btc_init_mem() != BT_STATUS_SUCCESS){ + osi_thread_free(btc_thread); + btc_thread = NULL; + osi_thread_event_deinit(); return BT_STATUS_NOMEM; } #endif @@ -599,6 +611,12 @@ void btc_deinit(void) osi_thread_free(btc_thread); btc_thread = NULL; + + /* Tear down the osi_event subsystem last: btc_gap_ble_deinit() above may + * still call osi_event_delete(), which needs the global event lock. This + * mirrors moving osi_thread_event_init() into btc_init(); osi_deinit() + * (run earlier via bte_main_shutdown()) no longer owns this lifecycle. */ + osi_thread_event_deinit(); } int get_btc_work_queue_size(void) diff --git a/components/bt/common/osi/include/osi/thread.h b/components/bt/common/osi/include/osi/thread.h index eb72d94e26d..a4038bf230a 100644 --- a/components/bt/common/osi/include/osi/thread.h +++ b/components/bt/common/osi/include/osi/thread.h @@ -118,4 +118,7 @@ void osi_event_delete(struct osi_event* event); */ bool osi_thread_post_event(struct osi_event *event, uint32_t timeout); +int osi_thread_event_init(void); +void osi_thread_event_deinit(void); + #endif /* __THREAD_H__ */ diff --git a/components/bt/common/osi/thread.c b/components/bt/common/osi/thread.c index 1b5bd8b00e5..ec409c634d1 100644 --- a/components/bt/common/osi/thread.c +++ b/components/bt/common/osi/thread.c @@ -18,12 +18,13 @@ #include -#include "osi/allocator.h" #include "freertos/FreeRTOS.h" #include "freertos/queue.h" +#include "osi/allocator.h" +#include "osi/list.h" +#include "osi/mutex.h" #include "osi/semaphore.h" #include "osi/thread.h" -#include "osi/mutex.h" struct work_item { osi_thread_func_t func; @@ -58,17 +59,43 @@ struct osi_thread_start_arg { struct osi_event { struct work_item item; osi_mutex_t lock; - uint16_t is_queued; - uint16_t queue_idx; osi_thread_t *thread; + size_t ref_count; + uint8_t flags; + uint8_t queue_idx; }; +#define OSI_EVENT_FLAG_QUEUED (1U << 0) +#define OSI_EVENT_FLAG_POSTING (1U << 1) +#define OSI_EVENT_FLAG_DELETING (1U << 2) +#define OSI_EVENT_FLAG_RUNNING (1U << 3) + +#define OSI_EVENT_HAS_FLAG(event, flag) (((event)->flags & (flag)) != 0) +#define OSI_EVENT_SET_FLAG(event, flag) ((event)->flags |= (uint8_t)(flag)) +#define OSI_EVENT_CLEAR_FLAG(event, flag) ((event)->flags &= (uint8_t)(~(flag))) + static const size_t DEFAULT_WORK_QUEUE_CAPACITY = 100; +static list_t *s_osi_event_list; +static osi_mutex_t s_osi_event_lock; #if OSI_THREAD_DEBUG static void osi_thread_run_item(osi_thread_t *thread, int wq_idx, struct work_item *item); #endif +static void osi_thread_generic_event_handler(void *context); +static void osi_thread_generic_event_drain(void *context); + +static void osi_event_lock(void) +{ + assert(s_osi_event_lock != NULL); + osi_mutex_lock(&s_osi_event_lock, OSI_MUTEX_MAX_TIMEOUT); +} + +static void osi_event_unlock(void) +{ + osi_mutex_unlock(&s_osi_event_lock); +} + static struct work_queue *osi_work_queue_create(size_t capacity) { if (capacity == 0) { @@ -353,6 +380,38 @@ void osi_thread_free(osi_thread_t *thread) osi_thread_stop(thread); + /* The thread has stopped, so any work items still queued will never be + * drained by osi_thread_run. We must reclaim them here before the queues + * are destroyed, but we MUST NOT blindly execute their handlers: + * + * - Event work items (func == osi_thread_generic_event_handler) hold a + * reference on their osi_event. If that reference is never released the + * osi_event leaks. We reclaim it via osi_thread_generic_event_drain(), + * which only drops the queued reference (and frees the event if it was + * already deleted) WITHOUT running the user callback. This is safe on + * every shutdown path, including ones where the osi_event subsystem has + * already been torn down (osi_thread_event_deinit() freed + * s_osi_event_lock): the release path uses the per-event lock only and + * never touches the global s_osi_event_lock. + * + * - Any other work item was posted directly via osi_thread_post() with an + * arbitrary handler (e.g. btu_hci_msg_process, bta_sys_event, alarm + * handlers). Running such a handler here would dispatch into + * protocol-stack state (L2CAP/BTA/...) that may already have been freed + * by the caller before osi_thread_free() (e.g. BTU_ShutDown() calls + * btu_task_shut_down() first), turning the drain into a use-after-free. + * These items are therefore discarded, matching the pre-existing + * behavior where a destroyed queue silently dropped its contents. */ + for (int i = 0; i < thread->work_queue_num; i++) { + struct work_item item; + while (thread->work_queues[i] && + osi_thead_work_queue_get(thread->work_queues[i], &item) == true) { + if (item.func == osi_thread_generic_event_handler) { + osi_thread_generic_event_drain(item.context); + } + } + } + for (int i = 0; i < thread->work_queue_num; i++) { if (thread->work_queues[i]) { osi_work_queue_delete(thread->work_queues[i]); @@ -435,22 +494,13 @@ int osi_thread_queue_wait_size(osi_thread_t *thread, int wq_idx) } -struct osi_event *osi_event_create(osi_thread_func_t func, void *context) +static bool osi_event_add_alive_locked(struct osi_event *event) { - struct osi_event *event = osi_calloc(sizeof(struct osi_event)); - if (event != NULL) { - if (osi_mutex_new(&event->lock) == 0) { - event->item.func = func; - event->item.context = context; - return event; - } - osi_free(event); - } - - return NULL; + assert(s_osi_event_list != NULL); + return list_append(s_osi_event_list, event); } -void osi_event_delete(struct osi_event* event) +static void osi_event_free(struct osi_event *event) { if (event != NULL) { osi_mutex_free(&event->lock); @@ -459,59 +509,319 @@ void osi_event_delete(struct osi_event* event) } } -bool osi_event_bind(struct osi_event* event, osi_thread_t *thread, int queue_idx) +static bool osi_event_is_idle(const struct osi_event *event) { - if (event == NULL || event->thread != NULL) { + return !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_QUEUED) && + !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_POSTING) && + !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_RUNNING); +} + +static bool osi_event_should_free(const struct osi_event *event) +{ + return OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING) && + osi_event_is_idle(event); +} + +static bool osi_event_can_bind_locked(const struct osi_event *event, osi_thread_t *thread, int queue_idx) +{ + return !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING) && + event->thread == NULL && + thread != NULL && + queue_idx >= 0 && + queue_idx < thread->work_queue_num; +} + +static bool osi_event_can_post_locked(const struct osi_event *event) +{ + if (event->thread == NULL || event->queue_idx >= event->thread->work_queue_num) { return false; } - if (thread == NULL || queue_idx >= thread->work_queue_num) { + return !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING) && + event->item.func != NULL && + !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_QUEUED) && + !OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_POSTING); +} + +static bool osi_event_is_alive_locked(const struct osi_event *event) +{ + /* Do not dereference event here: callers may pass a stale pointer racing + * with osi_event_delete(). The alive list is the ownership boundary. */ + return s_osi_event_list != NULL && list_contains(s_osi_event_list, event); +} + +static bool osi_event_remove_alive_locked(struct osi_event *event) +{ + bool removed = false; + + if (s_osi_event_list == NULL) { return false; } - event->thread = thread; - event->queue_idx = queue_idx; + removed = list_delete(s_osi_event_list, event); + return removed; +} - return true; +struct osi_event *osi_event_create(osi_thread_func_t func, void *context) +{ + bool added = false; + struct osi_event *event = osi_calloc(sizeof(struct osi_event)); + + if (event == NULL) { + return NULL; + } + + if (osi_mutex_new(&event->lock) != 0) { + osi_free(event); + return NULL; + } + + event->item.func = func; + event->item.context = context; + event->ref_count = 1; + + osi_event_lock(); + added = osi_event_add_alive_locked(event); + osi_event_unlock(); + if (added) { + return event; + } + + osi_mutex_free(&event->lock); + osi_free(event); + return NULL; +} + +/* ref_count is protected by the per-event lock (event->lock), NOT the global + * s_osi_event_lock. This keeps the reference-release path independent of the + * global event subsystem: it must stay valid even after + * osi_thread_event_deinit() has freed s_osi_event_lock (e.g. when a thread is + * freed on a shutdown path that tears the event subsystem down first). The + * global lock is only used to gate the alive-list membership that decides + * whether a new reference may be acquired. */ +static bool osi_event_acquire(struct osi_event *event) +{ + bool acquired = false; + + if (event == NULL) { + return false; + } + + /* Hold the global lock so the event cannot be removed from the alive list + * (and thus cannot be freed) while we take a fresh reference. Nesting is + * always global-lock-outer, event->lock-inner; no path takes them in the + * reverse order, so this cannot deadlock. */ + osi_event_lock(); + if (osi_event_is_alive_locked(event)) { + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + assert(event->ref_count > 0); + event->ref_count++; + osi_mutex_unlock(&event->lock); + acquired = true; + } + osi_event_unlock(); + + return acquired; +} + +static void osi_event_retain(struct osi_event *event) +{ + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + assert(event->ref_count > 0); + event->ref_count++; + osi_mutex_unlock(&event->lock); +} + +static void osi_event_release(struct osi_event *event) +{ + bool should_free = false; + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + assert(event->ref_count > 0); + event->ref_count--; + if (event->ref_count == 0) { + should_free = osi_event_should_free(event); + } + osi_mutex_unlock(&event->lock); + + if (should_free) { + osi_event_free(event); + } +} + +void osi_event_delete(struct osi_event *event) +{ + bool removed = false; + + if (event == NULL) { + return; + } + + osi_event_lock(); + removed = osi_event_remove_alive_locked(event); + osi_event_unlock(); + if (!removed) { + return; + } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_DELETING); + event->item.func = NULL; + event->item.context = NULL; + osi_mutex_unlock(&event->lock); + + osi_event_release(event); +} + +bool osi_event_bind(struct osi_event *event, osi_thread_t *thread, int queue_idx) +{ + bool ret = false; + + if (!osi_event_acquire(event)) { + return false; + } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + if (osi_event_can_bind_locked(event, thread, queue_idx)) { + event->thread = thread; + event->queue_idx = queue_idx; + ret = true; + } + osi_mutex_unlock(&event->lock); + + osi_event_release(event); + + return ret; } static void osi_thread_generic_event_handler(void *context) { struct osi_event *event = (struct osi_event *)context; - if (event != NULL && event->item.func != NULL) { - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - event->is_queued = 0; - osi_mutex_unlock(&event->lock); - event->item.func(event->item.context); + osi_thread_func_t func = NULL; + void *func_context = NULL; + + if (event == NULL) { + return; } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_QUEUED); + if (OSI_EVENT_HAS_FLAG(event, OSI_EVENT_FLAG_DELETING)) { + osi_mutex_unlock(&event->lock); + osi_event_release(event); + return; + } + OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_RUNNING); + func = event->item.func; + func_context = event->item.context; + osi_mutex_unlock(&event->lock); + + if (func != NULL) { + func(func_context); + } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_RUNNING); + osi_mutex_unlock(&event->lock); + + osi_event_release(event); +} + +/* Reclaim a queued event work item during thread teardown WITHOUT invoking the + * user callback. It only clears the QUEUED flag and drops the reference the + * queued item owns (osi_thread_post_event() retained it); this frees the event + * if it was already deleted, and leaves a still-live event untouched. Unlike + * osi_thread_generic_event_handler(), it never dispatches into stack state that + * may already have been freed on the shutdown path. */ +static void osi_thread_generic_event_drain(void *context) +{ + struct osi_event *event = (struct osi_event *)context; + + if (event == NULL) { + return; + } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_QUEUED); + osi_mutex_unlock(&event->lock); + + osi_event_release(event); } bool osi_thread_post_event(struct osi_event *event, uint32_t timeout) { - assert(event != NULL && event->thread != NULL); - assert(event->queue_idx >= 0 && event->queue_idx < event->thread->work_queue_num); bool ret = false; - if (event->is_queued == 0) { - uint16_t acquire_cnt = 0; - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - event->is_queued += 1; - acquire_cnt = event->is_queued; - osi_mutex_unlock(&event->lock); + osi_thread_t *thread = NULL; + uint8_t queue_idx = 0; - if (acquire_cnt == 1) { - ret = osi_thread_post(event->thread, osi_thread_generic_event_handler, event, event->queue_idx, timeout); - if (!ret) { - // clear "is_queued" when post failure, to allow for following event posts - osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); - event->is_queued = 0; - osi_mutex_unlock(&event->lock); - } + if (!osi_event_acquire(event)) { + return false; + } + + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + if (!osi_event_can_post_locked(event)) { + osi_mutex_unlock(&event->lock); + osi_event_release(event); + return false; + } + OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_QUEUED); + OSI_EVENT_SET_FLAG(event, OSI_EVENT_FLAG_POSTING); + thread = event->thread; + queue_idx = event->queue_idx; + osi_mutex_unlock(&event->lock); + + /* The queued work item owns a reference until the generic handler drains it. */ + osi_event_retain(event); + ret = osi_thread_post(thread, osi_thread_generic_event_handler, event, queue_idx, timeout); + osi_mutex_lock(&event->lock, OSI_MUTEX_MAX_TIMEOUT); + OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_POSTING); + if (!ret) { + // clear "is_queued" when post failure, to allow for following event posts + OSI_EVENT_CLEAR_FLAG(event, OSI_EVENT_FLAG_QUEUED); + } + osi_mutex_unlock(&event->lock); + + if (!ret) { + osi_event_release(event); + } + osi_event_release(event); + + return ret; +} + +int osi_thread_event_init(void) +{ + int ret = -1; + + do { + if (osi_mutex_new(&s_osi_event_lock) != 0) { + break; } + + s_osi_event_list = list_new(NULL); + if (s_osi_event_list == NULL) { + break; + } + + ret = 0; + } while (0); + + if (ret != 0) { + osi_thread_event_deinit(); } return ret; } +void osi_thread_event_deinit(void) +{ + if (s_osi_event_list != NULL) { + list_free(s_osi_event_list); + s_osi_event_list = NULL; + } + osi_mutex_free(&s_osi_event_lock); +} + #if OSI_THREAD_DEBUG static void osi_thread_run_item(osi_thread_t *thread, int wq_idx, struct work_item *item) {