From 6e48173373d792cb19bd7bbd9aef2063c7d2c2b6 Mon Sep 17 00:00:00 2001 From: yangfeng Date: Thu, 11 Jun 2026 14:44:50 +0800 Subject: [PATCH] fix: Fix the critical issues of btu and bt_common from AI review report - advance connect queue on synchronous connect_cb failure - lock bta_alarm_hash_map in all BTA timer APIs - free controller params after stack disable; cleanup on init fail - handle BTE_InitStack failure and signal init future - validate HCI remote name event length before parse - drop stale L2CAP quick-timer alarm events --- .../bt/host/bluedroid/api/esp_bt_main.c | 1 + .../bt/host/bluedroid/bta/sys/bta_sys_main.c | 17 ++++--- .../bt/host/bluedroid/btc/core/btc_main.c | 45 ++++++++++++++++--- .../bluedroid/btc/core/btc_profile_queue.c | 8 +++- .../host/bluedroid/btc/include/btc/btc_main.h | 1 + .../common/include/common/bt_common_types.h | 2 +- components/bt/host/bluedroid/main/bte_main.c | 23 ++++++---- .../bt/host/bluedroid/stack/btu/btu_hcif.c | 5 +++ .../bt/host/bluedroid/stack/btu/btu_init.c | 7 +-- .../bt/host/bluedroid/stack/btu/btu_task.c | 19 ++++++-- .../host/bluedroid/stack/include/stack/btu.h | 2 +- 11 files changed, 101 insertions(+), 29 deletions(-) diff --git a/components/bt/host/bluedroid/api/esp_bt_main.c b/components/bt/host/bluedroid/api/esp_bt_main.c index 4bcd1c3caa5..46fac4e87c9 100644 --- a/components/bt/host/bluedroid/api/esp_bt_main.c +++ b/components/bt/host/bluedroid/api/esp_bt_main.c @@ -203,6 +203,7 @@ esp_err_t esp_bluedroid_init_with_cfg(esp_bluedroid_config_t *cfg) if (future_await(*future_p) == FUTURE_FAIL) { LOG_ERROR("Bluedroid Initialize Fail"); + btc_cleanup_partial_init(); btc_deinit(); bluedroid_config_deinit(); #if HEAP_MEMORY_STATS diff --git a/components/bt/host/bluedroid/bta/sys/bta_sys_main.c b/components/bt/host/bluedroid/bta/sys/bta_sys_main.c index 3bf41d9dabc..1dc09dabcf5 100644 --- a/components/bt/host/bluedroid/bta/sys/bta_sys_main.c +++ b/components/bt/host/bluedroid/bta/sys/bta_sys_main.c @@ -636,11 +636,11 @@ void bta_sys_start_timer(TIMER_LIST_ENT *p_tle, UINT16 type, INT32 timeout_ms) return; } } - osi_mutex_unlock(&bta_alarm_lock); alarm = hash_map_get(bta_alarm_hash_map, p_tle); if (alarm == NULL) { APPL_TRACE_ERROR("%s unable to create alarm.", __func__); + osi_mutex_unlock(&bta_alarm_lock); return; } @@ -648,6 +648,7 @@ void bta_sys_start_timer(TIMER_LIST_ENT *p_tle, UINT16 type, INT32 timeout_ms) p_tle->ticks = timeout_ms; //osi_alarm_set(alarm, (period_ms_t)timeout_ms, bta_alarm_cb, p_tle); osi_alarm_set(alarm, (period_ms_t)timeout_ms); + osi_mutex_unlock(&bta_alarm_lock); } bool hash_iter_ro_cb(hash_map_entry_t *hash_map_entry, void *context) @@ -682,12 +683,12 @@ BOOLEAN bta_sys_timer_is_active(TIMER_LIST_ENT *p_tle) { assert(p_tle != NULL); + osi_mutex_lock(&bta_alarm_lock, OSI_MUTEX_MAX_TIMEOUT); osi_alarm_t *alarm = hash_map_get(bta_alarm_hash_map, p_tle); - if (alarm != NULL && osi_alarm_is_active(alarm)) { - return TRUE; - } + BOOLEAN active = (alarm != NULL && osi_alarm_is_active(alarm)); + osi_mutex_unlock(&bta_alarm_lock); - return FALSE; + return active; } /******************************************************************************* @@ -703,12 +704,15 @@ void bta_sys_stop_timer(TIMER_LIST_ENT *p_tle) { assert(p_tle != NULL); + osi_mutex_lock(&bta_alarm_lock, OSI_MUTEX_MAX_TIMEOUT); osi_alarm_t *alarm = hash_map_get(bta_alarm_hash_map, p_tle); if (alarm == NULL) { APPL_TRACE_DEBUG("%s expected alarm was not in bta alarm hash map.", __func__); + osi_mutex_unlock(&bta_alarm_lock); return; } osi_alarm_cancel(alarm); + osi_mutex_unlock(&bta_alarm_lock); } /******************************************************************************* @@ -724,13 +728,16 @@ void bta_sys_free_timer(TIMER_LIST_ENT *p_tle) { assert(p_tle != NULL); + osi_mutex_lock(&bta_alarm_lock, OSI_MUTEX_MAX_TIMEOUT); osi_alarm_t *alarm = hash_map_get(bta_alarm_hash_map, p_tle); if (alarm == NULL) { APPL_TRACE_DEBUG("%s expected alarm was not in bta alarm hash map.", __func__); + osi_mutex_unlock(&bta_alarm_lock); return; } osi_alarm_cancel(alarm); hash_map_erase(bta_alarm_hash_map, p_tle); + osi_mutex_unlock(&bta_alarm_lock); } /******************************************************************************* diff --git a/components/bt/host/bluedroid/btc/core/btc_main.c b/components/bt/host/bluedroid/btc/core/btc_main.c index 4de7c8aabd7..334a5b1ea17 100644 --- a/components/bt/host/bluedroid/btc/core/btc_main.c +++ b/components/bt/host/bluedroid/btc/core/btc_main.c @@ -18,6 +18,8 @@ #include "bta_dm_int.h" static future_t *main_future[BTC_MAIN_FUTURE_NUM]; +static SemaphoreHandle_t s_init_done_sem = NULL; +static bool s_init_clean = false; extern int bte_main_boot_entry(void *cb); extern int bte_main_shutdown(void); @@ -44,18 +46,46 @@ static void btc_disable_bluetooth(void) } } -void btc_init_callback(void) +void btc_init_callback(bt_status_t status) { - future_ready(*btc_main_get_future_p(BTC_MAIN_INIT_FUTURE), FUTURE_SUCCESS); + s_init_clean = (status == BT_STATUS_SUCCESS) ? false : true; + future_ready(*btc_main_get_future_p(BTC_MAIN_INIT_FUTURE), + (status == BT_STATUS_SUCCESS) ? FUTURE_SUCCESS : FUTURE_FAIL); +} + +void btc_cleanup_partial_init(void) +{ + if (s_init_clean) { + xSemaphoreTake(s_init_done_sem, portMAX_DELAY); + bte_main_shutdown(); +#if (SMP_INCLUDED) + btc_config_clean_up(); +#endif + osi_alarm_deinit(); + osi_alarm_delete_mux(); +#if BTA_DYNAMIC_MEMORY + vSemaphoreDelete(deinit_semaphore); + deinit_semaphore = NULL; +#endif /* #if BTA_DYNAMIC_MEMORY */ + vSemaphoreDelete(s_init_done_sem); + s_init_done_sem = NULL; + s_init_clean = false; + } else { + if (s_init_done_sem) { + vSemaphoreDelete(s_init_done_sem); + s_init_done_sem = NULL; + } + osi_alarm_deinit(); + osi_alarm_delete_mux(); + } } static void btc_init_bluetooth(void) { osi_alarm_create_mux(); osi_alarm_init(); - if (bte_main_boot_entry(btc_init_callback) != 0) { - osi_alarm_deinit(); - osi_alarm_delete_mux(); + s_init_done_sem = xSemaphoreCreateBinary(); + if ((s_init_done_sem == NULL) || (bte_main_boot_entry(btc_init_callback) != 0)) { future_ready(*btc_main_get_future_p(BTC_MAIN_INIT_FUTURE), FUTURE_FAIL); return; } @@ -71,6 +101,7 @@ static void btc_init_bluetooth(void) #if BTA_DYNAMIC_MEMORY deinit_semaphore = xSemaphoreCreateBinary(); #endif /* #if BTA_DYNAMIC_MEMORY */ + xSemaphoreGive(s_init_done_sem); } @@ -98,6 +129,10 @@ static void btc_deinit_bluetooth(void) vSemaphoreDelete(deinit_semaphore); deinit_semaphore = NULL; #endif /* #if BTA_DYNAMIC_MEMORY */ + if (s_init_done_sem) { + vSemaphoreDelete(s_init_done_sem); + s_init_done_sem = NULL; + } } void btc_main_call_handler(btc_msg_t *msg) diff --git a/components/bt/host/bluedroid/btc/core/btc_profile_queue.c b/components/bt/host/bluedroid/btc/core/btc_profile_queue.c index 5e63dfe54c4..231948cff8f 100644 --- a/components/bt/host/bluedroid/btc/core/btc_profile_queue.c +++ b/components/bt/host/bluedroid/btc/core/btc_profile_queue.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -144,7 +144,11 @@ bt_status_t btc_queue_connect_next(void) } p_head->busy = true; - return p_head->connect_cb(&p_head->bda, p_head->uuid); + bt_status_t status = p_head->connect_cb(&p_head->bda, p_head->uuid); + if (status != BT_STATUS_SUCCESS) { + btc_queue_advance(); + } + return status; } diff --git a/components/bt/host/bluedroid/btc/include/btc/btc_main.h b/components/bt/host/bluedroid/btc/include/btc/btc_main.h index 83f87b56a3b..474fa00bc6e 100644 --- a/components/bt/host/bluedroid/btc/include/btc/btc_main.h +++ b/components/bt/host/bluedroid/btc/include/btc/btc_main.h @@ -69,4 +69,5 @@ void btc_deinit_bluetooth(future_t *future); #endif void btc_main_call_handler(btc_msg_t *msg); +void btc_cleanup_partial_init(void); #endif /* __BTC_BT_MAIN_H__ */ diff --git a/components/bt/host/bluedroid/common/include/common/bt_common_types.h b/components/bt/host/bluedroid/common/include/common/bt_common_types.h index 7b96579e1df..eeadbde8d4f 100644 --- a/components/bt/host/bluedroid/common/include/common/bt_common_types.h +++ b/components/bt/host/bluedroid/common/include/common/bt_common_types.h @@ -14,7 +14,7 @@ #include "common/bt_defs.h" #include "osi/thread.h" -typedef void (* bluedroid_init_done_cb_t)(void); +typedef void (* bluedroid_init_done_cb_t)(bt_status_t status); typedef struct { uint8_t client_if; diff --git a/components/bt/host/bluedroid/main/bte_main.c b/components/bt/host/bluedroid/main/bte_main.c index d50544ced4f..4b4d7da6716 100644 --- a/components/bt/host/bluedroid/main/bte_main.c +++ b/components/bt/host/bluedroid/main/bte_main.c @@ -55,7 +55,7 @@ static const hci_t *hci; ** Static functions *******************************************************************************/ static void bte_main_disable(void); -static void bte_main_enable(void); +static bool bte_main_enable(void); /******************************************************************************* ** Externs @@ -92,7 +92,11 @@ int bte_main_boot_entry(bluedroid_init_done_cb_t cb) } //Enable HCI - bte_main_enable(); + if (!bte_main_enable()) { + osi_deinit(); + bluedroid_init_done_cb = NULL; + return -3; + } return 0; } @@ -108,13 +112,12 @@ int bte_main_boot_entry(bluedroid_init_done_cb_t cb) ******************************************************************************/ void bte_main_shutdown(void) { + bte_main_disable(); #if (BT_BLE_DYNAMIC_ENV_MEMORY == TRUE) free_controller_param(); #endif - bte_main_disable(); - osi_deinit(); } @@ -125,19 +128,23 @@ void bte_main_shutdown(void) ** Description BTE MAIN API - Creates all the BTE tasks. Should be called ** part of the Bluetooth stack enable sequence ** -** Returns None +** Returns true for success, otherwise false ** ******************************************************************************/ -static void bte_main_enable(void) +static bool bte_main_enable(void) { APPL_TRACE_DEBUG("Enable HCI\n"); if (hci_start_up()) { APPL_TRACE_ERROR("Start HCI Host Layer Failure\n"); - return; + return false; } //Now Test Case Not Supported BTU - BTU_StartUp(); + if (!BTU_StartUp()) { + hci_shut_down(); + return false; + } + return true; } /****************************************************************************** diff --git a/components/bt/host/bluedroid/stack/btu/btu_hcif.c b/components/bt/host/bluedroid/stack/btu/btu_hcif.c index 49cc69874aa..a1a8cd8394f 100644 --- a/components/bt/host/bluedroid/stack/btu/btu_hcif.c +++ b/components/bt/host/bluedroid/stack/btu/btu_hcif.c @@ -1034,6 +1034,11 @@ static void btu_hcif_rmt_name_request_comp_evt (UINT8 *p, UINT16 evt_len) UINT8 status; BD_ADDR bd_addr; + if (evt_len < (1 + BD_ADDR_LEN)) { + HCI_TRACE_ERROR("HCI_RMT_NAME_REQUEST_COMP_EVT param too short (len=%u)", evt_len); + return; + } + STREAM_TO_UINT8 (status, p); STREAM_TO_BDADDR (bd_addr, p); diff --git a/components/bt/host/bluedroid/stack/btu/btu_init.c b/components/bt/host/bluedroid/stack/btu/btu_init.c index a00b3243982..14747a03fbc 100644 --- a/components/bt/host/bluedroid/stack/btu/btu_init.c +++ b/components/bt/host/bluedroid/stack/btu/btu_init.c @@ -148,10 +148,10 @@ void btu_free_core(void) ** NOTE: Must be called before creating any tasks ** (RPC, BTU, HCIT, APPL, etc.) ** -** Returns void +** Returns true for success, otherwise false ** ******************************************************************************/ -void BTU_StartUp(void) +bool BTU_StartUp(void) { #if BTU_DYNAMIC_MEMORY btu_cb_ptr = (tBTU_CB *)osi_malloc(sizeof(tBTU_CB)); @@ -194,11 +194,12 @@ void BTU_StartUp(void) goto error_exit; } - return; + return true; error_exit:; LOG_ERROR("%s Unable to allocate resources for bt_workqueue", __func__); BTU_ShutDown(); + return false; } /***************************************************************************** diff --git a/components/bt/host/bluedroid/stack/btu/btu_task.c b/components/bt/host/bluedroid/stack/btu/btu_task.c index 6619a392966..242ff8b2301 100644 --- a/components/bt/host/bluedroid/stack/btu/btu_task.c +++ b/components/bt/host/bluedroid/stack/btu/btu_task.c @@ -272,7 +272,13 @@ void btu_task_start_up(void *param) btu_init_core(); /* Initialize any optional stack components */ - BTE_InitStack(); + if (BTE_InitStack() != BT_STATUS_SUCCESS) { + HCI_TRACE_ERROR("BTE_InitStack failed"); + if (bluedroid_init_done_cb) { + bluedroid_init_done_cb(BT_STATUS_NOMEM); + } + return; + } #if (defined(BTA_INCLUDED) && BTA_INCLUDED == TRUE) bta_sys_init(); @@ -280,11 +286,9 @@ void btu_task_start_up(void *param) // Inform the bt jni thread initialization is ok. // btif_transfer_context(btif_init_ok, 0, NULL, 0, NULL); -#if(defined(BT_APP_DEMO) && BT_APP_DEMO == TRUE) if (bluedroid_init_done_cb) { - bluedroid_init_done_cb(); + bluedroid_init_done_cb(BT_STATUS_SUCCESS); } -#endif } void btu_task_shut_down(void) @@ -546,6 +550,13 @@ static void btu_l2cap_alarm_process(void *param) TIMER_LIST_ENT *p_tle = (TIMER_LIST_ENT *)param; assert(p_tle != NULL); + osi_mutex_lock(&btu_l2cap_alarm_lock, OSI_MUTEX_MAX_TIMEOUT); + if (!hash_map_has_key(btu_l2cap_alarm_hash_map, p_tle) || p_tle->in_use == FALSE) { + osi_mutex_unlock(&btu_l2cap_alarm_lock); + return; + } + osi_mutex_unlock(&btu_l2cap_alarm_lock); + switch (p_tle->event) { case BTU_TTYPE_L2CAP_CHNL: /* monitor or retransmission timer */ case BTU_TTYPE_L2CAP_FCR_ACK: /* ack timer */ diff --git a/components/bt/host/bluedroid/stack/include/stack/btu.h b/components/bt/host/bluedroid/stack/include/stack/btu.h index 5009976e3ea..5d2a821d8f5 100644 --- a/components/bt/host/bluedroid/stack/include/stack/btu.h +++ b/components/bt/host/bluedroid/stack/include/stack/btu.h @@ -293,7 +293,7 @@ void btu_hcif_cmd_timeout (UINT8 controller_id); void btu_init_core(void); void btu_free_core(void); -void BTU_StartUp(void); +bool BTU_StartUp(void); void BTU_ShutDown(void); void btu_task_start_up(void *param);