From 80414d0012c85a6c1e40f39136afc726794a1981 Mon Sep 17 00:00:00 2001 From: Zhi Wei Jian Date: Wed, 25 Mar 2026 13:58:25 +0800 Subject: [PATCH] fix(ble/bluedroid): Fix init failure handling, storage/config and GATT leaks - btc_main: handle bte_main_boot_entry failure with cleanup and future_ready(FUTURE_FAIL) - btc_ble_storage: fix key/length validation in _btc_storage_get_ble_bonding_key - btc_config: align return/error contract with callers - btc_dm: use safe BTA_SERVICE_ID_TO_SERVICE_MASK, fix sec_cb_handler type - btc_gatt_util: fix btc_to_bta_response/set_read_value length and bounds - btc_gatts: future_free on early return, max_nb_attr uint16_t, fail cleanup, handle bounds - btc_ble_cte/btc_iso_ble: fix callback type/param consistency with BTA (cherry picked from commit 6f5d9e3440abc84cfc960381254669d3aeb8c721) Co-authored-by: zhiweijian --- .../host/bluedroid/btc/core/btc_ble_storage.c | 1 + .../bt/host/bluedroid/btc/core/btc_config.c | 2 +- .../bt/host/bluedroid/btc/core/btc_dm.c | 12 ++-- .../bt/host/bluedroid/btc/core/btc_main.c | 9 ++- .../btc/profile/std/cte/btc_ble_cte.c | 2 +- .../btc/profile/std/gatt/btc_gatt_util.c | 15 ++-- .../btc/profile/std/gatt/btc_gatts.c | 70 +++++++++++++++++-- .../btc/profile/std/iso/btc_iso_ble.c | 2 +- 8 files changed, 94 insertions(+), 19 deletions(-) diff --git a/components/bt/host/bluedroid/btc/core/btc_ble_storage.c b/components/bt/host/bluedroid/btc/core/btc_ble_storage.c index 2c7c87ef7a..ac810a94bc 100644 --- a/components/bt/host/bluedroid/btc/core/btc_ble_storage.c +++ b/components/bt/host/bluedroid/btc/core/btc_ble_storage.c @@ -177,6 +177,7 @@ static bt_status_t _btc_storage_get_ble_bonding_key(bt_bdaddr_t *remote_bd_addr, break; case BTM_LE_KEY_LID: name = BTC_BLE_STORAGE_LE_KEY_LID_STR; + break; default: return BT_STATUS_FAIL; } diff --git a/components/bt/host/bluedroid/btc/core/btc_config.c b/components/bt/host/bluedroid/btc/core/btc_config.c index 966c33b522..c7edd3f669 100644 --- a/components/bt/host/bluedroid/btc/core/btc_config.c +++ b/components/bt/host/bluedroid/btc/core/btc_config.c @@ -105,7 +105,7 @@ bool btc_config_init(void) return true; -error:; +error: config_free(config); osi_mutex_free(&lock); config = NULL; diff --git a/components/bt/host/bluedroid/btc/core/btc_dm.c b/components/bt/host/bluedroid/btc/core/btc_dm.c index 02b5a16ef4..5f91481b6f 100644 --- a/components/bt/host/bluedroid/btc/core/btc_dm.c +++ b/components/bt/host/bluedroid/btc/core/btc_dm.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -29,7 +29,8 @@ /****************************************************************************** ** Constants & Macros ******************************************************************************/ -#define BTA_SERVICE_ID_TO_SERVICE_MASK(id) (1 << (id)) +/* Use 1ULL to avoid UB: 1 << 32 is undefined when int is 32-bit (C11 ยง6.5.7) */ +#define BTA_SERVICE_ID_TO_SERVICE_MASK(id) ((tBTA_SERVICE_MASK)(1ULL << (id))) /****************************************************************************** ** Static variables @@ -697,7 +698,7 @@ bt_status_t btc_dm_enable_service(tBTA_SERVICE_ID service_id) { tBTA_SERVICE_ID *p_id = &service_id; - btc_dm_cb.btc_enabled_services |= (1 << service_id); + btc_dm_cb.btc_enabled_services |= (tBTA_SERVICE_MASK)(1ULL << service_id); BTC_TRACE_DEBUG("%s: current services:0x%x", __FUNCTION__, btc_dm_cb.btc_enabled_services); @@ -710,7 +711,7 @@ bt_status_t btc_dm_disable_service(tBTA_SERVICE_ID service_id) { tBTA_SERVICE_ID *p_id = &service_id; - btc_dm_cb.btc_enabled_services &= (tBTA_SERVICE_MASK)(~(1 << service_id)); + btc_dm_cb.btc_enabled_services &= (tBTA_SERVICE_MASK)(~(1ULL << service_id)); BTC_TRACE_DEBUG("%s: Current Services:0x%x", __FUNCTION__, btc_dm_cb.btc_enabled_services); @@ -808,8 +809,7 @@ void btc_dm_sec_cb_handler(btc_msg_t *msg) case BTA_DM_DISABLE_EVT: { tBTA_SERVICE_MASK service_mask = btc_get_enabled_services_mask(); for (int i = 0; i <= BTA_MAX_SERVICE_ID; i++) { - if (service_mask & - (tBTA_SERVICE_MASK)(BTA_SERVICE_ID_TO_SERVICE_MASK(i))) { + if (service_mask & BTA_SERVICE_ID_TO_SERVICE_MASK(i)) { btc_in_execute_service_request(i, FALSE); } } diff --git a/components/bt/host/bluedroid/btc/core/btc_main.c b/components/bt/host/bluedroid/btc/core/btc_main.c index 0802202797..4de7c8aabd 100644 --- a/components/bt/host/bluedroid/btc/core/btc_main.c +++ b/components/bt/host/bluedroid/btc/core/btc_main.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -53,7 +53,12 @@ static void btc_init_bluetooth(void) { osi_alarm_create_mux(); osi_alarm_init(); - bte_main_boot_entry(btc_init_callback); + if (bte_main_boot_entry(btc_init_callback) != 0) { + osi_alarm_deinit(); + osi_alarm_delete_mux(); + future_ready(*btc_main_get_future_p(BTC_MAIN_INIT_FUTURE), FUTURE_FAIL); + return; + } #if (SMP_INCLUDED) btc_config_init(); diff --git a/components/bt/host/bluedroid/btc/profile/std/cte/btc_ble_cte.c b/components/bt/host/bluedroid/btc/profile/std/cte/btc_ble_cte.c index 0d01f80c69..9d4d8a53b3 100644 --- a/components/bt/host/bluedroid/btc/profile/std/cte/btc_ble_cte.c +++ b/components/bt/host/bluedroid/btc/profile/std/cte/btc_ble_cte.c @@ -26,7 +26,7 @@ static inline void btc_cte_ble_cb_to_app(esp_ble_cte_cb_event_t event, esp_ble_c static void btc_ble_cte_callback(tBTM_BLE_CTE_EVENT event, tBTM_BLE_CTE_CB_PARAMS *params) { - esp_ble_cte_cb_param_t param; + esp_ble_cte_cb_param_t param = {0}; bt_status_t ret; btc_msg_t msg; msg.sig = BTC_SIG_API_CB; diff --git a/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatt_util.c b/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatt_util.c index 4d0a7b272f..9d8ee85412 100644 --- a/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatt_util.c +++ b/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatt_util.c @@ -114,9 +114,11 @@ void btc_to_bta_response(tBTA_GATTS_RSP *p_dest, esp_gatt_rsp_t *p_src) { p_dest->attr_value.auth_req = p_src->attr_value.auth_req; p_dest->attr_value.handle = p_src->attr_value.handle; - p_dest->attr_value.len = p_src->attr_value.len; p_dest->attr_value.offset = p_src->attr_value.offset; - memcpy(p_dest->attr_value.value, p_src->attr_value.value, ESP_GATT_MAX_ATTR_LEN); + uint16_t copy_len = (p_src->attr_value.len <= ESP_GATT_MAX_ATTR_LEN) + ? p_src->attr_value.len : ESP_GATT_MAX_ATTR_LEN; + p_dest->attr_value.len = copy_len; /* match actual bytes copied (defensive if src len > buffer) */ + memcpy(p_dest->attr_value.value, p_src->attr_value.value, copy_len); } uint16_t get_uuid16(tBT_UUID *p_uuid) @@ -146,13 +148,18 @@ uint16_t set_read_value(uint8_t *gattc_if, esp_ble_gattc_cb_param_t *p_dest, tBT if (( p_src->status == BTA_GATT_OK ) && (p_src->p_value != NULL)) { BTC_TRACE_DEBUG("%s len = %d ", __func__, p_src->p_value->len); - p_dest->read.value_len = p_src->p_value->len; if ( p_src->p_value->len > 0 && p_src->p_value->p_value != NULL ) { + p_dest->read.value_len = p_src->p_value->len; p_dest->read.value = p_src->p_value->p_value; + len += p_src->p_value->len; + } else { + /* len>0 but p_value==NULL would leave value uninitialized; avoid that */ + p_dest->read.value_len = 0; + p_dest->read.value = NULL; } - len += p_src->p_value->len; } else { p_dest->read.value_len = 0; + p_dest->read.value = NULL; } return len; diff --git a/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatts.c b/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatts.c index 893e29e173..116443cb85 100644 --- a/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatts.c +++ b/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatts.c @@ -31,7 +31,7 @@ esp_btc_creat_tab_t *btc_creat_tab_env_ptr; #endif static esp_gatt_status_t btc_gatts_check_valid_attr_tab(esp_gatts_attr_db_t *gatts_attr_db, - uint8_t max_nb_attr); + uint16_t max_nb_attr); static inline void btc_gatts_cb_to_app(esp_gatts_cb_event_t event, esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param) { @@ -42,6 +42,14 @@ static inline void btc_gatts_cb_to_app(esp_gatts_cb_event_t event, esp_gatt_if_t } } +/* Report CREAT_ATTR_TAB error to app and reset env; use on all early-exit error paths in btc_gatts_act_create_attr_tab */ +static void btc_gatts_creat_tab_fail_and_cleanup(esp_gatt_if_t gatts_if, esp_ble_gatts_cb_param_t *param) +{ + param->add_attr_tab.status = ESP_GATT_ERROR; + btc_gatts_cb_to_app(ESP_GATTS_CREAT_ATTR_TAB_EVT, gatts_if, param); + memset(&btc_creat_tab_env, 0, sizeof(esp_btc_creat_tab_t)); +} + static inline void btc_gatts_uuid_format_convert(esp_bt_uuid_t* dest_uuid, uint16_t src_uuid_len, uint8_t* src_uuid_p) { dest_uuid->len = src_uuid_len; @@ -293,11 +301,13 @@ static void btc_gatts_act_create_attr_tab(esp_gatts_attr_db_t *gatts_attr_db, btc_gatts_cb_to_app(ESP_GATTS_CREAT_ATTR_TAB_EVT, gatts_if, ¶m); //reset the env after sent the data to app memset(&btc_creat_tab_env, 0, sizeof(esp_btc_creat_tab_t)); + future_free(future_p); return; } if (future_await(future_p) == FUTURE_FAIL) { BTC_TRACE_ERROR("%s failed\n", __func__); + btc_gatts_creat_tab_fail_and_cleanup(gatts_if, ¶m); return; } break; @@ -320,10 +330,12 @@ static void btc_gatts_act_create_attr_tab(esp_gatts_attr_db_t *gatts_attr_db, btc_gatts_cb_to_app(ESP_GATTS_CREAT_ATTR_TAB_EVT, gatts_if, ¶m); //reset the env after sent the data to app memset(&btc_creat_tab_env, 0, sizeof(esp_btc_creat_tab_t)); + future_free(future_p); return; } if (future_await(future_p) == FUTURE_FAIL) { BTC_TRACE_ERROR("%s failed\n", __func__); + btc_gatts_creat_tab_fail_and_cleanup(gatts_if, ¶m); return; } break; @@ -338,9 +350,24 @@ static void btc_gatts_act_create_attr_tab(esp_gatts_attr_db_t *gatts_attr_db, if (future_await(future_p) == FUTURE_FAIL) { BTC_TRACE_ERROR("%s failed\n", __func__); + btc_gatts_creat_tab_fail_and_cleanup(gatts_if, ¶m); return; } + } else { + /* svc_start_hdl == 0: service not created, report error and return */ + param.add_attr_tab.status = ESP_GATT_ERROR; + btc_gatts_cb_to_app(ESP_GATTS_CREAT_ATTR_TAB_EVT, gatts_if, ¶m); + memset(&btc_creat_tab_env, 0, sizeof(esp_btc_creat_tab_t)); + future_free(future_p); + return; } + } else { + /* incl_svc_desc == NULL: no BTA call, future never completed; report error and return */ + param.add_attr_tab.status = ESP_GATT_INVALID_PDU; + btc_gatts_cb_to_app(ESP_GATTS_CREAT_ATTR_TAB_EVT, gatts_if, ¶m); + memset(&btc_creat_tab_env, 0, sizeof(esp_btc_creat_tab_t)); + future_free(future_p); + return; } break; } @@ -357,6 +384,20 @@ static void btc_gatts_act_create_attr_tab(esp_gatts_attr_db_t *gatts_attr_db, svc_hal = btc_creat_tab_env.svc_start_hdl; if((gatts_attr_db[i].att_desc.value) == NULL){ BTC_TRACE_ERROR("%s Characteristic declaration should not be NULL\n", __func__); + param.add_attr_tab.status = ESP_GATT_INVALID_PDU; + btc_gatts_cb_to_app(ESP_GATTS_CREAT_ATTR_TAB_EVT, gatts_if, ¶m); + memset(&btc_creat_tab_env, 0, sizeof(esp_btc_creat_tab_t)); + future_free(future_p); + return; + } + else if (i + 1 >= max_nb_attr) { + BTC_TRACE_ERROR("%s Characteristic declaration at index %d missing value attribute\n", + __func__, i); + param.add_attr_tab.status = ESP_GATT_INVALID_PDU; + btc_gatts_cb_to_app(ESP_GATTS_CREAT_ATTR_TAB_EVT, gatts_if, ¶m); + memset(&btc_creat_tab_env, 0, sizeof(esp_btc_creat_tab_t)); + future_free(future_p); + return; } else{ char_property = (uint8_t)(*(uint8_t*)(gatts_attr_db[i].att_desc.value)); @@ -372,9 +413,17 @@ static void btc_gatts_act_create_attr_tab(esp_gatts_attr_db_t *gatts_attr_db, if (future_await(future_p) == FUTURE_FAIL) { BTC_TRACE_ERROR("%s failed\n", __func__); + btc_gatts_creat_tab_fail_and_cleanup(gatts_if, ¶m); return; } } + } else { + /* svc_start_hdl == 0: service not created, report error and return */ + param.add_attr_tab.status = ESP_GATT_ERROR; + btc_gatts_cb_to_app(ESP_GATTS_CREAT_ATTR_TAB_EVT, gatts_if, ¶m); + memset(&btc_creat_tab_env, 0, sizeof(esp_btc_creat_tab_t)); + future_free(future_p); + return; } break; @@ -413,8 +462,16 @@ static void btc_gatts_act_create_attr_tab(esp_gatts_attr_db_t *gatts_attr_db, if (future_await(future_p) == FUTURE_FAIL) { BTC_TRACE_ERROR("%s failed\n", __func__); + btc_gatts_creat_tab_fail_and_cleanup(gatts_if, ¶m); return; } + } else { + /* svc_start_hdl == 0: service not created, report error and return */ + param.add_attr_tab.status = ESP_GATT_ERROR; + btc_gatts_cb_to_app(ESP_GATTS_CREAT_ATTR_TAB_EVT, gatts_if, ¶m); + memset(&btc_creat_tab_env, 0, sizeof(esp_btc_creat_tab_t)); + future_free(future_p); + return; } break; } @@ -440,9 +497,9 @@ static void btc_gatts_act_create_attr_tab(esp_gatts_attr_db_t *gatts_attr_db, } static esp_gatt_status_t btc_gatts_check_valid_attr_tab(esp_gatts_attr_db_t *gatts_attr_db, - uint8_t max_nb_attr) + uint16_t max_nb_attr) { - uint8_t svc_num = 0; + uint16_t svc_num = 0; uint16_t uuid = 0; for(int i = 0; i < max_nb_attr; i++) { @@ -601,7 +658,12 @@ static void btc_gatts_inter_cb(tBTA_GATTS_EVT event, tBTA_GATTS *p_data) case BTA_GATTS_ADD_CHAR_EVT: { uint16_t index = btc_creat_tab_env.handle_idx; btc_creat_tab_env.handles[index] = p_data->add_result.attr_id - 1; - btc_creat_tab_env.handles[index+1] = p_data->add_result.attr_id; + if (index + 1 < btc_creat_tab_env.num_handle) { + btc_creat_tab_env.handles[index+1] = p_data->add_result.attr_id; + } else { + BTC_TRACE_ERROR("%s handles[%d+1] out of bounds (num_handle=%d)", + __func__, index, btc_creat_tab_env.num_handle); + } break; } case BTA_GATTS_ADD_CHAR_DESCR_EVT: { diff --git a/components/bt/host/bluedroid/btc/profile/std/iso/btc_iso_ble.c b/components/bt/host/bluedroid/btc/profile/std/iso/btc_iso_ble.c index a572010f7c..572d7cc24a 100644 --- a/components/bt/host/bluedroid/btc/profile/std/iso/btc_iso_ble.c +++ b/components/bt/host/bluedroid/btc/profile/std/iso/btc_iso_ble.c @@ -27,7 +27,7 @@ static inline void btc_iso_ble_cb_to_app(esp_ble_iso_cb_event_t event, esp_ble_i static void btc_ble_iso_callback(tBTM_BLE_ISO_EVENT event, tBTM_BLE_ISO_CB_PARAMS *params) { - esp_ble_iso_cb_param_t param; + esp_ble_iso_cb_param_t param = {0}; bt_status_t ret; btc_msg_t msg; msg.sig = BTC_SIG_API_CB;