From f4cec2ac4e2ffa9c74de7ac67d2dacc09945152d Mon Sep 17 00:00:00 2001 From: zhiweijian Date: Wed, 18 Mar 2026 16:33:02 +0800 Subject: [PATCH 1/9] fix(ble/bluedroid): Add null/range checks and fix resource handling in BTA layer - bta_dm_int: fix BTA_SERVICE_ID_TO_SERVICE_MASK undefined behavior (1<=32) - bta_gattc_main: add event bounds check before state table lookup - bta_gattc_utils: null checks for remote_bda/p_rcb, fix list_free in clcb_dealloc, bta_to_btif_uuid fixes - bta_gatts_act: fix formatting/indent in send_service_change_indication - bta_gatts_api: validate attr_val/len, add error logs on alloc failure - bta_sys_main: null/range checks in sm_execute, alarm/hash_map error handling in bta_alarm_cb --- .../bluedroid/bta/dm/include/bta_dm_int.h | 3 +- .../host/bluedroid/bta/gatt/bta_gattc_main.c | 6 ++- .../host/bluedroid/bta/gatt/bta_gattc_utils.c | 32 ++++++++++---- .../host/bluedroid/bta/gatt/bta_gatts_act.c | 2 +- .../host/bluedroid/bta/gatt/bta_gatts_api.c | 17 ++++++-- .../bt/host/bluedroid/bta/sys/bta_sys_main.c | 43 ++++++++++++++++--- 6 files changed, 83 insertions(+), 20 deletions(-) diff --git a/components/bt/host/bluedroid/bta/dm/include/bta_dm_int.h b/components/bt/host/bluedroid/bta/dm/include/bta_dm_int.h index 59fe63d0a5..858d2fb45d 100644 --- a/components/bt/host/bluedroid/bta/dm/include/bta_dm_int.h +++ b/components/bt/host/bluedroid/bta/dm/include/bta_dm_int.h @@ -45,7 +45,8 @@ #define BTA_DM_MSG_LEN 50 -#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) ((UINT32)(1ULL << (id))) /* DM events */ enum { diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gattc_main.c b/components/bt/host/bluedroid/bta/gatt/bta_gattc_main.c index 09e1aea2fa..bbf2b346bf 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_main.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_main.c @@ -108,7 +108,6 @@ const tBTA_GATTC_ACTION bta_gattc_action[] = { #define BTA_GATTC_ACTIONS 1 /* number of actions */ #define BTA_GATTC_NEXT_STATE 1 /* position of next state */ #define BTA_GATTC_NUM_COLS 2 /* number of columns in state tables */ - /* state table for idle state */ static const UINT8 bta_gattc_st_idle[][BTA_GATTC_NUM_COLS] = { /* Event Action 1 Next state */ @@ -298,6 +297,11 @@ BOOLEAN bta_gattc_sm_execute(tBTA_GATTC_CLCB *p_clcb, UINT16 event, tBTA_GATTC_D event &= 0x00FF; + if (event >= sizeof(bta_gattc_st_idle) / sizeof(bta_gattc_st_idle[0])) { + APPL_TRACE_ERROR("bta_gattc_sm_execute: invalid event index %d", event); + return TRUE; + } + /* set next state */ p_clcb->state = state_table[event][BTA_GATTC_NEXT_STATE]; diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gattc_utils.c b/components/bt/host/bluedroid/bta/gatt/bta_gattc_utils.c index bc329b775b..1786835e33 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_utils.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_utils.c @@ -125,8 +125,13 @@ tBTA_GATTC_CLCB *bta_gattc_find_clcb_by_cif (UINT8 client_if, BD_ADDR remote_bda tBTA_GATTC_CLCB *p_clcb = &bta_gattc_cb.clcb[0]; UINT8 i; + if (remote_bda == NULL) { + return NULL; + } + for (i = 0; i < BTA_GATTC_CLCB_MAX; i ++, p_clcb ++) { if (p_clcb->in_use && + p_clcb->p_rcb != NULL && p_clcb->p_rcb->client_if == client_if && p_clcb->transport == transport && bdcmp(p_clcb->bda, remote_bda) == 0) { @@ -244,16 +249,16 @@ void bta_gattc_clcb_dealloc(tBTA_GATTC_CLCB *p_clcb) if (p_clcb) { p_srcb = p_clcb->p_srcb; - if (p_srcb->num_clcb) { + if (p_srcb != NULL && p_srcb->num_clcb) { p_srcb->num_clcb --; } - if (p_clcb->p_rcb->num_clcb) { + if (p_clcb->p_rcb != NULL && p_clcb->p_rcb->num_clcb) { p_clcb->p_rcb->num_clcb --; } /* if the srcb is no longer needed, reset the state */ - if ( p_srcb->num_clcb == 0) { + if (p_srcb != NULL && p_srcb->num_clcb == 0) { p_srcb->connected = FALSE; p_srcb->state = BTA_GATTC_SERV_IDLE; p_srcb->mtu = 0; @@ -268,13 +273,13 @@ void bta_gattc_clcb_dealloc(tBTA_GATTC_CLCB *p_clcb) #endif } - if ( p_clcb->p_q_cmd != NULL && !list_contains(p_clcb->p_cmd_list, p_clcb->p_q_cmd)){ + if (p_clcb->p_q_cmd != NULL && + (p_clcb->p_cmd_list == NULL || !list_contains(p_clcb->p_cmd_list, p_clcb->p_q_cmd))) { osi_free(p_clcb->p_q_cmd); p_clcb->p_q_cmd = NULL; } // don't forget to clear the command queue before dealloc the clcb. - list_clear(p_clcb->p_cmd_list); - osi_free((void *)p_clcb->p_cmd_list); + list_free(p_clcb->p_cmd_list); p_clcb->p_cmd_list = NULL; //osi_free_and_reset((void **)&p_clcb->p_q_cmd); memset(p_clcb, 0, sizeof(tBTA_GATTC_CLCB)); @@ -970,6 +975,10 @@ tBTA_GATTC_CLCB *bta_gattc_find_int_conn_clcb(tBTA_GATTC_DATA *p_msg) { tBTA_GATTC_CLCB *p_clcb = NULL; + if (p_msg == NULL) { + return NULL; + } + if (p_msg->int_conn.role == HCI_ROLE_SLAVE) { bta_gattc_conn_find_alloc(p_msg->int_conn.remote_bda); } @@ -1005,6 +1014,10 @@ tBTA_GATTC_CLCB *bta_gattc_find_int_disconn_clcb(tBTA_GATTC_DATA *p_msg) { tBTA_GATTC_CLCB *p_clcb = NULL; + if (p_msg == NULL) { + return NULL; + } + bta_gattc_conn_dealloc(p_msg->int_conn.remote_bda); if ((p_clcb = bta_gattc_find_clcb_by_conn_id(p_msg->int_conn.hdr.layer_specific)) == NULL) { /* connection attempt failed, send connection callback event */ @@ -1033,6 +1046,8 @@ void bta_to_btif_uuid(bt_uuid_t *p_dest, tBT_UUID *p_src) switch (p_src->len) { case 0: + /* Invalid/empty UUID: zero p_dest so callers don't use garbage */ + memset(p_dest->uu, 0, sizeof(p_dest->uu)); break; case LEN_UUID_16: @@ -1041,8 +1056,8 @@ void bta_to_btif_uuid(bt_uuid_t *p_dest, tBT_UUID *p_src) break; case LEN_UUID_32: - p_dest->uu[12] = p_src->uu.uuid16 & 0xff; - p_dest->uu[13] = (p_src->uu.uuid16 >> 8) & 0xff; + p_dest->uu[12] = p_src->uu.uuid32 & 0xff; + p_dest->uu[13] = (p_src->uu.uuid32 >> 8) & 0xff; p_dest->uu[14] = (p_src->uu.uuid32 >> 16) & 0xff; p_dest->uu[15] = (p_src->uu.uuid32 >> 24) & 0xff; break; @@ -1054,6 +1069,7 @@ void bta_to_btif_uuid(bt_uuid_t *p_dest, tBT_UUID *p_src) default: APPL_TRACE_ERROR("%s: Unknown UUID length %d!", __FUNCTION__, p_src->len); + memset(p_dest->uu, 0, sizeof(p_dest->uu)); break; } } diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gatts_act.c b/components/bt/host/bluedroid/bta/gatt/bta_gatts_act.c index 04bcb697f8..9688b5d2bc 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gatts_act.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gatts_act.c @@ -872,7 +872,7 @@ void bta_gatts_send_service_change_indication (tBTA_GATTS_DATA *p_msg) tBTA_GATTS_RCB *p_rcb = bta_gatts_find_app_rcb_by_app_if(p_msg->api_send_service_change.server_if); tBTA_GATTS_SERVICE_CHANGE service_change; tBTA_GATT_STATUS status = BTA_GATT_OK; - UINT16 addr[BD_ADDR_LEN] = {0}; + UINT8 addr[BD_ADDR_LEN] = {0}; if(memcmp(p_msg->api_send_service_change.remote_bda, addr, BD_ADDR_LEN) != 0) { BD_ADDR bd_addr; memcpy(bd_addr, p_msg->api_send_service_change.remote_bda, BD_ADDR_LEN); diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gatts_api.c b/components/bt/host/bluedroid/bta/gatt/bta_gatts_api.c index 06667a415b..f1d7d62e99 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gatts_api.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gatts_api.c @@ -223,6 +223,7 @@ void BTA_GATTS_AddCharacteristic (UINT16 service_id, const tBT_UUID * p_char_u UINT16 len = 0; if(attr_val != NULL){ len = attr_val->attr_len; + APPL_TRACE_DEBUG("attr_val->attr_len = %x, attr_max_len = %x\n",attr_val->attr_len, attr_val->attr_max_len); } if ((p_buf = (tBTA_GATTS_API_ADD_CHAR *) osi_malloc(sizeof(tBTA_GATTS_API_ADD_CHAR))) != NULL) { memset(p_buf, 0, sizeof(tBTA_GATTS_API_ADD_CHAR)); @@ -234,15 +235,22 @@ void BTA_GATTS_AddCharacteristic (UINT16 service_id, const tBT_UUID * p_char_u if(control !=NULL){ p_buf->control.auto_rsp = control->auto_rsp; } - if(attr_val != NULL){ - APPL_TRACE_DEBUG("!!!!!!attr_val->attr_len = %x\n",attr_val->attr_len); - APPL_TRACE_DEBUG("!!!!!!!attr_val->attr_max_len = %x\n",attr_val->attr_max_len); + + if(attr_val != NULL && len){ p_buf->attr_val.attr_len = attr_val->attr_len; p_buf->attr_val.attr_max_len = attr_val->attr_max_len; p_buf->attr_val.attr_val = (uint8_t *)osi_malloc(len); if(p_buf->attr_val.attr_val != NULL){ memcpy(p_buf->attr_val.attr_val, attr_val->attr_val, len); + } else { + p_buf->attr_val.attr_len = 0; + p_buf->attr_val.attr_max_len = 0; + APPL_TRACE_ERROR("Allocate fail for %s\n", __func__); } + } else { + p_buf->attr_val.attr_len = 0; + p_buf->attr_val.attr_max_len = 0; + p_buf->attr_val.attr_val = NULL; } if (p_char_uuid) { @@ -499,6 +507,9 @@ void BTA_SetAttributeValue(UINT16 attr_handle, UINT16 length, UINT8 *value) if(value != NULL){ if((p_buf->value = (UINT8 *)osi_malloc(length)) != NULL){ memcpy(p_buf->value, value, length); + } else { + p_buf->length = 0; + APPL_TRACE_ERROR("Allocate fail for %s\n", __func__); } } 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 ea1bc5a854..3bf41d9dab 100644 --- a/components/bt/host/bluedroid/bta/sys/bta_sys_main.c +++ b/components/bt/host/bluedroid/bta/sys/bta_sys_main.c @@ -103,6 +103,7 @@ enum { #define BTA_SYS_ACTIONS 2 /* number of actions */ #define BTA_SYS_NEXT_STATE 2 /* position of next state */ #define BTA_SYS_NUM_COLS 3 /* number of columns in state tables */ +#define BTA_SYS_NUM_STATES 4 /* OFF, STARTING, ON, STOPPING */ /* state table for OFF state */ @@ -213,22 +214,39 @@ BOOLEAN bta_sys_sm_execute(BT_HDR *p_msg) BOOLEAN freebuf = TRUE; tBTA_SYS_ST_TBL state_table; UINT8 action; + UINT8 evt_idx; int i; + if (p_msg == NULL) { + return freebuf; + } + + evt_idx = p_msg->event & 0x00ff; + if (evt_idx >= BTA_SYS_NUM_ACTIONS) { + APPL_TRACE_ERROR("bta_sys_sm_execute: event 0x%x out of range (evt_idx=%u)", p_msg->event, evt_idx); + return freebuf; + } + if (bta_sys_cb.state >= BTA_SYS_NUM_STATES) { + APPL_TRACE_ERROR("bta_sys_sm_execute: invalid state %u", bta_sys_cb.state); + return freebuf; + } + APPL_TRACE_EVENT("bta_sys_sm_execute state:%d, event:0x%x\n", bta_sys_cb.state, p_msg->event); /* look up the state table for the current state */ state_table = bta_sys_st_tbl[bta_sys_cb.state]; /* update state */ - bta_sys_cb.state = state_table[p_msg->event & 0x00ff][BTA_SYS_NEXT_STATE]; + bta_sys_cb.state = state_table[evt_idx][BTA_SYS_NEXT_STATE]; /* execute action functions */ for (i = 0; i < BTA_SYS_ACTIONS; i++) { - if ((action = state_table[p_msg->event & 0x00ff][i]) != BTA_SYS_IGNORE) { - (*bta_sys_action[action])( (tBTA_SYS_HW_MSG *) p_msg); - } else { + action = state_table[evt_idx][i]; + if (action == BTA_SYS_IGNORE) { break; } + if (action < BTA_SYS_NUM_ACTIONS) { + (*bta_sys_action[action])( (tBTA_SYS_HW_MSG *) p_msg); + } } return freebuf; @@ -598,16 +616,29 @@ void bta_alarm_cb(void *data) void bta_sys_start_timer(TIMER_LIST_ENT *p_tle, UINT16 type, INT32 timeout_ms) { + osi_alarm_t *alarm = NULL; + assert(p_tle != NULL); // Get the alarm for this p_tle. osi_mutex_lock(&bta_alarm_lock, OSI_MUTEX_MAX_TIMEOUT); if (!hash_map_has_key(bta_alarm_hash_map, p_tle)) { - hash_map_set(bta_alarm_hash_map, p_tle, osi_alarm_new("bta_sys", bta_alarm_cb, p_tle, 0)); + alarm = osi_alarm_new("bta_sys", bta_alarm_cb, p_tle, 0); + if (alarm == NULL) { + APPL_TRACE_ERROR("%s unable to create new alarm.", __func__); + osi_mutex_unlock(&bta_alarm_lock); + return; + } + if (!hash_map_set(bta_alarm_hash_map, p_tle, alarm)) { + APPL_TRACE_ERROR("%s unable to set alarm in map.", __func__); + osi_alarm_free(alarm); + osi_mutex_unlock(&bta_alarm_lock); + return; + } } osi_mutex_unlock(&bta_alarm_lock); - osi_alarm_t *alarm = hash_map_get(bta_alarm_hash_map, p_tle); + alarm = hash_map_get(bta_alarm_hash_map, p_tle); if (alarm == NULL) { APPL_TRACE_ERROR("%s unable to create alarm.", __func__); return; From 6f5d9e3440abc84cfc960381254669d3aeb8c721 Mon Sep 17 00:00:00 2001 From: zhiweijian Date: Wed, 18 Mar 2026 16:33:11 +0800 Subject: [PATCH 2/9] 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 --- .../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 966bd41385..bbcb17b670 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 546c3c71fe..ddbc571b34 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 ec25805cff..6376a3ac1f 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; From b83647f5ea1034b943fd542622dc80363130418a Mon Sep 17 00:00:00 2001 From: zhiweijian Date: Wed, 18 Mar 2026 16:33:22 +0800 Subject: [PATCH 3/9] fix(ble/bluedroid): Align config, controller indent and init error paths - bt_target: remove/align obsolete macros with Kconfig - device/controller: fix start_up() Secure Connections indent, get_ble_resolving_list_max_size return type - controller.h: align type/interface declarations with implementation - bte_init: remove unused/redundant code - bte_main: return -1 on osi_init failure, null check in bte_main_hci_send --- .../bluedroid/common/include/common/bt_target.h | 4 ---- components/bt/host/bluedroid/device/controller.c | 14 +++++++------- .../bluedroid/device/include/device/controller.h | 2 +- components/bt/host/bluedroid/main/bte_init.c | 13 ------------- components/bt/host/bluedroid/main/bte_main.c | 10 +++++++++- 5 files changed, 17 insertions(+), 26 deletions(-) diff --git a/components/bt/host/bluedroid/common/include/common/bt_target.h b/components/bt/host/bluedroid/common/include/common/bt_target.h index ac08c379ca..2fb47513c9 100644 --- a/components/bt/host/bluedroid/common/include/common/bt_target.h +++ b/components/bt/host/bluedroid/common/include/common/bt_target.h @@ -681,10 +681,6 @@ #define BTA_DM_QOS_INCLUDED FALSE #endif -#ifndef BTA_PAN_INCLUDED -#define BTA_PAN_INCLUDED FALSE -#endif - #ifndef BTA_HD_INCLUDED #define BTA_HD_INCLUDED FALSE #endif diff --git a/components/bt/host/bluedroid/device/controller.c b/components/bt/host/bluedroid/device/controller.c index d471c8be9e..0aa677fc55 100644 --- a/components/bt/host/bluedroid/device/controller.c +++ b/components/bt/host/bluedroid/device/controller.c @@ -223,13 +223,13 @@ static void start_up(void) } #endif -if ((bluedroid_config_get()->get_sc_enabled())) { - controller_param.secure_connections_supported = HCI_SC_CTRLR_SUPPORTED(controller_param.features_classic[2].as_array); - if (controller_param.secure_connections_supported) { - response = AWAIT_COMMAND(controller_param.packet_factory->make_write_secure_connections_host_support(HCI_SC_MODE_ENABLED)); - controller_param.packet_parser->parse_generic_command_complete(response); + if ((bluedroid_config_get()->get_sc_enabled())) { + controller_param.secure_connections_supported = HCI_SC_CTRLR_SUPPORTED(controller_param.features_classic[2].as_array); + if (controller_param.secure_connections_supported) { + response = AWAIT_COMMAND(controller_param.packet_factory->make_write_secure_connections_host_support(HCI_SC_MODE_ENABLED)); + controller_param.packet_parser->parse_generic_command_complete(response); + } } -} #if (BLE_INCLUDED == TRUE) #if (CLASSIC_BT_INCLUDED) @@ -546,7 +546,7 @@ static uint8_t get_ble_resolving_list_max_size(void) return controller_param.ble_resolving_list_max_size; } -static void set_ble_resolving_list_max_size(int resolving_list_max_size) +static void set_ble_resolving_list_max_size(uint8_t resolving_list_max_size) { assert(controller_param.readable); assert(controller_param.ble_supported); diff --git a/components/bt/host/bluedroid/device/include/device/controller.h b/components/bt/host/bluedroid/device/include/device/controller.h index 6345d7a9c9..610c9afafc 100644 --- a/components/bt/host/bluedroid/device/include/device/controller.h +++ b/components/bt/host/bluedroid/device/include/device/controller.h @@ -78,7 +78,7 @@ typedef struct controller_t { uint8_t (*get_ble_white_list_size)(void); uint8_t (*get_ble_resolving_list_max_size)(void); - void (*set_ble_resolving_list_max_size)(int resolving_list_max_size); + void (*set_ble_resolving_list_max_size)(uint8_t resolving_list_max_size); #if (BLE_50_FEATURE_SUPPORT == TRUE) #if (BLE_50_EXTEND_ADV_EN == TRUE) diff --git a/components/bt/host/bluedroid/main/bte_init.c b/components/bt/host/bluedroid/main/bte_init.c index c19b50b6aa..b688d7a27d 100644 --- a/components/bt/host/bluedroid/main/bte_init.c +++ b/components/bt/host/bluedroid/main/bte_init.c @@ -152,10 +152,6 @@ #include "bta_gatts_int.h" #endif -#if BTA_PAN_INCLUDED==TRUE -#include "bta_pan_int.h" -#endif - #if BTA_PBA_CLIENT_INCLUDED == TRUE #include "bta_pba_client_int.h" #endif @@ -481,9 +477,6 @@ bt_status_t BTE_InitStack(void) } memset((void *)bta_jv_cb_ptr, 0, sizeof(tBTA_JV_CB)); #endif //JV -#if BTA_HS_INCLUDED == TRUE - memset((void *)bta_hs_cb_ptr, 0, sizeof(tBTA_HS_CB)); -#endif #if BTA_SDP_INCLUDED == TRUE if ((bta_sdp_cb_ptr = (tBTA_SDP_CB *)osi_malloc(sizeof(tBTA_SDP_CB))) == NULL) { goto error_exit; @@ -525,9 +518,6 @@ bt_status_t BTE_InitStack(void) } memset((void *)bta_hd_cb_ptr, 0, sizeof(tBTA_HD_CB)); #endif -#if BTA_HL_INCLUDED==TRUE - memset((void *)bta_hl_cb_ptr, 0, sizeof(tBTA_HL_CB)); -#endif #if GATTC_INCLUDED==TRUE if ((bta_gattc_cb_ptr = (tBTA_GATTC_CB *)osi_malloc(sizeof(tBTA_GATTC_CB))) == NULL) { goto error_exit; @@ -540,9 +530,6 @@ bt_status_t BTE_InitStack(void) } memset((void *)bta_gatts_cb_ptr, 0, sizeof(tBTA_GATTS_CB)); #endif -#if BTA_PAN_INCLUDED==TRUE - memset((void *)bta_pan_cb_ptr, 0, sizeof(tBTA_PAN_CB)); -#endif #if BTA_PBA_CLIENT_INCLUDED == TRUE if ((bta_pba_client_cb_ptr = (tBTA_PBA_CLIENT_CB *)osi_malloc(sizeof(tBTA_PBA_CLIENT_CB))) == NULL) { goto error_exit; diff --git a/components/bt/host/bluedroid/main/bte_main.c b/components/bt/host/bluedroid/main/bte_main.c index 73cbafc644..d50544ced4 100644 --- a/components/bt/host/bluedroid/main/bte_main.c +++ b/components/bt/host/bluedroid/main/bte_main.c @@ -86,7 +86,10 @@ int bte_main_boot_entry(bluedroid_init_done_cb_t cb) bluedroid_init_done_cb = cb; - osi_init(); + if (osi_init() != 0) { + APPL_TRACE_ERROR("%s failed to initialize OS layer.\n", __func__); + return -1; + } //Enable HCI bte_main_enable(); @@ -235,6 +238,11 @@ void bte_main_lpm_wake_bt_device(void) ******************************************************************************/ void bte_main_hci_send (BT_HDR *p_msg, UINT16 event) { + if (!p_msg) { + APPL_TRACE_ERROR("%s null message\n", __func__); + return; + } + UINT16 sub_event = event & BT_SUB_EVT_MASK; /* local controller ID */ p_msg->event = event; From 778dd2ab5eef551454e63b5659f10480cb7f8944 Mon Sep 17 00:00:00 2001 From: zhiweijian Date: Wed, 18 Mar 2026 16:33:34 +0800 Subject: [PATCH 4/9] fix(ble/bluedroid): Add length/pointer checks and fix error paths - hci_hal_h4: validate packet length and pointers in hci_packet_complete, hdl_rx_adv_rpt, callbacks - hci_layer: align hci_start_up error path and return; validate packet len in filter_incoming_event - hci_packet_factory: ensure BT_HDR length/offset initialized in make_command_no_params - packet_fragmenter: validate length before fragment_and_dispatch --- components/bt/host/bluedroid/hci/hci_hal_h4.c | 17 +++++++++++------ components/bt/host/bluedroid/hci/hci_layer.c | 18 +++++++++++------- .../bt/host/bluedroid/hci/hci_packet_factory.c | 2 ++ .../bt/host/bluedroid/hci/packet_fragmenter.c | 3 ++- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/components/bt/host/bluedroid/hci/hci_hal_h4.c b/components/bt/host/bluedroid/hci/hci_hal_h4.c index ae51b55dcd..a1f12bb9bf 100644 --- a/components/bt/host/bluedroid/hci/hci_hal_h4.c +++ b/components/bt/host/bluedroid/hci/hci_hal_h4.c @@ -34,7 +34,6 @@ #include "esp_bt.h" #endif #include "esp_bluedroid_hci.h" -#include "stack/hcimsgs.h" #if (C2H_FLOW_CONTROL_INCLUDED == TRUE) #include "l2c_int.h" @@ -319,9 +318,12 @@ static void hci_packet_complete(BT_HDR *packet){ } #endif ///C2H_FLOW_CONTROL_INCLUDED == TRUE -bool host_recv_adv_packet(uint8_t *packet) +bool host_recv_adv_packet(uint8_t *packet, uint16_t len) { assert(packet); + if (len < 4) { + return false; + } if(packet[0] == DATA_TYPE_EVENT && packet[1] == HCI_BLE_EVENT) { if(packet[3] == HCI_BLE_ADV_PKT_RPT_EVT || packet[3] == HCI_BLE_DIRECT_ADV_EVT #if (BLE_ADV_REPORT_FLOW_CONTROL == TRUE) @@ -543,7 +545,7 @@ static void hci_hal_h4_hdl_rx_adv_rpt(pkt_linked_item_t *linked_pkt) BT_HDR* packet = (BT_HDR *)linked_pkt->data; stream = packet->data + packet->offset; - assert(host_recv_adv_packet(stream) == true); + assert(host_recv_adv_packet(stream, packet->len) == true); STREAM_TO_UINT8(type, stream); packet->offset++; @@ -588,7 +590,10 @@ static void host_send_pkt_available_cb(void) void bt_record_hci_data(uint8_t *data, uint16_t len) { #if (BT_HCI_LOG_INCLUDED == TRUE) - if ((data[0] == DATA_TYPE_EVENT) && (data[1] == HCI_BLE_EVENT) && ((data[3] == HCI_BLE_ADV_PKT_RPT_EVT) || (data[3] == HCI_BLE_DIRECT_ADV_EVT) + if (len < 2) { + return; + } + if ((len >= 4) && (data[0] == DATA_TYPE_EVENT) && (data[1] == HCI_BLE_EVENT) && ((data[3] == HCI_BLE_ADV_PKT_RPT_EVT) || (data[3] == HCI_BLE_DIRECT_ADV_EVT) #if (BLE_ADV_REPORT_FLOW_CONTROL == TRUE) || (data[3] == HCI_BLE_ADV_DISCARD_REPORT_EVT) #endif // (BLE_ADV_REPORT_FLOW_CONTROL == TRUE) @@ -641,7 +646,7 @@ static int host_recv_pkt_cb(uint8_t *data, uint16_t len) } #endif // #if (BLE_FEAT_ISO_EN == TRUE) - bool is_adv_rpt = host_recv_adv_packet(data); + bool is_adv_rpt = host_recv_adv_packet(data, len); if (!is_adv_rpt) { pkt_size = BT_HDR_SIZE + len; @@ -700,7 +705,7 @@ static int host_recv_pkt_cb(uint8_t *data, uint16_t len) hci_upstream_data_post(OSI_THREAD_MAX_TIMEOUT); - BTTRC_DUMP_BUFFER("Recv Pkt", pkt->data, len); + BTTRC_DUMP_BUFFER("Recv Pkt", data, len); return 0; } diff --git a/components/bt/host/bluedroid/hci/hci_layer.c b/components/bt/host/bluedroid/hci/hci_layer.c index fbc0dab461..ac56596667 100644 --- a/components/bt/host/bluedroid/hci/hci_layer.c +++ b/components/bt/host/bluedroid/hci/hci_layer.c @@ -117,7 +117,7 @@ int hci_start_up(void) hci_host_thread = osi_thread_create(HCI_HOST_TASK_NAME, HCI_HOST_TASK_STACK_SIZE, HCI_HOST_TASK_PRIO, HCI_HOST_TASK_PINNED_TO_CORE, HCI_HOST_TASK_WORKQUEUE_NUM, workqueue_len); if (hci_host_thread == NULL) { - return -2; + goto error; } osi_event_bind(hci_host_env.downstream_data_ready, hci_host_thread, HCI_DOWNSTREAM_DATA_QUEUE_IDX); @@ -137,13 +137,15 @@ error: void hci_shut_down(void) { hci_host_startup_flag = false; + + /* Close HAL and cleanup before freeing thread: hal->open() and osi_event_bind() + * stored references to hci_host_thread; they must not use it after osi_thread_free(). */ + if (hci_host_thread != NULL) { + hal->close(); + packet_fragmenter->cleanup(); + } hci_layer_deinit_env(); - packet_fragmenter->cleanup(); - - //low_power_manager->cleanup(); - hal->close(); - osi_thread_free(hci_host_thread); hci_host_thread = NULL; } @@ -445,7 +447,7 @@ static bool filter_incoming_event(BT_HDR *packet) { pkt_linked_item_t *wait_entry = NULL; hci_cmd_metadata_t *metadata = NULL; - uint8_t *stream = packet->data + packet->offset; + uint8_t *stream; uint8_t event_code; command_opcode_t opcode; @@ -453,6 +455,8 @@ static bool filter_incoming_event(BT_HDR *packet) return true; } + stream = packet->data + packet->offset; + if (packet->len < HCI_EVENT_PREAMBLE_SIZE) { HCI_TRACE_WARNING("dropping too short HCI event (len=%u)", packet->len); osi_free(packet); diff --git a/components/bt/host/bluedroid/hci/hci_packet_factory.c b/components/bt/host/bluedroid/hci/hci_packet_factory.c index 6a84f9ca29..f710046d0e 100644 --- a/components/bt/host/bluedroid/hci/hci_packet_factory.c +++ b/components/bt/host/bluedroid/hci/hci_packet_factory.c @@ -250,6 +250,8 @@ static BT_HDR *make_command_no_params(uint16_t opcode) static BT_HDR *make_command(uint16_t opcode, size_t parameter_size, uint8_t **stream_out) { BT_HDR *packet = HCI_GET_CMD_BUF(parameter_size); + assert(packet != NULL); + hci_cmd_metadata_t *metadata = HCI_GET_CMD_METAMSG(packet); metadata->opcode = opcode; diff --git a/components/bt/host/bluedroid/hci/packet_fragmenter.c b/components/bt/host/bluedroid/hci/packet_fragmenter.c index 0827ecb4fe..e280a98183 100644 --- a/components/bt/host/bluedroid/hci/packet_fragmenter.c +++ b/components/bt/host/bluedroid/hci/packet_fragmenter.c @@ -73,9 +73,10 @@ static void fragment_and_dispatch(BT_HDR *packet) uint16_t continuation_handle; uint16_t max_data_size, max_packet_size, remaining_length; uint16_t event = packet->event & MSG_EVT_MASK; - uint8_t *stream = packet->data + packet->offset; + uint8_t *stream; assert(packet != NULL); + stream = packet->data + packet->offset; // We only fragment ACL packets if (event != MSG_STACK_TO_HC_HCI_ACL) { From 65b2cb27280e5b9a0ed908505a01e24060688277 Mon Sep 17 00:00:00 2001 From: zhiweijian Date: Wed, 18 Mar 2026 16:33:45 +0800 Subject: [PATCH 5/9] fix(ble/bluedroid): BLE GAP/ACL/ISO/SCO null checks, evt_len and resource handling - btm_acl: malloc/list_append failure handling, remove/memset order in btm_acl_removed - btm_ble: remove incorrect sec_flags in SMP_OOB/NC/SC_OOB fall-through - btm_ble_5_gap: btm_ble_hci_status_to_str unreachable return, BTM_BleSetExtendedAdvParams/BleStartExtAdv leak and bounds - btm_ble_addr: fix indent in btm_find_dev_by_identity_addr - btm_ble_gap: null check p_service_data, pass evt_len to btm_ble_process_adv_pkt, bounds in process_adv_pkt - btm_ble_iso: align param types with declaration - btm_ble_privacy: handle BTM_BLE_IRK_LIST_INVALID_INDEX in update_resolving_list, comment fixes - btm_devctl: fix btm_vsc_complete param order/type - btm_sco: add evt_len to btm_sco_process_num_completed_pkts for bounds check - btm_ble_int.h/btm_int.h: add evt_len to process_adv_pkt and process_num_completed_pkts declarations --- .../bt/host/bluedroid/stack/btm/btm_acl.c | 185 +++++++++--------- .../bt/host/bluedroid/stack/btm/btm_ble.c | 4 +- .../host/bluedroid/stack/btm/btm_ble_5_gap.c | 22 ++- .../host/bluedroid/stack/btm/btm_ble_addr.c | 4 +- .../bt/host/bluedroid/stack/btm/btm_ble_gap.c | 37 ++-- .../bt/host/bluedroid/stack/btm/btm_ble_iso.c | 2 +- .../bluedroid/stack/btm/btm_ble_privacy.c | 13 +- .../bt/host/bluedroid/stack/btm/btm_devctl.c | 2 +- .../bt/host/bluedroid/stack/btm/btm_sco.c | 15 +- .../bluedroid/stack/btm/include/btm_ble_int.h | 2 +- .../bluedroid/stack/btm/include/btm_int.h | 2 +- 11 files changed, 166 insertions(+), 122 deletions(-) diff --git a/components/bt/host/bluedroid/stack/btm/btm_acl.c b/components/bt/host/bluedroid/stack/btm/btm_acl.c index 6187d3cb8f..6d53ba49b0 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_acl.c +++ b/components/bt/host/bluedroid/stack/btm/btm_acl.c @@ -362,128 +362,131 @@ void btm_acl_created (BD_ADDR bda, DEV_CLASS dc, UINT8 bdn[BTM_MAX_REM_BD_NAME_L } else { p = (tACL_CONN *)osi_malloc(sizeof(tACL_CONN)); - if (p && !list_append(btm_cb.p_acl_db_list, p)) { + if (p == NULL) { + BTM_TRACE_ERROR("btm_acl_created: osi_malloc failed for tACL_CONN\n"); + return; + } + if (!list_append(btm_cb.p_acl_db_list, p)) { + BTM_TRACE_ERROR("btm_acl_created: list_append failed\n"); osi_free(p); return; } - if (p) { - memset(p, 0, sizeof(tACL_CONN)); - p->in_use = TRUE; - p->hci_handle = hci_handle; - p->link_role = link_role; - p->link_up_issued = FALSE; - memcpy (p->remote_addr, bda, BD_ADDR_LEN); - /* Set the default version of the peer device to version4.0 before exchange the version with it. - If the peer device act as a master and don't exchange the version with us, then it can only use the - legacy connect instead of secure connection in the pairing step. */ - p->lmp_version = HCI_PROTO_VERSION_4_0; + memset(p, 0, sizeof(tACL_CONN)); + p->in_use = TRUE; + p->hci_handle = hci_handle; + p->link_role = link_role; + p->link_up_issued = FALSE; + memcpy (p->remote_addr, bda, BD_ADDR_LEN); + /* Set the default version of the peer device to version4.0 before exchange the version with it. + If the peer device act as a master and don't exchange the version with us, then it can only use the + legacy connect instead of secure connection in the pairing step. */ + p->lmp_version = HCI_PROTO_VERSION_4_0; #if BLE_INCLUDED == TRUE - p->transport = transport; + p->transport = transport; #if BLE_PRIVACY_SPT == TRUE - if (transport == BT_TRANSPORT_LE) { - btm_ble_refresh_local_resolvable_private_addr(bda, - btm_cb.ble_ctr_cb.addr_mgnt_cb.private_addr); - } + if (transport == BT_TRANSPORT_LE) { + btm_ble_refresh_local_resolvable_private_addr(bda, + btm_cb.ble_ctr_cb.addr_mgnt_cb.private_addr); + } #else - p->conn_addr_type = BLE_ADDR_PUBLIC; - memcpy(p->conn_addr, &controller_get_interface()->get_address()->address, BD_ADDR_LEN); - BTM_TRACE_DEBUG ("conn_addr: RemBdAddr: %02x%02x%02x%02x%02x%02x\n", - p->conn_addr[0], p->conn_addr[1], p->conn_addr[2], p->conn_addr[3], p->conn_addr[4], p->conn_addr[5]); + p->conn_addr_type = BLE_ADDR_PUBLIC; + memcpy(p->conn_addr, &controller_get_interface()->get_address()->address, BD_ADDR_LEN); + BTM_TRACE_DEBUG ("conn_addr: RemBdAddr: %02x%02x%02x%02x%02x%02x\n", + p->conn_addr[0], p->conn_addr[1], p->conn_addr[2], p->conn_addr[3], p->conn_addr[4], p->conn_addr[5]); #endif #endif #if (CLASSIC_BT_INCLUDED == TRUE) - p->switch_role_state = BTM_ACL_SWKEY_STATE_IDLE; + p->switch_role_state = BTM_ACL_SWKEY_STATE_IDLE; - p->p_pm_mode_db = btm_pm_sm_alloc(); + p->p_pm_mode_db = btm_pm_sm_alloc(); #if BTM_PM_DEBUG == TRUE - BTM_TRACE_DEBUG( "btm_pm_sm_alloc handle:%d st:%d", hci_handle, p->p_pm_mode_db->state); + BTM_TRACE_DEBUG( "btm_pm_sm_alloc handle:%d st:%d", hci_handle, p->p_pm_mode_db->state); #endif // BTM_PM_DEBUG #endif // (CLASSIC_BT_INCLUDED == TRUE) #if (CLASSIC_BT_INCLUDED == TRUE) - btm_sec_update_legacy_auth_state(p, BTM_ACL_LEGACY_AUTH_NONE); + btm_sec_update_legacy_auth_state(p, BTM_ACL_LEGACY_AUTH_NONE); #endif - if (dc) { - memcpy (p->remote_dc, dc, DEV_CLASS_LEN); - } + if (dc) { + memcpy (p->remote_dc, dc, DEV_CLASS_LEN); + } - if (bdn) { - memcpy (p->remote_name, bdn, BTM_MAX_REM_BD_NAME_LEN); - } + if (bdn) { + memcpy (p->remote_name, bdn, BTM_MAX_REM_BD_NAME_LEN); + } - /* if BR/EDR do something more */ - if (transport == BT_TRANSPORT_BR_EDR) { - btsnd_hcic_read_rmt_clk_offset (p->hci_handle); - btsnd_hcic_rmt_ver_req (p->hci_handle); - } - p_dev_rec = btm_find_dev_by_handle (hci_handle); + /* if BR/EDR do something more */ + if (transport == BT_TRANSPORT_BR_EDR) { + btsnd_hcic_read_rmt_clk_offset (p->hci_handle); + btsnd_hcic_rmt_ver_req (p->hci_handle); + } + p_dev_rec = btm_find_dev_by_handle (hci_handle); #if (BLE_INCLUDED == TRUE) - if (p_dev_rec ) { - BTM_TRACE_DEBUG ("device_type=0x%x\n", p_dev_rec->device_type); - } + if (p_dev_rec ) { + BTM_TRACE_DEBUG ("device_type=0x%x\n", p_dev_rec->device_type); + } #endif - if (p_dev_rec && !(transport == BT_TRANSPORT_LE)) { - if (!p_dev_rec->remote_secure_connection_previous_state) { - /* If remote features already known, copy them and continue connection setup */ - if ((p_dev_rec->num_read_pages) && - (p_dev_rec->num_read_pages <= (HCI_EXT_FEATURES_PAGE_MAX + 1))) { - memcpy (p->peer_lmp_features, p_dev_rec->features, - (HCI_FEATURE_BYTES_PER_PAGE * p_dev_rec->num_read_pages)); - p->num_read_pages = p_dev_rec->num_read_pages; + if (p_dev_rec && !(transport == BT_TRANSPORT_LE)) { + if (!p_dev_rec->remote_secure_connection_previous_state) { + /* If remote features already known, copy them and continue connection setup */ + if ((p_dev_rec->num_read_pages) && + (p_dev_rec->num_read_pages <= (HCI_EXT_FEATURES_PAGE_MAX + 1))) { + memcpy (p->peer_lmp_features, p_dev_rec->features, + (HCI_FEATURE_BYTES_PER_PAGE * p_dev_rec->num_read_pages)); + p->num_read_pages = p_dev_rec->num_read_pages; #if (CLASSIC_BT_INCLUDED == TRUE) - const UINT8 req_pend = (p_dev_rec->sm4 & BTM_SM4_REQ_PEND); + const UINT8 req_pend = (p_dev_rec->sm4 & BTM_SM4_REQ_PEND); #endif ///CLASSIC_BT_INCLUDED == TRUE - /* Store the Peer Security Capabilities (in SM4 and rmt_sec_caps) */ + /* Store the Peer Security Capabilities (in SM4 and rmt_sec_caps) */ #if (SMP_INCLUDED == TRUE) - btm_sec_set_peer_sec_caps(p, p_dev_rec); + btm_sec_set_peer_sec_caps(p, p_dev_rec); #endif ///SMP_INCLUDED == TRUE #if (CLASSIC_BT_INCLUDED == TRUE) - BTM_TRACE_API("%s: pend:%d\n", __FUNCTION__, req_pend); - if (req_pend) { - /* Request for remaining Security Features (if any) */ - l2cu_resubmit_pending_sec_req (p_dev_rec->bd_addr); - } -#endif ///CLASSIC_BT_INCLUDED == TRUE - btm_establish_continue (p); - return; + BTM_TRACE_API("%s: pend:%d\n", __FUNCTION__, req_pend); + if (req_pend) { + /* Request for remaining Security Features (if any) */ + l2cu_resubmit_pending_sec_req (p_dev_rec->bd_addr); } - } else { - /* If remote features indicated secure connection (SC) mode, check the remote features again*/ - /* this is to prevent from BIAS attack where attacker can downgrade SC mode*/ - btm_read_remote_features (p->hci_handle); +#endif ///CLASSIC_BT_INCLUDED == TRUE + btm_establish_continue (p); + return; } - } - -#if (BLE_INCLUDED == TRUE) - /* If here, features are not known yet */ - if (p_dev_rec && transport == BT_TRANSPORT_LE) { -#if BLE_PRIVACY_SPT == TRUE - btm_ble_get_acl_remote_addr (p_dev_rec, p->active_remote_addr, - &p->active_remote_addr_type); -#endif - - if (link_role == HCI_ROLE_MASTER) { - btsnd_hcic_ble_read_remote_feat(p->hci_handle); - } else if (HCI_LE_SLAVE_INIT_FEAT_EXC_SUPPORTED(controller_get_interface()->get_features_ble()->as_array) - && link_role == HCI_ROLE_SLAVE) { - btsnd_hcic_rmt_ver_req (p->hci_handle); - } else { - btm_establish_continue(p); - } - } else -#endif - { + } else { + /* If remote features indicated secure connection (SC) mode, check the remote features again*/ + /* this is to prevent from BIAS attack where attacker can downgrade SC mode*/ btm_read_remote_features (p->hci_handle); } - - /* read page 1 - on rmt feature event for buffer reasons */ - return; } + +#if (BLE_INCLUDED == TRUE) + /* If here, features are not known yet */ + if (p_dev_rec && transport == BT_TRANSPORT_LE) { +#if BLE_PRIVACY_SPT == TRUE + btm_ble_get_acl_remote_addr (p_dev_rec, p->active_remote_addr, + &p->active_remote_addr_type); +#endif + + if (link_role == HCI_ROLE_MASTER) { + btsnd_hcic_ble_read_remote_feat(p->hci_handle); + } else if (HCI_LE_SLAVE_INIT_FEAT_EXC_SUPPORTED(controller_get_interface()->get_features_ble()->as_array) + && link_role == HCI_ROLE_SLAVE) { + btsnd_hcic_rmt_ver_req (p->hci_handle); + } else { + btm_establish_continue(p); + } + } else +#endif + { + btm_read_remote_features (p->hci_handle); + } + + /* read page 1 - on rmt feature event for buffer reasons */ + return; } } @@ -593,11 +596,9 @@ void btm_acl_removed (BD_ADDR bda, tBT_TRANSPORT transport) #if (CLASSIC_BT_INCLUDED == TRUE) list_remove(btm_cb.p_pm_mode_db_list, p->p_pm_mode_db); #endif // #if (CLASSIC_BT_INCLUDED == TRUE) - /* Clear the ACL connection data */ - memset(p, 0, sizeof(tACL_CONN)); - if (list_remove(btm_cb.p_acl_db_list, p)) { - p = NULL; - } + /* Remove and free the ACL connection data */ + list_remove(btm_cb.p_acl_db_list, p); + p = NULL; } } diff --git a/components/bt/host/bluedroid/stack/btm/btm_ble.c b/components/bt/host/bluedroid/stack/btm/btm_ble.c index 7ea143e756..5a35fa0e9b 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_ble.c +++ b/components/bt/host/bluedroid/stack/btm/btm_ble.c @@ -2173,9 +2173,7 @@ UINT8 btm_proc_smp_cback(tSMP_EVT event, BD_ADDR bd_addr, tSMP_EVT_DATA *p_data) case SMP_OOB_REQ_EVT: case SMP_NC_REQ_EVT: case SMP_SC_OOB_REQ_EVT: - /* fall through */ - p_dev_rec->sec_flags |= BTM_SEC_LE_AUTHENTICATED; - + /* fall through */ case SMP_SEC_REQUEST_EVT: if (event == SMP_SEC_REQUEST_EVT && btm_cb.pairing_state != BTM_PAIR_STATE_IDLE) { BTM_TRACE_DEBUG("%s: Ignoring SMP Security request", __func__); diff --git a/components/bt/host/bluedroid/stack/btm/btm_ble_5_gap.c b/components/bt/host/bluedroid/stack/btm/btm_ble_5_gap.c index 3ecfe99c69..089f7758e1 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_ble_5_gap.c +++ b/components/bt/host/bluedroid/stack/btm/btm_ble_5_gap.c @@ -203,8 +203,6 @@ static const char *btm_ble_hci_status_to_str(tHCI_STATUS status) default: return "Invalid HCI status code."; } - - return NULL; } #endif /* !UC_BT_STACK_NO_LOG */ @@ -384,6 +382,7 @@ tBTM_STATUS BTM_BleSetExtendedAdvParams(UINT8 instance, tBTM_BLE_GAP_EXT_ADV_PAR if ((status = btm_ble_ext_adv_params_validate(params)) != BTM_SUCCESS) { BTM_TRACE_ERROR("%s, invalid extend adv params.", __func__); + goto end; } if (params->type & BTM_BLE_GAP_SET_EXT_ADV_PROP_CONNECTABLE) { @@ -567,10 +566,6 @@ tBTM_STATUS BTM_BleStartExtAdv(BOOLEAN enable, UINT8 num, tBTM_BLE_EXT_ADV *ext_ BTM_TRACE_ERROR("LE EA En=%d: cmd err=0x%x", enable, err); status = BTM_HCI_ERROR | err; } - - osi_free(instance); - osi_free(duration); - osi_free(max_events); } else { // enable = false, num == 0 or ext_adv = NULL @@ -584,6 +579,15 @@ tBTM_STATUS BTM_BleStartExtAdv(BOOLEAN enable, UINT8 num, tBTM_BLE_EXT_ADV *ext_ end: + if (instance) { + osi_free(instance); + } + if (duration) { + osi_free(duration); + } + if (max_events) { + osi_free(max_events); + } if (!enable && status == BTM_SUCCESS) { // disable all ext adv @@ -602,6 +606,9 @@ end: for (uint8_t i = 0; i < num; i++) { uint8_t index = ext_adv[i].instance; + if (index >= MAX_BLE_ADV_INSTANCE) { + continue; + } adv_record[index].invalid = false; adv_record[index].enabled = false; adv_record[index].instance = INVALID_VALUE_8BIT; @@ -616,6 +623,9 @@ end: for (uint8_t i = 0; i < num; i++) { uint8_t index = ext_adv[i].instance; + if (index >= MAX_BLE_ADV_INSTANCE) { + continue; + } adv_record[index].invalid = true; adv_record[index].enabled = true; adv_record[index].instance = ext_adv[i].instance; diff --git a/components/bt/host/bluedroid/stack/btm/btm_ble_addr.c b/components/bt/host/bluedroid/stack/btm/btm_ble_addr.c index 7d8c3b0b93..15bb8b4daa 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_ble_addr.c +++ b/components/bt/host/bluedroid/stack/btm/btm_ble_addr.c @@ -441,12 +441,12 @@ tBTM_SEC_DEV_REC *btm_find_dev_by_identity_addr(BD_ADDR bd_addr, UINT8 addr_type context.free_check = FALSE; p_node = list_foreach(btm_cb.p_sec_dev_rec_list, btm_find_sec_dev_in_list, &context); if (p_node) { - p_dev_rec = list_node(p_node); + p_dev_rec = list_node(p_node); if ((p_dev_rec->ble.static_addr_type & (~BLE_ADDR_TYPE_ID_BIT)) != (addr_type & (~BLE_ADDR_TYPE_ID_BIT))) { BTM_TRACE_WARNING("%s find pseudo->random match with diff addr type: %d vs %d", __func__, p_dev_rec->ble.static_addr_type, addr_type); - } + } } return p_dev_rec; #endif diff --git a/components/bt/host/bluedroid/stack/btm/btm_ble_gap.c b/components/bt/host/bluedroid/stack/btm/btm_ble_gap.c index 90461db35c..51973d310e 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_ble_gap.c +++ b/components/bt/host/bluedroid/stack/btm/btm_ble_gap.c @@ -1979,7 +1979,7 @@ UINT8 *btm_ble_build_adv_data(tBTM_BLE_AD_MASK *p_data_mask, UINT8 **p_dst, } /* 16bits/32bits/128bits Service Data */ if (len > MIN_ADV_LENGTH && data_mask & BTM_BLE_AD_BIT_SERVICE_DATA && - p_data && p_data->p_service_data->len != 0 && p_data->p_service_data->p_val) { + p_data && p_data->p_service_data && p_data->p_service_data->len != 0 && p_data->p_service_data->p_val) { if (len > (p_data->p_service_data->service_uuid.len + MIN_ADV_LENGTH)) { if (p_data->p_service_data->len > (len - MIN_ADV_LENGTH)) { cp_len = len - MIN_ADV_LENGTH - p_data->p_service_data->service_uuid.len; @@ -3063,7 +3063,7 @@ static void btm_adv_pkt_handler(void *arg) STREAM_TO_UINT8 (hci_evt_len, p); STREAM_TO_UINT8 (ble_sub_code, p); if (ble_sub_code == HCI_BLE_ADV_PKT_RPT_EVT) { - btm_ble_process_adv_pkt(p); + btm_ble_process_adv_pkt(p, hci_evt_len); } else if (ble_sub_code == HCI_BLE_ADV_DISCARD_REPORT_EVT) { btm_ble_process_adv_discard_evt(p); } else if (ble_sub_code == HCI_BLE_DIRECT_ADV_EVT) { @@ -3083,7 +3083,6 @@ static void btm_adv_pkt_handler(void *arg) } UNUSED(hci_evt_code); - UNUSED(hci_evt_len); } /******************************************************************************* @@ -3099,7 +3098,7 @@ static void btm_adv_pkt_handler(void *arg) ** Returns void ** *******************************************************************************/ -void btm_ble_process_adv_pkt (UINT8 *p_data) +void btm_ble_process_adv_pkt (UINT8 *p_data, UINT8 evt_len) { BD_ADDR bda; UINT8 evt_type = 0, *p = p_data; @@ -3119,10 +3118,22 @@ void btm_ble_process_adv_pkt (UINT8 *p_data) return; } + /* sub_code(1) already consumed by caller, need at least num_reports(1) */ + if (evt_len < 2) { + BTM_TRACE_ERROR("btm_ble_process_adv_pkt: evt too short (len=%u)", evt_len); + return; + } + /* Extract the number of reports in this event. */ STREAM_TO_UINT8(num_reports, p); + UINT8 remaining = evt_len - 2; while (num_reports--) { + /* Per-report minimum: evt_type(1) + addr_type(1) + bda(6) + data_len(1) + rssi(1) = 10 */ + if (remaining < 10) { + BTM_TRACE_ERROR("btm_ble_process_adv_pkt: remaining %u too short for report", remaining); + break; + } #if (defined BLE_PRIVACY_SPT && BLE_PRIVACY_SPT == TRUE) /* Save current report start position for address resolution callback */ UINT8 *pp = p; @@ -3131,8 +3142,6 @@ void btm_ble_process_adv_pkt (UINT8 *p_data) STREAM_TO_UINT8 (evt_type, p); STREAM_TO_UINT8 (addr_type, p); STREAM_TO_BDADDR (bda, p); - //BTM_TRACE_ERROR("btm_ble_process_adv_pkt:bda= %0x:%0x:%0x:%0x:%0x:%0x\n", - // bda[0],bda[1],bda[2],bda[3],bda[4],bda[5]); #if (defined BLE_PRIVACY_SPT && BLE_PRIVACY_SPT == TRUE) #if (!CONTROLLER_RPA_LIST_ENABLE) @@ -3142,17 +3151,21 @@ void btm_ble_process_adv_pkt (UINT8 *p_data) /* map address to security record */ match = btm_identity_addr_to_random_pseudo(bda, &addr_type, FALSE); - - // BTM_TRACE_ERROR("btm_ble_process_adv_pkt:bda= %0x:%0x:%0x:%0x:%0x:%0x\n", - // bda[0],bda[1],bda[2],bda[3],bda[4],bda[5]); - /* always do RRA resolution on host */ +#endif + /* Validate data_len before any path (callee reads 1 + data_len + 1 = data_len+2 bytes from p) */ + data_len = *p; /* read without advancing; p points to data_len byte */ + if (data_len + 2 > remaining - 8) { + BTM_TRACE_ERROR("btm_ble_process_adv_pkt: data_len %u + data + rssi exceeds remaining %u", data_len, (UINT16)(remaining - 8)); + break; + } +#if (defined BLE_PRIVACY_SPT && BLE_PRIVACY_SPT == TRUE) + /* RRA path: resolve first, btm_ble_process_adv_pkt_cont will be called from callback; else process now */ if (!match && BTM_BLE_IS_RESOLVE_BDA(bda)) { btm_ble_resolve_random_addr(bda, btm_ble_resolve_random_addr_on_adv, pp); } else #endif btm_ble_process_adv_pkt_cont(bda, addr_type, evt_type, p); #if (defined BLE_PRIVACY_SPT && BLE_PRIVACY_SPT == TRUE && (!CONTROLLER_RPA_LIST_ENABLE)) - //save current adv addr information if p_dev_rec!= NULL tBTM_SEC_DEV_REC *p_dev_rec = btm_find_dev (bda); if(p_dev_rec) { p_dev_rec->ble.current_addr_type = temp_addr_type; @@ -3161,9 +3174,11 @@ void btm_ble_process_adv_pkt (UINT8 *p_data) } #endif STREAM_TO_UINT8(data_len, p); + remaining -= 9; /* evt_type(1) + addr_type(1) + bda(6) + data_len(1) */ /* Advance to the next event data_len + rssi byte */ p += data_len + 1; + remaining -= (data_len + 1); } } diff --git a/components/bt/host/bluedroid/stack/btm/btm_ble_iso.c b/components/bt/host/bluedroid/stack/btm/btm_ble_iso.c index 49483702a8..947c920bff 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_ble_iso.c +++ b/components/bt/host/bluedroid/stack/btm/btm_ble_iso.c @@ -46,7 +46,7 @@ void btm_ble_iso_read_iso_tx_sync_complete(UINT8 *p) if (cb_params.btm_read_tx_sync.status != HCI_SUCCESS) { cb_params.btm_read_tx_sync.status = (BTM_HCI_ERROR | cb_params.btm_read_tx_sync.status); } - cb_params.btm_read_tx_sync.conn_hdl = (cb_params.btm_read_tx_sync.conn_hdl & 0xEFF); + cb_params.btm_read_tx_sync.conn_hdl = (cb_params.btm_read_tx_sync.conn_hdl & 0x0FFF); cb_params.btm_read_tx_sync.time_offset = (cb_params.btm_read_tx_sync.time_offset & 0xFFFFFF); BTM_TRACE_DEBUG("read tx sync cmpl, status 0x%x conn_hdl 0x%x pkt_seq_num %d tx_time_stamp %ld time_offset %ld\n", cb_params.btm_read_tx_sync.status, cb_params.btm_read_tx_sync.conn_hdl, cb_params.btm_read_tx_sync.pkt_seq_num, diff --git a/components/bt/host/bluedroid/stack/btm/btm_ble_privacy.c b/components/bt/host/bluedroid/stack/btm/btm_ble_privacy.c index 09a0835f6a..72dbebd24b 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_ble_privacy.c +++ b/components/bt/host/bluedroid/stack/btm/btm_ble_privacy.c @@ -49,6 +49,7 @@ #define BTM_BLE_META_CLEAR_IRK_LEN 1 #define BTM_BLE_META_READ_IRK_LEN 2 #define BTM_BLE_META_ADD_WL_ATTR_LEN 9 +#define BTM_BLE_IRK_LIST_INVALID_INDEX 0xFF #if CONTROLLER_RPA_LIST_ENABLE && BLE_SMP_ID_RESET_ENABLE static bool is_deleting_zero_addr; @@ -166,7 +167,7 @@ void btm_ble_clear_irk_index(UINT8 index) ** ** Description find the first available IRK list index ** -** Returns index from 0 ~ max (127 default) +** Returns index from 0 ~ max-1, or BTM_BLE_IRK_LIST_INVALID_INDEX if full ** *******************************************************************************/ UINT8 btm_ble_find_irk_index(void) @@ -187,7 +188,7 @@ UINT8 btm_ble_find_irk_index(void) } BTM_TRACE_ERROR ("%s failed, list full", __func__); - return i; + return BTM_BLE_IRK_LIST_INVALID_INDEX; } /******************************************************************************* @@ -209,7 +210,13 @@ void btm_ble_update_resolving_list(BD_ADDR pseudo_bda, BOOLEAN add) if (add) { p_dev_rec->ble.in_controller_list |= BTM_RESOLVING_LIST_BIT; if (!controller_get_interface()->supports_ble_privacy()) { - p_dev_rec->ble.resolving_list_index = btm_ble_find_irk_index(); + UINT8 irk_index = btm_ble_find_irk_index(); + if (irk_index != BTM_BLE_IRK_LIST_INVALID_INDEX) { + p_dev_rec->ble.resolving_list_index = irk_index; + } else { + p_dev_rec->ble.in_controller_list &= ~BTM_RESOLVING_LIST_BIT; + BTM_TRACE_WARNING("%s: IRK list full, cannot add to resolving list", __func__); + } } } else { p_dev_rec->ble.in_controller_list &= ~BTM_RESOLVING_LIST_BIT; diff --git a/components/bt/host/bluedroid/stack/btm/btm_devctl.c b/components/bt/host/bluedroid/stack/btm/btm_devctl.c index 057eba2dbb..4b70ca1c60 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_devctl.c +++ b/components/bt/host/bluedroid/stack/btm/btm_devctl.c @@ -780,8 +780,8 @@ void btm_vsc_complete (UINT8 *p, UINT16 opcode, UINT16 evt_len, if(ble_cb && ble_cb->update_exceptional_list_cmp_cb) { (*ble_cb->update_exceptional_list_cmp_cb)(status, subcode, length, p); } - break; #endif // ((BLE_42_SCAN_EN == TRUE) || (BLE_50_EXTEND_SCAN_EN == TRUE)) + break; } case HCI_VENDOR_BLE_CLEAR_ADV: { uint8_t status; diff --git a/components/bt/host/bluedroid/stack/btm/btm_sco.c b/components/bt/host/bluedroid/stack/btm/btm_sco.c index 6964028000..a9a2d76f46 100644 --- a/components/bt/host/bluedroid/stack/btm/btm_sco.c +++ b/components/bt/host/bluedroid/stack/btm/btm_sco.c @@ -322,7 +322,7 @@ void btm_sco_check_send_pkts (UINT16 sco_inx) } } -void btm_sco_process_num_completed_pkts (UINT8 *p) +void btm_sco_process_num_completed_pkts (UINT8 *p, UINT8 evt_len) { UINT8 num_handles, xx; UINT16 handle; @@ -330,7 +330,20 @@ void btm_sco_process_num_completed_pkts (UINT8 *p) UINT16 sco_inx; tSCO_CB *p_cb = &btm_cb.sco_cb; tSCO_CONN * p_ccb; + + if (evt_len < 1) { + BTM_TRACE_ERROR ("btm_sco_process_num_completed_pkts: evt too short (len=%u)", evt_len); + return; + } + STREAM_TO_UINT8 (num_handles, p); + + if (num_handles > (evt_len - 1) / 4) { + BTM_TRACE_ERROR ("btm_sco_process_num_completed_pkts: num_handles %u exceeds evt_len %u, truncating", + num_handles, evt_len); + num_handles = (evt_len - 1) / 4; + } + for (xx = 0; xx < num_handles; xx++) { STREAM_TO_UINT16 (handle, p); STREAM_TO_UINT16 (num_sent, p); diff --git a/components/bt/host/bluedroid/stack/btm/include/btm_ble_int.h b/components/bt/host/bluedroid/stack/btm/include/btm_ble_int.h index 06a2023599..9340c81dd8 100644 --- a/components/bt/host/bluedroid/stack/btm/include/btm_ble_int.h +++ b/components/bt/host/bluedroid/stack/btm/include/btm_ble_int.h @@ -401,7 +401,7 @@ extern "C" { void btm_ble_timeout(TIMER_LIST_ENT *p_tle); #if (BLE_42_SCAN_EN == TRUE) -void btm_ble_process_adv_pkt (UINT8 *p); +void btm_ble_process_adv_pkt (UINT8 *p, UINT8 evt_len); void btm_ble_process_adv_discard_evt(UINT8 *p); void btm_ble_process_direct_adv_pkt (UINT8 *p); bool btm_ble_adv_pkt_ready(void); diff --git a/components/bt/host/bluedroid/stack/btm/include/btm_int.h b/components/bt/host/bluedroid/stack/btm/include/btm_int.h index e3e8c816bd..6c13beaa50 100644 --- a/components/bt/host/bluedroid/stack/btm/include/btm_int.h +++ b/components/bt/host/bluedroid/stack/btm/include/btm_int.h @@ -1192,7 +1192,7 @@ void btm_pm_proc_ssr_evt (UINT8 *p, UINT16 evt_len); void btm_sco_chk_pend_unpark (UINT8 hci_status, UINT16 hci_handle); #if (BTM_SCO_HCI_INCLUDED == TRUE ) void btm_sco_process_num_bufs (UINT16 num_lm_sco_bufs); -void btm_sco_process_num_completed_pkts (UINT8 *p); +void btm_sco_process_num_completed_pkts (UINT8 *p, UINT8 evt_len); #endif /* (BTM_SCO_HCI_INCLUDED == TRUE ) */ #else #define btm_sco_chk_pend_unpark(hci_status, hci_handle) From 537661fb2e31514bae7e5ad2e068ff7aff3b43e5 Mon Sep 17 00:00:00 2001 From: zhiweijian Date: Wed, 18 Mar 2026 16:33:51 +0800 Subject: [PATCH 6/9] fix(ble/bluedroid): Event length checks and timer/alarm error handling - btu_hcif: validate p_msg->len and hci_evt_len in process_event; pass evt_len to sub-handlers; fix cs_subevt num_steps_reported==0 malloc; bounds in command_complete and role_change_evt - btu_task: handle osi_alarm_new/hash_map_set failure in btu_start_timer, btu_start_quick_timer, btu_start_timer_oneshot --- .../bt/host/bluedroid/stack/btu/btu_hcif.c | 260 +++++++++++++++--- .../bt/host/bluedroid/stack/btu/btu_task.c | 32 ++- 2 files changed, 247 insertions(+), 45 deletions(-) diff --git a/components/bt/host/bluedroid/stack/btu/btu_hcif.c b/components/bt/host/bluedroid/stack/btu/btu_hcif.c index 1ae6e6aaa7..913d33e7d3 100644 --- a/components/bt/host/bluedroid/stack/btu/btu_hcif.c +++ b/components/bt/host/bluedroid/stack/btu/btu_hcif.c @@ -100,7 +100,7 @@ static void btu_hcif_hardware_error_evt (UINT8 *p); static void btu_hcif_flush_occured_evt (void); static void btu_hcif_role_change_evt (UINT8 *p); #endif // #if (CLASSIC_BT_INCLUDED == TRUE) -static void btu_hcif_num_compl_data_pkts_evt (UINT8 *p); +static void btu_hcif_num_compl_data_pkts_evt (UINT8 *p, UINT8 evt_len); #if (CLASSIC_BT_INCLUDED == TRUE) static void btu_hcif_mode_change_evt (UINT8 *p); @@ -194,14 +194,14 @@ static void btu_ble_periodic_adv_sync_trans_recv(UINT8 *p); #if (BLE_FEAT_ISO_BIG_BROADCASTER_EN == TRUE) void btu_ble_create_big_cmd_status(UINT8 status); -static void btu_ble_big_create_complete_evt(UINT8 *p); +static void btu_ble_big_create_complete_evt(UINT8 *p, UINT8 evt_len); void btu_ble_big_terminate_cmd_status(UINT8 status); static void btu_ble_big_terminate_complete_evt(UINT8 *p); #endif // #if (BLE_FEAT_ISO_BIG_BROADCASTER_EN == TRUE) #if (BLE_FEAT_ISO_BIG_SYNCER_EN == TRUE) static void btu_ble_create_big_sync_cmd_status(UINT8 status); -static void btu_ble_big_sync_establish_evt(UINT8 *p); +static void btu_ble_big_sync_establish_evt(UINT8 *p, UINT8 evt_len); static void btu_ble_big_sync_lost_evt(UINT8 *p); static void btu_ble_biginfo_adv_report_evt(UINT8 *p); #endif // #if (BLE_FEAT_ISO_BIG_SYNCER_EN == TRUE) @@ -222,10 +222,10 @@ static void btu_ble_cis_disconnected(UINT16 handle, UINT8 reason); #if (BLE_FEAT_CTE_EN == TRUE) #if (BLE_FEAT_CTE_CONNECTIONLESS_EN == TRUE) -static void btu_ble_cte_connless_iq_report_evt(UINT8 *p); +static void btu_ble_cte_connless_iq_report_evt(UINT8 *p, UINT8 evt_len); #endif // #if (BLE_FEAT_CTE_CONNECTIONLESS_EN == TRUE) #if (BLE_FEAT_CTE_CONNECTION_EN == TRUE) -static void btu_ble_cte_conn_iq_report_evt(UINT8 *p); +static void btu_ble_cte_conn_iq_report_evt(UINT8 *p, UINT8 evt_len); static void btu_ble_cte_req_failed_evt(UINT8 *p); #endif // #if (BLE_FEAT_CTE_CONNECTION_EN == TRUE) #endif // #if (BLE_FEAT_CTE_EN == TRUE) @@ -241,7 +241,7 @@ static void btu_ble_subrate_change_evt(UINT8 *p); #if (BT_BLE_FEAT_PAWR_EN == TRUE) static void btu_ble_pa_subevt_data_request_evt(UINT8 *p); -static void btu_ble_pa_response_report_evt(UINT8 *p); +static void btu_ble_pa_response_report_evt(UINT8 *p, UINT8 evt_len); #endif // (BT_BLE_FEAT_PAWR_EN == TRUE) #if (BT_BLE_FEAT_CHANNEL_SOUNDING == TRUE) @@ -250,8 +250,8 @@ static void btu_ble_cs_read_remote_fae_tab_evt(UINT8 *p); static void btu_ble_cs_security_enable_cmpl_evt(UINT8 *p); static void btu_ble_cs_config_cmpl_evt(UINT8 *p); static void btu_ble_cs_proc_enable_cmpl_evt(UINT8 *p); -static void btu_ble_cs_subevt_result_evt(UINT8 *p); -static void btu_ble_cs_subevt_result_continue_evt(UINT8 *p); +static void btu_ble_cs_subevt_result_evt(UINT8 *p, UINT8 evt_len); +static void btu_ble_cs_subevt_result_continue_evt(UINT8 *p, UINT8 evt_len); #endif // (BT_BLE_FEAT_CHANNEL_SOUNDING == TRUE) #if (BLE_42_ADV_EN == TRUE) @@ -287,9 +287,20 @@ void btu_hcif_process_event (UNUSED_ATTR UINT8 controller_id, BT_HDR *p_msg) #if BLE_INCLUDED == TRUE UINT8 ble_sub_code; #endif + if (p_msg->len < 2) { + HCI_TRACE_ERROR("%s: HCI event too short (len=%u), dropping", __func__, p_msg->len); + return; + } + STREAM_TO_UINT8 (hci_evt_code, p); STREAM_TO_UINT8 (hci_evt_len, p); + if (hci_evt_len > p_msg->len - 2) { + HCI_TRACE_ERROR("%s: HCI evt 0x%02x param len %u exceeds buffer (avail=%u), dropping", + __func__, hci_evt_code, hci_evt_len, p_msg->len - 2); + return; + } + switch (hci_evt_code) { #if (CLASSIC_BT_INCLUDED == TRUE) case HCI_INQUIRY_COMP_EVT: @@ -372,7 +383,7 @@ void btu_hcif_process_event (UNUSED_ATTR UINT8 controller_id, BT_HDR *p_msg) break; #endif // #if (CLASSIC_BT_INCLUDED == TRUE) case HCI_NUM_COMPL_DATA_PKTS_EVT: - btu_hcif_num_compl_data_pkts_evt (p); + btu_hcif_num_compl_data_pkts_evt (p, hci_evt_len); break; #if (CLASSIC_BT_INCLUDED == TRUE) case HCI_MODE_CHANGE_EVT: @@ -469,6 +480,10 @@ void btu_hcif_process_event (UNUSED_ATTR UINT8 controller_id, BT_HDR *p_msg) #if (BLE_INCLUDED == TRUE) case HCI_BLE_EVENT: + if (hci_evt_len < 1) { + HCI_TRACE_ERROR("%s: BLE meta event param len %u invalid (need >= 1), dropping", __func__, hci_evt_len); + break; + } STREAM_TO_UINT8 (ble_sub_code, p); hci_evt_len--; HCI_TRACE_DEBUG("BLE HCI(id=%d) event = 0x%02x)", hci_evt_code, ble_sub_code); @@ -572,7 +587,7 @@ void btu_hcif_process_event (UNUSED_ATTR UINT8 controller_id, BT_HDR *p_msg) #endif // #if (BLE_FEAT_ISO_CIG_PERIPHERAL_EN == TRUE) #if (BLE_FEAT_ISO_BIG_BROADCASTER_EN == TRUE) case HCI_BLE_BIG_CREATE_COMPLETE_EVT: - btu_ble_big_create_complete_evt(p); + btu_ble_big_create_complete_evt(p, hci_evt_len); break; case HCI_BLE_BIG_TERMINATE_COMPLETE_EVT: btu_ble_big_terminate_complete_evt(p); @@ -580,7 +595,7 @@ void btu_hcif_process_event (UNUSED_ATTR UINT8 controller_id, BT_HDR *p_msg) #endif // #if (BLE_FEAT_ISO_BIG_BROADCASTER_EN == TRUE) #if (BLE_FEAT_ISO_BIG_SYNCER_EN == TRUE) case HCI_BLE_BIG_SYNC_ESTABLISHED_EVT: - btu_ble_big_sync_establish_evt(p); + btu_ble_big_sync_establish_evt(p, hci_evt_len); break; case HCI_BLE_BIG_SYNC_LOST_EVT: btu_ble_big_sync_lost_evt(p); @@ -593,13 +608,13 @@ void btu_hcif_process_event (UNUSED_ATTR UINT8 controller_id, BT_HDR *p_msg) #if (BLE_FEAT_CTE_EN == TRUE) #if (BLE_FEAT_CTE_CONNECTIONLESS_EN == TRUE) case HCI_BLE_CONNLESS_IQ_REPORT_EVT: - btu_ble_cte_connless_iq_report_evt(p); + btu_ble_cte_connless_iq_report_evt(p, hci_evt_len); break; #endif // #if (BLE_FEAT_CTE_CONNECTIONLESS_EN == TRUE) #if (BLE_FEAT_CTE_CONNECTION_EN == TRUE) case HCI_BLE_CONN_IQ_REPORT_EVT: - btu_ble_cte_conn_iq_report_evt(p); + btu_ble_cte_conn_iq_report_evt(p, hci_evt_len); break; case HCI_BLE_CTE_REQUEST_FAILED_EVT: btu_ble_cte_req_failed_evt(p); @@ -625,7 +640,7 @@ void btu_hcif_process_event (UNUSED_ATTR UINT8 controller_id, BT_HDR *p_msg) btu_ble_pa_subevt_data_request_evt(p); break; case HCI_BLE_PA_RESPONSE_REPORT_EVT: - btu_ble_pa_response_report_evt(p); + btu_ble_pa_response_report_evt(p, hci_evt_len); break; #endif // #if (BT_BLE_FEAT_PAWR_EN == TRUE) #if (BT_BLE_FEAT_CHANNEL_SOUNDING == TRUE) @@ -645,10 +660,10 @@ void btu_hcif_process_event (UNUSED_ATTR UINT8 controller_id, BT_HDR *p_msg) btu_ble_cs_proc_enable_cmpl_evt(p); break; case HCI_BLE_CS_SUBEVENT_RESULT_EVT: - btu_ble_cs_subevt_result_evt(p); + btu_ble_cs_subevt_result_evt(p, hci_evt_len); break; case HCI_BLE_CS_SUBEVENT_RESULT_CONTINUE_EVT: - btu_ble_cs_subevt_result_continue_evt(p); + btu_ble_cs_subevt_result_continue_evt(p, hci_evt_len); break; #endif // (BT_BLE_FEAT_CHANNEL_SOUNDING == TRUE) } @@ -1410,7 +1425,7 @@ static void btu_hcif_hdl_command_complete (UINT16 opcode, UINT8 *p, UINT16 evt_l #if (BLE_FEAT_ISO_EN == TRUE) #if (BLE_FEAT_ISO_BIG_SYNCER_EN == TRUE) case HCI_BLE_BIG_TERMINATE_SYNC: - UINT16 big_handle; + UINT8 big_handle; STREAM_TO_UINT8(status, p); STREAM_TO_UINT8(big_handle, p); btm_ble_big_sync_terminate_complete(status, big_handle); @@ -1936,14 +1951,14 @@ static void btu_hcif_role_change_evt (UINT8 *p) ** Returns void ** *******************************************************************************/ -static void btu_hcif_num_compl_data_pkts_evt (UINT8 *p) +static void btu_hcif_num_compl_data_pkts_evt (UINT8 *p, UINT8 evt_len) { /* Process for L2CAP and SCO */ - l2c_link_process_num_completed_pkts (p); + l2c_link_process_num_completed_pkts (p, evt_len); /* Send on to SCO */ #if (BTM_SCO_HCI_INCLUDED == TRUE) && (BTM_SCO_INCLUDED == TRUE) - btm_sco_process_num_completed_pkts (p); + btm_sco_process_num_completed_pkts (p, evt_len); #endif } #if (CLASSIC_BT_INCLUDED == TRUE) @@ -2564,19 +2579,36 @@ static void btu_ble_ext_adv_report_evt(UINT8 *p, UINT16 evt_len) UINT16 evt_type = 0; uint8_t addr_type; BD_ADDR bda; + UINT16 remaining; if (!p) { HCI_TRACE_ERROR("%s, Invalid params.", __func__); return; } + if (evt_len < 1) { + HCI_TRACE_ERROR("%s, evt_len too short (%u)", __func__, evt_len); + return; + } + STREAM_TO_UINT8(num_reports, p); + remaining = evt_len - 1; if (num_reports == 0) { HCI_TRACE_ERROR("%s, Invalid number reports is 0", __func__); } + /* Per-report fixed fields: evt_type(2) + addr_type(1) + bda(6) + + * primary_phy(1) + secondary_phy(1) + sid(1) + tx_power(1) + rssi(1) + + * per_adv_interval(2) + dir_addr_type(1) + dir_addr(6) + adv_data_len(1) = 24 */ + #define EXT_ADV_RPT_FIXED_LEN 24 + while (num_reports--) { + if (remaining < EXT_ADV_RPT_FIXED_LEN) { + HCI_TRACE_ERROR("%s, remaining %u too short for report (need %u)", + __func__, remaining, EXT_ADV_RPT_FIXED_LEN); + return; + } #if (defined BLE_PRIVACY_SPT && BLE_PRIVACY_SPT == TRUE) /* Save current report start position for address resolution callback */ pp = p; @@ -2624,6 +2656,14 @@ static void btu_ble_ext_adv_report_evt(UINT8 *p, UINT16 evt_len) STREAM_TO_UINT8(ext_adv_report.dir_addr_type, p); STREAM_TO_BDADDR(ext_adv_report.dir_addr, p); STREAM_TO_UINT8(ext_adv_report.adv_data_len, p); + remaining -= EXT_ADV_RPT_FIXED_LEN; + + if (ext_adv_report.adv_data_len > remaining) { + HCI_TRACE_ERROR("%s, adv_data_len %u exceeds remaining %u", + __func__, ext_adv_report.adv_data_len, remaining); + return; + } + if (ext_adv_report.adv_data_len) { ext_adv_report.adv_data = p; } else { @@ -2632,6 +2672,7 @@ static void btu_ble_ext_adv_report_evt(UINT8 *p, UINT16 evt_len) btm_ble_ext_adv_report_evt(&ext_adv_report); p += ext_adv_report.adv_data_len; + remaining -= ext_adv_report.adv_data_len; } } @@ -2948,7 +2989,7 @@ void btu_ble_create_big_cmd_status(UINT8 status) } } -static void btu_ble_big_create_complete_evt(UINT8 *p) +static void btu_ble_big_create_complete_evt(UINT8 *p, UINT8 evt_len) { HCI_TRACE_DEBUG("%s", __func__); tBTM_BLE_BIG_CREATE_CMPL big_cmpl = {0}; @@ -2959,6 +3000,15 @@ static void btu_ble_big_create_complete_evt(UINT8 *p) return; } + /* Fixed fields: status(1) + big_handle(1) + big_sync_delay(3) + * + transport_latency(3) + phy(1) + nse(1) + bn(1) + pto(1) + irc(1) + * + max_pdu(2) + iso_interval(2) + num_bis(1) = 18 bytes + * (subevent_code already subtracted from evt_len by caller) */ + if (evt_len < 18) { + HCI_TRACE_ERROR("%s, evt too short (len=%u, need>=18)", __func__, evt_len); + return; + } + STREAM_TO_UINT8(big_cmpl.status, p); STREAM_TO_UINT8(big_cmpl.big_handle, p); STREAM_TO_UINT24(big_cmpl.big_sync_delay, p); @@ -2972,11 +3022,14 @@ static void btu_ble_big_create_complete_evt(UINT8 *p) STREAM_TO_UINT16(big_cmpl.iso_interval, p); STREAM_TO_UINT8(num_bis, p); - // Validate num_bis to prevent buffer overflow if (num_bis > BLE_ISO_BIS_MAX_COUNT) { HCI_TRACE_ERROR("%s, num_bis %d exceeds BLE_ISO_BIS_MAX_COUNT %d", __func__, num_bis, BLE_ISO_BIS_MAX_COUNT); num_bis = BLE_ISO_BIS_MAX_COUNT; } + if (num_bis > (evt_len - 18) / 2) { + HCI_TRACE_ERROR("%s, num_bis %d exceeds evt_len %u, truncating", __func__, num_bis, evt_len); + num_bis = (evt_len - 18) / 2; + } big_cmpl.num_bis = num_bis; for (uint8_t i = 0; i < num_bis; i++) @@ -3025,7 +3078,7 @@ void btu_ble_create_big_sync_cmd_status(UINT8 status) } } -static void btu_ble_big_sync_establish_evt(UINT8 *p) +static void btu_ble_big_sync_establish_evt(UINT8 *p, UINT8 evt_len) { tBTM_BLE_BIG_SYNC_ESTAB_CMPL big_estb = {0}; UINT8 num_bis; @@ -3035,6 +3088,15 @@ static void btu_ble_big_sync_establish_evt(UINT8 *p) return; } + /* Fixed fields: status(1) + big_handle(1) + transport_latency_big(3) + * + nse(1) + bn(1) + pto(1) + irc(1) + max_pdu(2) + iso_interval(2) + * + num_bis(1) = 14 bytes + * (subevent_code already subtracted from evt_len by caller) */ + if (evt_len < 14) { + HCI_TRACE_ERROR("%s, evt too short (len=%u, need>=14)", __func__, evt_len); + return; + } + STREAM_TO_UINT8(big_estb.status, p); STREAM_TO_UINT8(big_estb.big_handle, p); STREAM_TO_UINT24(big_estb.transport_latency_big, p); @@ -3046,11 +3108,14 @@ static void btu_ble_big_sync_establish_evt(UINT8 *p) STREAM_TO_UINT16(big_estb.iso_interval, p); STREAM_TO_UINT8(num_bis, p); - // Validate num_bis to prevent buffer overflow if (num_bis > BLE_ISO_BIS_MAX_COUNT) { HCI_TRACE_ERROR("%s, num_bis %d exceeds BLE_ISO_BIS_MAX_COUNT %d", __func__, num_bis, BLE_ISO_BIS_MAX_COUNT); num_bis = BLE_ISO_BIS_MAX_COUNT; } + if (num_bis > (evt_len - 14) / 2) { + HCI_TRACE_ERROR("%s, num_bis %d exceeds evt_len %u, truncating", __func__, num_bis, evt_len); + num_bis = (evt_len - 14) / 2; + } big_estb.num_bis = num_bis; for (uint8_t i = 0; i < num_bis; i++) @@ -3106,7 +3171,7 @@ static void btu_ble_biginfo_adv_report_evt(UINT8 *p) #if (BLE_FEAT_CTE_EN == TRUE) #if (BLE_FEAT_CTE_CONNECTIONLESS_EN == TRUE) -static void btu_ble_cte_connless_iq_report_evt(UINT8 *p) +static void btu_ble_cte_connless_iq_report_evt(UINT8 *p, UINT8 evt_len) { tBTM_BLE_CTE_CONNLESS_IQ_REPORT_EVT connless_iq_rpt = {0}; UINT8 sample_count; @@ -3116,6 +3181,15 @@ static void btu_ble_cte_connless_iq_report_evt(UINT8 *p) return; } + /* Fixed fields: sync_handle(2) + channel_idx(1) + rssi(2) + * + rssi_ant_id(1) + cte_type(1) + slot_dur(1) + pkt_status(1) + * + periodic_evt_counter(2) + sample_count(1) = 12 bytes + * (subevent_code already subtracted from evt_len by caller) */ + if (evt_len < 12) { + HCI_TRACE_ERROR("%s, evt too short (len=%u, need>=12)", __func__, evt_len); + return; + } + STREAM_TO_UINT16(connless_iq_rpt.sync_handle, p); STREAM_TO_UINT8(connless_iq_rpt.channel_idx, p); STREAM_TO_UINT16(connless_iq_rpt.rssi, p); @@ -3126,11 +3200,14 @@ static void btu_ble_cte_connless_iq_report_evt(UINT8 *p) STREAM_TO_UINT16(connless_iq_rpt.periodic_evt_counter, p); STREAM_TO_UINT8(sample_count, p); - // Validate sample_count to prevent buffer overflow if (sample_count > BTM_BLE_CTE_MAX_SAMPLE_COUNT) { HCI_TRACE_ERROR("%s, sample_count %d exceeds maximum %d", __func__, sample_count, BTM_BLE_CTE_MAX_SAMPLE_COUNT); sample_count = BTM_BLE_CTE_MAX_SAMPLE_COUNT; } + if (sample_count > (evt_len - 12) / 2) { + HCI_TRACE_ERROR("%s, sample_count %d exceeds evt_len %u, truncating", __func__, sample_count, evt_len); + sample_count = (evt_len - 12) / 2; + } connless_iq_rpt.sample_count = sample_count; for (uint8_t i = 0; i < sample_count; i++) @@ -3148,7 +3225,7 @@ static void btu_ble_cte_connless_iq_report_evt(UINT8 *p) #endif // #if (BLE_FEAT_CTE_CONNECTIONLESS_EN == TRUE) #if (BLE_FEAT_CTE_CONNECTION_EN == TRUE) -static void btu_ble_cte_conn_iq_report_evt(UINT8 *p) +static void btu_ble_cte_conn_iq_report_evt(UINT8 *p, UINT8 evt_len) { tBTM_BLE_CTE_CONN_IQ_REPORT_EVT conn_iq_rpt = {0}; UINT8 sample_count; @@ -3158,6 +3235,15 @@ static void btu_ble_cte_conn_iq_report_evt(UINT8 *p) return; } + /* Fixed fields: conn_handle(2) + rx_phy(1) + data_channel_idx(1) + * + rssi(2) + rssi_ant_id(1) + cte_type(1) + slot_dur(1) + pkt_status(1) + * + conn_evt_counter(2) + sample_count(1) = 13 bytes + * (subevent_code already subtracted from evt_len by caller) */ + if (evt_len < 13) { + HCI_TRACE_ERROR("%s, evt too short (len=%u, need>=13)", __func__, evt_len); + return; + } + STREAM_TO_UINT16(conn_iq_rpt.conn_handle, p); STREAM_TO_UINT8(conn_iq_rpt.rx_phy, p); STREAM_TO_UINT8(conn_iq_rpt.data_channel_idx, p); @@ -3169,11 +3255,14 @@ static void btu_ble_cte_conn_iq_report_evt(UINT8 *p) STREAM_TO_UINT16(conn_iq_rpt.conn_evt_counter, p); STREAM_TO_UINT8(sample_count, p); - // Validate sample_count to prevent buffer overflow if (sample_count > BTM_BLE_CTE_MAX_SAMPLE_COUNT) { HCI_TRACE_ERROR("%s, sample_count %d exceeds maximum %d", __func__, sample_count, BTM_BLE_CTE_MAX_SAMPLE_COUNT); sample_count = BTM_BLE_CTE_MAX_SAMPLE_COUNT; } + if (sample_count > (evt_len - 13) / 2) { + HCI_TRACE_ERROR("%s, sample_count %d exceeds evt_len %u, truncating", __func__, sample_count, evt_len); + sample_count = (evt_len - 13) / 2; + } conn_iq_rpt.sample_count = sample_count; for (uint8_t i = 0; i < sample_count; i++) @@ -3279,7 +3368,7 @@ static void btu_ble_pa_subevt_data_request_evt(UINT8 *p) btm_ble_pa_subevt_data_req_evt(&pa_subevt_req_evt); } -static void btu_ble_pa_response_report_evt(UINT8 *p) +static void btu_ble_pa_response_report_evt(UINT8 *p, UINT8 evt_len) { tBTM_BLE_PA_RSP_REPORT_EVT pa_rsp_rpt_evt = {0}; if (!p) { @@ -3287,33 +3376,58 @@ static void btu_ble_pa_response_report_evt(UINT8 *p) return; } + /* Fixed fields: adv_handle(1) + subevt(1) + tx_status(1) + num_rsp(1) = 4 bytes + * (subevent_code already subtracted from evt_len by caller) */ + if (evt_len < 4) { + HCI_TRACE_ERROR("%s, evt too short (len=%u, need>=4)", __func__, evt_len); + return; + } + STREAM_TO_UINT8(pa_rsp_rpt_evt.adv_handle, p); STREAM_TO_UINT8(pa_rsp_rpt_evt.subevt, p); STREAM_TO_UINT8(pa_rsp_rpt_evt.tx_status, p); STREAM_TO_UINT8(pa_rsp_rpt_evt.num_rsp, p); - // num_rsp is UINT8, range 0x00 to 0xFF (0 to 255), no need to validate + UINT8 remaining = evt_len - 4; + if (pa_rsp_rpt_evt.num_rsp) { pa_rsp_rpt_evt.rsp_data_info = osi_malloc(pa_rsp_rpt_evt.num_rsp * sizeof(tBTM_BLE_PA_RSP_DATA_INFO)); if (pa_rsp_rpt_evt.rsp_data_info) { for (UINT8 i = 0; i < pa_rsp_rpt_evt.num_rsp; i++) { + /* Per-response fixed: tx_power(1)+rssi(1)+cte_type(1)+rsp_slot(1)+data_status(1)+data_len(1) = 6 */ + if (remaining < 6) { + HCI_TRACE_ERROR("%s, remaining %u too short for response %d", __func__, remaining, i); + pa_rsp_rpt_evt.num_rsp = i; + break; + } STREAM_TO_UINT8(pa_rsp_rpt_evt.rsp_data_info[i].tx_power, p); STREAM_TO_UINT8(pa_rsp_rpt_evt.rsp_data_info[i].rssi, p); STREAM_TO_UINT8(pa_rsp_rpt_evt.rsp_data_info[i].cte_type, p); STREAM_TO_UINT8(pa_rsp_rpt_evt.rsp_data_info[i].rsp_slot, p); STREAM_TO_UINT8(pa_rsp_rpt_evt.rsp_data_info[i].data_status, p); STREAM_TO_UINT8(pa_rsp_rpt_evt.rsp_data_info[i].data_len, p); - // data_len is UINT8, range 0x00 to 0xFF (0 to 255), no need to validate + remaining -= 6; + if (pa_rsp_rpt_evt.rsp_data_info[i].data_len > remaining) { + HCI_TRACE_ERROR("%s, data_len %d exceeds remaining %u at index %d", + __func__, pa_rsp_rpt_evt.rsp_data_info[i].data_len, remaining, i); + pa_rsp_rpt_evt.rsp_data_info[i].data_len = 0; + pa_rsp_rpt_evt.rsp_data_info[i].data = NULL; + pa_rsp_rpt_evt.num_rsp = i + 1; + break; + } if (pa_rsp_rpt_evt.rsp_data_info[i].data_len) { pa_rsp_rpt_evt.rsp_data_info[i].data = osi_malloc(pa_rsp_rpt_evt.rsp_data_info[i].data_len); if (pa_rsp_rpt_evt.rsp_data_info[i].data) { STREAM_TO_ARRAY(pa_rsp_rpt_evt.rsp_data_info[i].data, p, pa_rsp_rpt_evt.rsp_data_info[i].data_len); } else { HCI_TRACE_ERROR("%s, no enough memory for data_len %d at index %d", __func__, pa_rsp_rpt_evt.rsp_data_info[i].data_len, i); + p += pa_rsp_rpt_evt.rsp_data_info[i].data_len; + remaining -= pa_rsp_rpt_evt.rsp_data_info[i].data_len; pa_rsp_rpt_evt.rsp_data_info[i].data_len = 0; } + remaining -= pa_rsp_rpt_evt.rsp_data_info[i].data_len; } else { pa_rsp_rpt_evt.rsp_data_info[i].data = NULL; } @@ -3462,13 +3576,25 @@ static void btu_ble_cs_proc_enable_cmpl_evt(UINT8 *p) btm_ble_cs_proc_enable_cmpl_evt(&proc_en); } -static void btu_ble_cs_subevt_result_evt(UINT8 *p) +static void btu_ble_cs_subevt_result_evt(UINT8 *p, UINT8 evt_len) { tBTM_BLE_CS_SUBEVT_RESULT_CMPL_EVT subevt_result = {0}; if (!p) { HCI_TRACE_ERROR("%s, Invalid params.", __func__); return; } + + /* Fixed fields: conn_handle(2) + config_id(1) + + * start_acl_conn_event_counter(2) + procedure_counter(2) + + * frequency_compensation(2) + reference_power_level(1) + + * procedure_done_status(1) + subevent_done_status(1) + abort_reason(1) + + * num_ant_paths(1) + num_steps_reported(1) = 15 bytes + * (subevent_code already subtracted from evt_len by caller) */ + if (evt_len < 15) { + HCI_TRACE_ERROR("%s, evt too short (len=%u, need>=15)", __func__, evt_len); + return; + } + STREAM_TO_UINT16(subevt_result.conn_handle, p); STREAM_TO_UINT8(subevt_result.config_id, p); STREAM_TO_UINT16(subevt_result.start_acl_conn_event_counter, p); @@ -3482,27 +3608,49 @@ static void btu_ble_cs_subevt_result_evt(UINT8 *p) UINT8 num_steps_reported; STREAM_TO_UINT8(num_steps_reported, p); - // Validate num_steps_reported per BLE spec: Range 0x00 to 0xA0 (0 to 160) if (num_steps_reported > BTM_BLE_CS_MAX_STEPS_REPORTED) { HCI_TRACE_ERROR("%s, num_steps_reported %d exceeds maximum %d", __func__, num_steps_reported, BTM_BLE_CS_MAX_STEPS_REPORTED); num_steps_reported = BTM_BLE_CS_MAX_STEPS_REPORTED; } subevt_result.num_steps_reported = num_steps_reported; - subevt_result.step_info = osi_malloc(subevt_result.num_steps_reported * sizeof(tBTM_BLE_CS_STEP_INFO)); - if (subevt_result.step_info) { + UINT8 remaining = evt_len - 15; + subevt_result.step_info = (subevt_result.num_steps_reported > 0) + ? osi_malloc(subevt_result.num_steps_reported * sizeof(tBTM_BLE_CS_STEP_INFO)) + : NULL; + if (subevt_result.step_info || subevt_result.num_steps_reported == 0) { for (uint8_t i = 0; i < subevt_result.num_steps_reported; i++) { + /* Per-step fixed: step_mode(1) + step_channel(1) + step_data_len(1) = 3 */ + if (remaining < 3) { + HCI_TRACE_ERROR("%s, remaining %u too short for step %d", __func__, remaining, i); + subevt_result.num_steps_reported = i; + break; + } STREAM_TO_UINT8(subevt_result.step_info[i].step_mode, p); STREAM_TO_UINT8(subevt_result.step_info[i].step_channel, p); STREAM_TO_UINT8(subevt_result.step_info[i].step_data_len, p); - // step_data_len is UINT8, range 0x00 to 0xFF (0 to 255), no need to validate + remaining -= 3; + if (subevt_result.step_info[i].step_data_len > remaining) { + HCI_TRACE_ERROR("%s, step_data_len %d exceeds remaining %u at step %d", + __func__, subevt_result.step_info[i].step_data_len, remaining, i); + subevt_result.step_info[i].step_data_len = 0; + subevt_result.step_info[i].data = NULL; + subevt_result.num_steps_reported = i + 1; + break; + } subevt_result.step_info[i].data = osi_malloc(subevt_result.step_info[i].step_data_len); if (subevt_result.step_info[i].data) { STREAM_TO_ARRAY(subevt_result.step_info[i].data, p, subevt_result.step_info[i].step_data_len); } else if (subevt_result.step_info[i].step_data_len) { HCI_TRACE_ERROR("%s, no memory.", __func__); + p += subevt_result.step_info[i].step_data_len; + subevt_result.step_info[i].step_data_len = 0; + subevt_result.step_info[i].data = NULL; + subevt_result.num_steps_reported = i + 1; + break; } + remaining -= subevt_result.step_info[i].step_data_len; } } @@ -3521,7 +3669,7 @@ static void btu_ble_cs_subevt_result_evt(UINT8 *p) } -static void btu_ble_cs_subevt_result_continue_evt(UINT8 *p) +static void btu_ble_cs_subevt_result_continue_evt(UINT8 *p, UINT8 evt_len) { tBTM_BLE_CS_SUBEVT_RESULT_CONTINUE_EVT subevt_continue_result = {0}; @@ -3530,6 +3678,15 @@ static void btu_ble_cs_subevt_result_continue_evt(UINT8 *p) return; } + /* Fixed fields: conn_handle(2) + config_id(1) + proc_done_status(1) + * + subevt_done_status(1) + abort_reason(1) + num_ant_paths(1) + * + num_steps_reported(1) = 8 bytes + * (subevent_code already subtracted from evt_len by caller) */ + if (evt_len < 8) { + HCI_TRACE_ERROR("%s, evt too short (len=%u, need>=8)", __func__, evt_len); + return; + } + STREAM_TO_UINT16(subevt_continue_result.conn_handle, p); STREAM_TO_UINT8(subevt_continue_result.config_id, p); STREAM_TO_UINT8(subevt_continue_result.proc_done_status, p); @@ -3539,26 +3696,47 @@ static void btu_ble_cs_subevt_result_continue_evt(UINT8 *p) UINT8 num_steps_reported; STREAM_TO_UINT8(num_steps_reported, p); - // Validate num_steps_reported per BLE spec: Range 0x00 to 0xA0 (0 to 160) if (num_steps_reported > BTM_BLE_CS_MAX_STEPS_REPORTED) { HCI_TRACE_ERROR("%s, num_steps_reported %d exceeds maximum %d", __func__, num_steps_reported, BTM_BLE_CS_MAX_STEPS_REPORTED); num_steps_reported = BTM_BLE_CS_MAX_STEPS_REPORTED; } subevt_continue_result.num_steps_reported = num_steps_reported; - subevt_continue_result.step_info = osi_malloc(subevt_continue_result.num_steps_reported * sizeof(tBTM_BLE_CS_STEP_INFO)); - if (subevt_continue_result.step_info) { + UINT8 remaining = evt_len - 8; + subevt_continue_result.step_info = (subevt_continue_result.num_steps_reported > 0) + ? osi_malloc(subevt_continue_result.num_steps_reported * sizeof(tBTM_BLE_CS_STEP_INFO)) + : NULL; + if (subevt_continue_result.step_info || subevt_continue_result.num_steps_reported == 0) { for (uint8_t i = 0; i < subevt_continue_result.num_steps_reported; i++) { + if (remaining < 3) { + HCI_TRACE_ERROR("%s, remaining %u too short for step %d", __func__, remaining, i); + subevt_continue_result.num_steps_reported = i; + break; + } STREAM_TO_UINT8(subevt_continue_result.step_info[i].step_mode, p); STREAM_TO_UINT8(subevt_continue_result.step_info[i].step_channel, p); STREAM_TO_UINT8(subevt_continue_result.step_info[i].step_data_len, p); - // step_data_len is UINT8, range 0x00 to 0xFF (0 to 255), no need to validate + remaining -= 3; + if (subevt_continue_result.step_info[i].step_data_len > remaining) { + HCI_TRACE_ERROR("%s, step_data_len %d exceeds remaining %u at step %d", + __func__, subevt_continue_result.step_info[i].step_data_len, remaining, i); + subevt_continue_result.step_info[i].step_data_len = 0; + subevt_continue_result.step_info[i].data = NULL; + subevt_continue_result.num_steps_reported = i + 1; + break; + } subevt_continue_result.step_info[i].data = osi_malloc(subevt_continue_result.step_info[i].step_data_len); if (subevt_continue_result.step_info[i].data) { STREAM_TO_ARRAY(subevt_continue_result.step_info[i].data, p, subevt_continue_result.step_info[i].step_data_len); } else if (subevt_continue_result.step_info[i].step_data_len) { HCI_TRACE_ERROR("%s, no memory.", __func__); + p += subevt_continue_result.step_info[i].step_data_len; + subevt_continue_result.step_info[i].step_data_len = 0; + subevt_continue_result.step_info[i].data = NULL; + subevt_continue_result.num_steps_reported = i + 1; + break; } + remaining -= subevt_continue_result.step_info[i].step_data_len; } } else { HCI_TRACE_ERROR("%s, no memory for step_info.", __func__); diff --git a/components/bt/host/bluedroid/stack/btu/btu_task.c b/components/bt/host/bluedroid/stack/btu/btu_task.c index 23fbd72af7..2fe59a9260 100644 --- a/components/bt/host/bluedroid/stack/btu/btu_task.c +++ b/components/bt/host/bluedroid/stack/btu/btu_task.c @@ -449,7 +449,17 @@ void btu_start_timer(TIMER_LIST_ENT *p_tle, UINT16 type, UINT32 timeout_sec) osi_mutex_lock(&btu_general_alarm_lock, OSI_MUTEX_MAX_TIMEOUT); if (!hash_map_has_key(btu_general_alarm_hash_map, p_tle)) { alarm = osi_alarm_new("btu_gen", btu_general_alarm_cb, (void *)p_tle, 0); - hash_map_set(btu_general_alarm_hash_map, p_tle, alarm); + if (alarm == NULL) { + HCI_TRACE_ERROR("%s Unable to create new alarm", __func__); + osi_mutex_unlock(&btu_general_alarm_lock); + return; + } + if (!hash_map_set(btu_general_alarm_hash_map, p_tle, alarm)) { + HCI_TRACE_ERROR("%s Unable to set alarm in map", __func__); + osi_alarm_free(alarm); + osi_mutex_unlock(&btu_general_alarm_lock); + return; + } } osi_mutex_unlock(&btu_general_alarm_lock); @@ -569,8 +579,12 @@ void btu_start_quick_timer(TIMER_LIST_ENT *p_tle, UINT16 type, UINT32 timeout_ti osi_mutex_unlock(&btu_l2cap_alarm_lock); return; } - - hash_map_set(btu_l2cap_alarm_hash_map, p_tle, (void *)alarm); + if (!hash_map_set(btu_l2cap_alarm_hash_map, p_tle, (void *)alarm)) { + HCI_TRACE_ERROR("%s Unable to set alarm in map", __func__); + osi_alarm_free(alarm); + osi_mutex_unlock(&btu_l2cap_alarm_lock); + return; + } } osi_mutex_unlock(&btu_l2cap_alarm_lock); @@ -656,7 +670,17 @@ void btu_start_timer_oneshot(TIMER_LIST_ENT *p_tle, UINT16 type, UINT32 timeout_ osi_mutex_lock(&btu_oneshot_alarm_lock, OSI_MUTEX_MAX_TIMEOUT); if (!hash_map_has_key(btu_oneshot_alarm_hash_map, p_tle)) { alarm = osi_alarm_new("btu_oneshot", btu_oneshot_alarm_cb, (void *)p_tle, 0); - hash_map_set(btu_oneshot_alarm_hash_map, p_tle, alarm); + if (alarm == NULL) { + HCI_TRACE_ERROR("%s Unable to create new alarm", __func__); + osi_mutex_unlock(&btu_oneshot_alarm_lock); + return; + } + if (!hash_map_set(btu_oneshot_alarm_hash_map, p_tle, alarm)) { + HCI_TRACE_ERROR("%s Unable to set alarm in map", __func__); + osi_alarm_free(alarm); + osi_mutex_unlock(&btu_oneshot_alarm_lock); + return; + } } osi_mutex_unlock(&btu_oneshot_alarm_lock); From 1d31286f1ab4bd252fee37b766a60bb6b903d6c6 Mon Sep 17 00:00:00 2001 From: zhiweijian Date: Wed, 18 Mar 2026 16:33:59 +0800 Subject: [PATCH 7/9] fix(ble/bluedroid): Fix double-free, exec write, bounds and HCI param checks - gap_ble: add length/attribute checks in gap_proc_write_req - gatt_cl: set p_cmd->p_cmd=NULL before memset to avoid double-free; pending_cl_req %= GATT_CL_MAX_LCB - gatt_sr: fix exec write zeroed_attrs and offset/len bounds, OOM cleanup - gatt_sr_hash: null checks for p_attr->p_next, p_data+=2, len==0 in gatts_calculate_datebase_hash, gatts_show_local_database - gatt_utils: explicit return NULL, indent, idxGATT_MAX_ATTR_LEN, gatt_cleanup_upon_disc dealloc branch - hciblecmds: length/handle validation in BLE ext adv/BIG sync HCI commands --- .../bt/host/bluedroid/stack/gap/gap_ble.c | 3 + .../bt/host/bluedroid/stack/gatt/gatt_cl.c | 4 +- .../bt/host/bluedroid/stack/gatt/gatt_sr.c | 97 +++++++++++++++++-- .../host/bluedroid/stack/gatt/gatt_sr_hash.c | 24 ++++- .../bt/host/bluedroid/stack/gatt/gatt_utils.c | 36 ++++--- .../bt/host/bluedroid/stack/hcic/hciblecmds.c | 42 ++++---- 6 files changed, 157 insertions(+), 49 deletions(-) diff --git a/components/bt/host/bluedroid/stack/gap/gap_ble.c b/components/bt/host/bluedroid/stack/gap/gap_ble.c index 823fadc983..97e7d9da82 100644 --- a/components/bt/host/bluedroid/stack/gap/gap_ble.c +++ b/components/bt/host/bluedroid/stack/gap/gap_ble.c @@ -315,6 +315,9 @@ UINT8 gap_proc_write_req( tGATTS_REQ_TYPE type, tGATT_WRITE_REQ *p_data) switch (p_db_attr->uuid) { #if (GATTS_DEVICE_NAME_WRITABLE == TRUE) case GATT_UUID_GAP_DEVICE_NAME: { + if (p_data->len > BD_NAME_LEN) { + return GATT_INVALID_ATTR_LEN; + } UINT8 *p_val = p_data->value; p_val[p_data->len] = '\0'; BTM_SetLocalDeviceName((char *)p_val, BT_DEVICE_TYPE_BLE); diff --git a/components/bt/host/bluedroid/stack/gatt/gatt_cl.c b/components/bt/host/bluedroid/stack/gatt/gatt_cl.c index 98214f8797..5d7c7e2bb3 100644 --- a/components/bt/host/bluedroid/stack/gatt/gatt_cl.c +++ b/components/bt/host/bluedroid/stack/gatt/gatt_cl.c @@ -1146,9 +1146,11 @@ BOOLEAN gatt_cl_send_next_cmd_inq(tGATT_TCB *p_tcb) } } else { GATT_TRACE_ERROR("gatt_cl_send_next_cmd_inq: L2CAP sent error"); - + /* attp_send_msg_to_l2cap() already freed p_cmd->p_cmd on failure */ + p_cmd->p_cmd = NULL; memset(p_cmd, 0, sizeof(tGATT_CMD_Q)); p_tcb->pending_cl_req ++; + p_tcb->pending_cl_req %= GATT_CL_MAX_LCB; p_cmd = &p_tcb->cl_cmd_q[p_tcb->pending_cl_req]; } diff --git a/components/bt/host/bluedroid/stack/gatt/gatt_sr.c b/components/bt/host/bluedroid/stack/gatt/gatt_sr.c index 25bc51a02c..62b170b1a2 100644 --- a/components/bt/host/bluedroid/stack/gatt/gatt_sr.c +++ b/components/bt/host/bluedroid/stack/gatt/gatt_sr.c @@ -486,9 +486,11 @@ void gatt_process_exec_write_req (tGATT_TCB *p_tcb, UINT8 op_code, UINT16 len, U tGATT_IF gatt_if; UINT16 conn_id; UINT16 queue_num = 0; - BOOLEAN is_first = TRUE; + UINT16 zeroed_cap = 0, zeroed_count = 0; + void **zeroed_attrs = NULL; BOOLEAN is_prepare_write_valid = FALSE; BOOLEAN is_need_dequeue_sr_cmd = FALSE; + BOOLEAN sr_cmd_already_dequeued = FALSE; tGATT_PREPARE_WRITE_RECORD *prepare_record = NULL; tGATT_PREPARE_WRITE_QUEUE_DATA * queue_data = NULL; @@ -529,6 +531,7 @@ void gatt_process_exec_write_req (tGATT_TCB *p_tcb, UINT8 op_code, UINT16 len, U gatt_exec_write_rsp.op_code = GATT_RSP_EXEC_WRITE; gatt_send_packet(p_tcb, (UINT8 *)(&gatt_exec_write_rsp), sizeof(gatt_exec_write_rsp)); gatt_dequeue_sr_cmd(p_tcb); + sr_cmd_already_dequeued = TRUE; if (flag != GATT_PREP_WRITE_CANCEL){ is_prepare_write_valid = TRUE; } @@ -549,23 +552,99 @@ void gatt_process_exec_write_req (tGATT_TCB *p_tcb, UINT8 op_code, UINT16 len, U gatt_send_error_rsp(p_tcb, prepare_record->error_code_app, GATT_REQ_EXEC_WRITE, 0, is_need_dequeue_sr_cmd); } - //dequeue prepare write data + /* Track which attributes we've zeroed so we only zero on first write per attribute. + * Zeroing all up-front would leave attr_len=0 for attributes whose write is later + * skipped (e.g. overflow), causing data loss. */ + if (is_prepare_write_valid && queue_num > 0) { + zeroed_cap = queue_num; /* max unique attributes in this exec is at most queue_num */ + zeroed_attrs = (void **)osi_calloc(zeroed_cap * sizeof(void *)); + if (zeroed_attrs == NULL) { + GATT_TRACE_ERROR("%s: no memory for zeroed_attrs, abort exec write", __func__); + while (fixed_queue_try_peek_first(prepare_record->queue)) { + queue_data = fixed_queue_dequeue(prepare_record->queue, FIXED_QUEUE_MAX_TIMEOUT); + osi_free(queue_data); + } + fixed_queue_free(prepare_record->queue, NULL); + prepare_record->queue = NULL; + { + UINT16 total_num_saved = prepare_record->total_num; + prepare_record->total_num = 0; + prepare_record->error_code_app = GATT_SUCCESS; + /* Only send error (and dequeue via gatt_send_error_rsp) if we did not already send success and dequeue (first branch) */ + if (!sr_cmd_already_dequeued) { + gatt_send_error_rsp(p_tcb, GATT_NO_RESOURCES, GATT_REQ_EXEC_WRITE, 0, TRUE); + } else { + /* Success already sent to peer; notify apps and clear prep_cnt so state stays consistent */ + if (!gatt_sr_is_prep_cnt_zero(p_tcb)) { + if (total_num_saved > queue_num) { + trans_id = gatt_sr_enqueue_cmd(p_tcb, op_code, 0); + gatt_sr_copy_prep_cnt_to_cback_cnt(p_tcb); + } + for (i = 0; i < GATT_MAX_APPS; i++) { + if (p_tcb->prep_cnt[i]) { + gatt_if = (tGATT_IF) (i + 1); + conn_id = GATT_CREATE_CONN_ID(p_tcb->tcb_idx, gatt_if); + gatt_sr_send_req_callback(conn_id, + trans_id, + GATTS_REQ_TYPE_WRITE_EXEC, + (tGATTS_DATA *)&flag); + p_tcb->prep_cnt[i] = 0; + } + } + /* OOM path: we enqueued sr_cmd but will not get app response flow; clear sr_cmd so later requests are not dropped */ + if (total_num_saved > queue_num) { + gatt_dequeue_sr_cmd(p_tcb); + } + } + } + } + return; + } + } + while(fixed_queue_try_peek_first(prepare_record->queue)) { queue_data = fixed_queue_dequeue(prepare_record->queue, FIXED_QUEUE_MAX_TIMEOUT); if (is_prepare_write_valid){ if((queue_data->p_attr->p_value != NULL) && (queue_data->p_attr->p_value->attr_val.attr_val != NULL)){ - if(is_first) { - //clear attr_val.attr_len before handle prepare write data - queue_data->p_attr->p_value->attr_val.attr_len = 0; - is_first = FALSE; + UINT16 attr_max_len = queue_data->p_attr->p_value->attr_val.attr_max_len; + if (queue_data->offset > attr_max_len || + queue_data->len > attr_max_len - queue_data->offset) { + GATT_TRACE_ERROR("%s: exec write overflow prevented, offset=%u len=%u max=%u", + __func__, queue_data->offset, queue_data->len, attr_max_len); + } else { + UINT16 k; + BOOLEAN need_zero = TRUE; + /* Zero attr_len only on first successful write to this attribute in this exec */ + if (zeroed_attrs != NULL) { + for (k = 0; k < zeroed_count; k++) { + if (zeroed_attrs[k] == (void *)queue_data->p_attr) { + need_zero = FALSE; + break; + } + } + } + if (need_zero) { + queue_data->p_attr->p_value->attr_val.attr_len = 0; + if (zeroed_attrs != NULL && zeroed_count < zeroed_cap) { + zeroed_attrs[zeroed_count++] = (void *)queue_data->p_attr; + } + } + memcpy(queue_data->p_attr->p_value->attr_val.attr_val + queue_data->offset, + queue_data->value, queue_data->len); + { + UINT16 write_end = queue_data->offset + queue_data->len; + if (write_end > queue_data->p_attr->p_value->attr_val.attr_len) { + queue_data->p_attr->p_value->attr_val.attr_len = write_end; + } + } } - memcpy(queue_data->p_attr->p_value->attr_val.attr_val+queue_data->offset, queue_data->value, queue_data->len); - //don't forget to increase the attribute value length in the gatts database. - queue_data->p_attr->p_value->attr_val.attr_len += queue_data->len; } } osi_free(queue_data); } + if (zeroed_attrs != NULL) { + osi_free(zeroed_attrs); + } fixed_queue_free(prepare_record->queue, NULL); prepare_record->queue = NULL; diff --git a/components/bt/host/bluedroid/stack/gatt/gatt_sr_hash.c b/components/bt/host/bluedroid/stack/gatt/gatt_sr_hash.c index c7db69b6f5..79c6fdb207 100644 --- a/components/bt/host/bluedroid/stack/gatt/gatt_sr_hash.c +++ b/components/bt/host/bluedroid/stack/gatt/gatt_sr_hash.c @@ -86,10 +86,13 @@ static size_t calculate_database_info_size(void) len += 8 + get_uuid_stream_len(p_attr->p_value->incl_handle.service_type); } else if (p_attr->uuid == GATT_UUID_CHAR_DECLARE) { tBT_UUID char_uuid = {0}; - // Characteristic declaration + if (p_attr->p_next == NULL) { + GATT_TRACE_ERROR("%s: malformed DB, char decl at handle %u has no value attr", + __func__, p_attr->handle); + break; + } p_attr = (tGATT_ATTR16 *)p_attr->p_next; attr_uuid_to_bt_uuid((void *)p_attr, &char_uuid); - // Increment 1 to fetch characteristic uuid from value declaration attribute len += 7 + get_uuid_stream_len(char_uuid); } else if (p_attr->uuid == GATT_UUID_CHAR_DESCRIPTION || p_attr->uuid == GATT_UUID_CHAR_CLIENT_CONFIG || @@ -136,14 +139,17 @@ static void fill_database_info(UINT8 *p_data) gatt_build_uuid_to_stream(&p_data, p_attr->p_value->incl_handle.service_type); } else if (p_attr->uuid == GATT_UUID_CHAR_DECLARE) { tBT_UUID char_uuid = {0}; - // Characteristic declaration + if (p_attr->p_next == NULL) { + GATT_TRACE_ERROR("%s: malformed DB, char decl at handle %u has no value attr", + __func__, p_attr->handle); + break; + } UINT16_TO_STREAM(p_data, p_attr->handle); UINT16_TO_STREAM(p_data, GATT_UUID_CHAR_DECLARE); UINT8_TO_STREAM(p_data, p_attr->p_value->char_decl.property); UINT16_TO_STREAM(p_data, p_attr->p_value->char_decl.char_val_handle); p_attr = (tGATT_ATTR16 *)p_attr->p_next; attr_uuid_to_bt_uuid((void *)p_attr, &char_uuid); - // Increment 1 to fetch characteristic uuid from value declaration attribute gatt_build_uuid_to_stream(&p_data, char_uuid); } else if (p_attr->uuid == GATT_UUID_CHAR_DESCRIPTION || p_attr->uuid == GATT_UUID_CHAR_CLIENT_CONFIG || @@ -160,6 +166,7 @@ static void fill_database_info(UINT8 *p_data) // TODO: process extended properties descriptor if (p_attr->p_value->attr_val.attr_len == 2) { memcpy(p_data, p_attr->p_value->attr_val.attr_val, 2); + p_data += 2; } else { UINT16_TO_STREAM(p_data, 0x0000); } @@ -180,6 +187,11 @@ tGATT_STATUS gatts_calculate_datebase_hash(BT_OCTET16 hash) len = calculate_database_info_size(); + if (len == 0) { + memset(hash, 0, BT_OCTET16_LEN); + return GATT_SUCCESS; + } + data_buf = (UINT8 *)osi_malloc(len); if (data_buf == NULL) { GATT_TRACE_ERROR ("%s failed to allocate buffer (%u)\n", __func__, len); @@ -236,6 +248,10 @@ void gatts_show_local_database(void) tBT_UUID char_uuid = {0}; tGATT_ATTR16 *p_char_val; p_char_val = (tGATT_ATTR16 *)p_attr->p_next; + if (p_char_val == NULL) { + printf("characteristic (malformed - no value attr)\n"); + break; + } attr_uuid_to_bt_uuid((void *)p_char_val, &char_uuid); printf("%s\n", gatt_get_attr_name(p_attr->uuid)); diff --git a/components/bt/host/bluedroid/stack/gatt/gatt_utils.c b/components/bt/host/bluedroid/stack/gatt/gatt_utils.c index 1b817a0053..b536468e2e 100644 --- a/components/bt/host/bluedroid/stack/gatt/gatt_utils.c +++ b/components/bt/host/bluedroid/stack/gatt/gatt_utils.c @@ -797,11 +797,11 @@ tGATTS_SRV_CHG *gatt_is_bda_in_the_srv_chg_clt_list (BD_ADDR bda) p_buf = (tGATTS_SRV_CHG *)list_node(node); if (!memcmp( bda, p_buf->bda, BD_ADDR_LEN)) { GATT_TRACE_DEBUG("bda is in the srv chg clt list"); - break; + return p_buf; } } - return p_buf; + return NULL; } #endif // (GATTS_INCLUDED == TRUE) @@ -1006,11 +1006,11 @@ tGATT_TCB *gatt_allocate_tcb_by_bdaddr(BD_ADDR bda, tBT_TRANSPORT transport) allocated = TRUE; } if (i != GATT_INDEX_INVALID) { - p_tcb = gatt_tcb_alloc(i); - if (!p_tcb) { - return NULL; - } if (allocated) { + p_tcb = gatt_tcb_alloc(i); + if (!p_tcb) { + return NULL; + } memset(p_tcb, 0, sizeof(tGATT_TCB)); #if (SMP_INCLUDED == TRUE) p_tcb->pending_enc_clcb = fixed_queue_new(QUEUE_SIZE_MAX); @@ -1018,6 +1018,11 @@ tGATT_TCB *gatt_allocate_tcb_by_bdaddr(BD_ADDR bda, tBT_TRANSPORT transport) p_tcb->in_use = TRUE; p_tcb->tcb_idx = i; p_tcb->transport = transport; + } else { + p_tcb = gatt_get_tcb_by_idx(i); + if (!p_tcb) { + return NULL; + } } memcpy(p_tcb->peer_bda, bda, BD_ADDR_LEN); #if GATTS_ROBUST_CACHING_ENABLED @@ -1778,10 +1783,10 @@ tGATT_TCB *gatt_find_tcb_by_cid (UINT16 lcid) for(p_node = list_begin(gatt_cb.p_tcb_list); p_node; p_node = list_next(p_node)) { p_tcb = list_node(p_node); if (p_tcb->in_use && p_tcb->att_lcid == lcid) { - break; + return p_tcb; } } - return p_tcb; + return NULL; } /******************************************************************************* @@ -1969,7 +1974,7 @@ void gatt_sr_update_cback_cnt(tGATT_TCB *p_tcb, tGATT_IF gatt_if, BOOLEAN is_inc #if (GATTS_INCLUDED == TRUE) UINT8 idx = ((UINT8) gatt_if) - 1 ; - if (p_tcb) { + if (p_tcb && idx < GATT_MAX_APPS) { if (is_reset_first) { gatt_sr_reset_cback_cnt(p_tcb); } @@ -1999,9 +2004,9 @@ void gatt_sr_update_prep_cnt(tGATT_TCB *p_tcb, tGATT_IF gatt_if, BOOLEAN is_inc, UINT8 idx = ((UINT8) gatt_if) - 1 ; GATT_TRACE_DEBUG("gatt_sr_update_prep_cnt tcb idx=%d gatt_if=%d is_inc=%d is_reset_first=%d", - p_tcb->tcb_idx, gatt_if, is_inc, is_reset_first); + p_tcb ? p_tcb->tcb_idx : 0, gatt_if, is_inc, is_reset_first); - if (p_tcb) { + if (p_tcb && idx < GATT_MAX_APPS) { if (is_reset_first) { gatt_sr_reset_prep_cnt(p_tcb); } @@ -2164,6 +2169,11 @@ UINT8 gatt_send_write_msg (tGATT_TCB *p_tcb, UINT16 clcb_idx, UINT8 op_code, { tGATT_CL_MSG msg; + if (len > GATT_MAX_ATTR_LEN) { + GATT_TRACE_ERROR("%s: len %u exceeds max %u", __func__, len, GATT_MAX_ATTR_LEN); + return GATT_ILLEGAL_PARAMETER; + } + msg.attr_value.handle = handle; msg.attr_value.len = len; msg.attr_value.offset = offset; @@ -2312,9 +2322,9 @@ void gatt_cleanup_upon_disc(BD_ADDR bda, UINT16 reason, tBT_TRANSPORT transport) GATT_TRACE_DEBUG ("found p_clcb conn_id=%d clcb_idx=%d", p_clcb->conn_id, p_clcb->clcb_idx); if (p_clcb->operation != GATTC_OPTYPE_NONE) { gatt_end_operation(p_clcb, GATT_ERROR, NULL); - p_clcb = NULL; + } else { + gatt_clcb_dealloc(p_clcb); } - gatt_clcb_dealloc(p_clcb); } } #if (GATTC_INCLUDED == TRUE) diff --git a/components/bt/host/bluedroid/stack/hcic/hciblecmds.c b/components/bt/host/bluedroid/stack/hcic/hciblecmds.c index a688c12b00..445c61ec06 100644 --- a/components/bt/host/bluedroid/stack/hcic/hciblecmds.c +++ b/components/bt/host/bluedroid/stack/hcic/hciblecmds.c @@ -1329,6 +1329,10 @@ UINT8 btsnd_hcic_ble_set_ext_adv_data(UINT8 adv_handle, HCI_TRACE_EVENT("%s, adv_handle = %d, operation = %d, fragment_prefrence = %d,\ data_len = %d", __func__, adv_handle, operation, fragment_prefrence, data_len); + if (data_len > HCIC_PARAM_SIZE_EXT_ADV_WRITE_DATA) { + data_len = HCIC_PARAM_SIZE_EXT_ADV_WRITE_DATA; + } + HCIC_BLE_CMD_CREATED(p, pp, data_len + 4); UINT16_TO_STREAM(pp, HCI_BLE_SET_EXT_ADV_DATA); UINT8_TO_STREAM(pp, data_len + 4); @@ -1336,10 +1340,6 @@ UINT8 btsnd_hcic_ble_set_ext_adv_data(UINT8 adv_handle, UINT8_TO_STREAM(pp, operation); UINT8_TO_STREAM(pp, fragment_prefrence); - if (data_len > HCIC_PARAM_SIZE_EXT_ADV_WRITE_DATA) { - data_len = HCIC_PARAM_SIZE_EXT_ADV_WRITE_DATA; - } - UINT8_TO_STREAM (pp, data_len); if (p_data != NULL && data_len > 0){ @@ -1361,6 +1361,10 @@ UINT8 btsnd_hcic_ble_set_ext_adv_scan_rsp_data(UINT8 adv_handle, HCI_TRACE_EVENT("%s, adv_handle = %d, operation = %d, fragment_prefrence = %d,\n\ data_len = %d", __func__, adv_handle, operation, fragment_prefrence, data_len); + if (data_len > HCIC_PARAM_SIZE_EXT_ADV_WRITE_DATA) { + data_len = HCIC_PARAM_SIZE_EXT_ADV_WRITE_DATA; + } + HCIC_BLE_CMD_CREATED(p, pp, data_len + 4); UINT16_TO_STREAM(pp, HCI_BLE_SET_EXT_SCAN_RSP_DATA); @@ -1369,12 +1373,6 @@ UINT8 btsnd_hcic_ble_set_ext_adv_scan_rsp_data(UINT8 adv_handle, UINT8_TO_STREAM(pp, operation); UINT8_TO_STREAM(pp, fragment_prefrence); - memset(pp, 0, data_len); - - if (data_len > HCIC_PARAM_SIZE_EXT_ADV_WRITE_DATA) { - data_len = HCIC_PARAM_SIZE_EXT_ADV_WRITE_DATA; - } - UINT8_TO_STREAM (pp, data_len); if (p_data != NULL && data_len > 0) { ARRAY_TO_STREAM (pp, p_data, data_len); @@ -1540,6 +1538,10 @@ UINT8 btsnd_hcic_ble_set_periodic_adv_data(UINT8 adv_handle, HCI_TRACE_EVENT("%s, adv_handle = %d, operation = %d, len = %d", __func__, adv_handle, operation, len); + if (len > HCIC_PARAM_SIZE_WRITE_PERIODIC_ADV_DATA) { + len = HCIC_PARAM_SIZE_WRITE_PERIODIC_ADV_DATA; + } + HCIC_BLE_CMD_CREATED(p, pp, len + 3); UINT16_TO_STREAM(pp, HCI_BLE_SET_PERIOD_ADV_DATA); @@ -1547,12 +1549,6 @@ UINT8 btsnd_hcic_ble_set_periodic_adv_data(UINT8 adv_handle, UINT8_TO_STREAM(pp, adv_handle); UINT8_TO_STREAM(pp, operation); - //memset(pp, 0, len); - - if (len > HCIC_PARAM_SIZE_WRITE_PERIODIC_ADV_DATA) { - len = HCIC_PARAM_SIZE_WRITE_PERIODIC_ADV_DATA; - } - UINT8_TO_STREAM (pp, len); if (p_data != NULL && len > 0) { @@ -2268,17 +2264,19 @@ UINT8 btsnd_hcic_ble_big_sync_create(uint8_t big_handle, uint16_t sync_handle, HCI_TRACE_DEBUG("big sync create: big_handle %d sync_handle %d encryption %d mse %d big_sync_timeout %d", big_handle, sync_handle, encryption, mse, big_sync_timeout); - // for (uint8_t i = 0; i < num_bis; i++) - // { - // HCI_TRACE_ERROR("i %d bis %d", bis[i]); - // } + #define HCIC_PARAM_SIZE_BIG_SYNC_CREATE_FIXED 24 + if (num_bis > (HCIC_PARAM_SIZE_BIG_SYNC_CREATE_PARAMS - HCIC_PARAM_SIZE_BIG_SYNC_CREATE_FIXED)) { + HCI_TRACE_ERROR("%s: num_bis %u exceeds max", __func__, num_bis); + return FALSE; + } - HCIC_BLE_CMD_CREATED(p, pp, HCIC_PARAM_SIZE_BIG_SYNC_CREATE_PARAMS); + UINT8 param_size = HCIC_PARAM_SIZE_BIG_SYNC_CREATE_FIXED + num_bis; + HCIC_BLE_CMD_CREATED(p, pp, param_size); pp = (UINT8 *)(p + 1); UINT16_TO_STREAM(pp, HCI_BLE_BIG_CREATE_SYNC); - UINT8_TO_STREAM(pp, HCIC_PARAM_SIZE_BIG_SYNC_CREATE_PARAMS); + UINT8_TO_STREAM(pp, param_size); UINT8_TO_STREAM(pp, big_handle); UINT16_TO_STREAM(pp, sync_handle); From 16d523e9bf1f0520e1f27b1d26d8a02144e45bb2 Mon Sep 17 00:00:00 2001 From: zhiweijian Date: Wed, 18 Mar 2026 16:34:11 +0800 Subject: [PATCH 8/9] fix(ble/bluedroid): BLE credit, reject when p_rcb NULL, timeout and leak fixes - l2c_int: align struct/constant types with l2c_ble/l2c_main - l2c_api: null/state checks in L2CA_SendFixedChnlData - l2c_ble: reject when p_rcb==NULL, add L2CAP_CMD_BLE_FLOW_CTRL_CREDIT; l2cble_init_direct_conn int64_t timeout and link_timeout==0 fix - l2c_link: null/state checks and cleanup in hci_disc_comp/timeout/send_to_lower - l2c_main: free p_msg on FCR non-Basic and COC branches; fix LE credit handling; process_l2cap_cmd bounds - l2c_utils: credit/queue cleanup and null checks in l2cu_disconnect_chnl --- .../bluedroid/stack/l2cap/include/l2c_int.h | 4 +- .../bt/host/bluedroid/stack/l2cap/l2c_api.c | 5 ++ .../bt/host/bluedroid/stack/l2cap/l2c_ble.c | 47 ++++++++++++++++--- .../bt/host/bluedroid/stack/l2cap/l2c_link.c | 35 ++++++++++++-- .../bt/host/bluedroid/stack/l2cap/l2c_main.c | 41 +++++++++------- .../bt/host/bluedroid/stack/l2cap/l2c_utils.c | 20 +++++++- 6 files changed, 119 insertions(+), 33 deletions(-) diff --git a/components/bt/host/bluedroid/stack/l2cap/include/l2c_int.h b/components/bt/host/bluedroid/stack/l2cap/include/l2c_int.h index df956dc442..cfc5163664 100644 --- a/components/bt/host/bluedroid/stack/l2cap/include/l2c_int.h +++ b/components/bt/host/bluedroid/stack/l2cap/include/l2c_int.h @@ -461,7 +461,7 @@ typedef struct t_l2c_linkcb { */ /* create connection retry count*/ UINT8 retry_create_con; - UINT32 start_time_s; + int64_t start_time_s; #endif #if (L2CAP_ROUND_ROBIN_CHANNEL_SERVICE == TRUE) @@ -736,7 +736,7 @@ extern void l2c_link_timeout (tL2C_LCB *p_lcb); extern void l2c_info_timeout (tL2C_LCB *p_lcb); extern void l2c_link_check_send_pkts (tL2C_LCB *p_lcb, tL2C_CCB *p_ccb, BT_HDR *p_buf); extern void l2c_link_adjust_allocation (void); -extern void l2c_link_process_num_completed_pkts (UINT8 *p); +extern void l2c_link_process_num_completed_pkts (UINT8 *p, UINT8 evt_len); extern void l2c_link_process_num_completed_blocks (UINT8 controller_id, UINT8 *p, UINT16 evt_len); extern void l2c_link_processs_num_bufs (UINT16 num_lm_acl_bufs); extern UINT8 l2c_link_pkts_rcvd (UINT16 *num_pkts, UINT16 *handles); diff --git a/components/bt/host/bluedroid/stack/l2cap/l2c_api.c b/components/bt/host/bluedroid/stack/l2cap/l2c_api.c index 07377901a7..c0487f16a5 100644 --- a/components/bt/host/bluedroid/stack/l2cap/l2c_api.c +++ b/components/bt/host/bluedroid/stack/l2cap/l2c_api.c @@ -1863,6 +1863,11 @@ UINT16 L2CA_SendFixedChnlData (UINT16 fixed_cid, BD_ADDR rem_bda, BT_HDR *p_buf) BOOLEAN L2CA_CheckIsCongest(UINT16 fixed_cid, BD_ADDR addr) { + if ((fixed_cid < L2CAP_FIRST_FIXED_CHNL) || (fixed_cid > L2CAP_LAST_FIXED_CHNL)) { + L2CAP_TRACE_ERROR ("L2CA_CheckIsCongest() Invalid CID: 0x%04x", fixed_cid); + return TRUE; + } + tL2C_LCB *p_lcb; p_lcb = l2cu_find_lcb_by_bd_addr(addr, BT_TRANSPORT_LE); diff --git a/components/bt/host/bluedroid/stack/l2cap/l2c_ble.c b/components/bt/host/bluedroid/stack/l2cap/l2c_ble.c index 88cc764dcf..c430e55629 100644 --- a/components/bt/host/bluedroid/stack/l2cap/l2c_ble.c +++ b/components/bt/host/bluedroid/stack/l2cap/l2c_ble.c @@ -828,7 +828,6 @@ void l2cble_process_sig_cmd (tL2C_LCB *p_lcb, UINT8 *p, UINT16 pkt_len) STREAM_TO_UINT16(mps, p); STREAM_TO_UINT16(credits, p); L2CAP_TRACE_DEBUG("%s spsm %x, scid %x", __func__, spsm, scid); - UNUSED(spsm); p_ccb = l2cu_find_ccb_by_remote_cid(p_lcb, scid); if (p_ccb) { @@ -836,12 +835,11 @@ void l2cble_process_sig_cmd (tL2C_LCB *p_lcb, UINT8 *p, UINT16 pkt_len) break; } - #if 0 p_rcb = l2cu_find_ble_rcb_by_psm(spsm); if (p_rcb == NULL) { + l2cu_reject_ble_connection(p_lcb, id, L2CAP_LE_RESULT_NO_PSM); break; } - #endif p_ccb = l2cu_allocate_ccb(p_lcb, 0); if (p_ccb == NULL) { @@ -862,6 +860,34 @@ void l2cble_process_sig_cmd (tL2C_LCB *p_lcb, UINT8 *p, UINT16 pkt_len) l2cu_send_peer_ble_credit_based_conn_res(p_ccb, L2CAP_LE_RESULT_CONN_OK); break; } + case L2CAP_CMD_BLE_FLOW_CTRL_CREDIT: { + if (cmd_len < L2CAP_CMD_BLE_FLOW_CTRL_CREDIT_LEN) { + L2CAP_TRACE_WARNING ("L2CAP - LE - flow ctrl credit too short: %d", cmd_len); + return; + } + tL2C_CCB *p_ccb = NULL; + UINT16 lcid; + UINT16 credit; + STREAM_TO_UINT16(lcid, p); + STREAM_TO_UINT16(credit, p); + + p_ccb = l2cu_find_ccb_by_cid(p_lcb, lcid); + if (p_ccb == NULL) { + L2CAP_TRACE_WARNING ("L2CAP - LE - flow ctrl credit for unknown CID: 0x%04x", lcid); + break; + } + + if ((p_ccb->peer_conn_cfg.credits + credit) > L2CAP_LE_MAX_CREDIT) { + L2CAP_TRACE_WARNING ("L2CAP - LE - credit overflow, disconnecting CID: 0x%04x", lcid); + l2cble_send_peer_disc_req(p_ccb); + } else { + p_ccb->peer_conn_cfg.credits += credit; + L2CAP_TRACE_DEBUG ("L2CAP - LE - credits updated to %d for CID: 0x%04x", + p_ccb->peer_conn_cfg.credits, lcid); + l2c_link_check_send_pkts(p_ccb->p_lcb, NULL, NULL); + } + break; + } case L2CAP_CMD_DISC_REQ: { if (cmd_len < 4) { L2CAP_TRACE_WARNING ("L2CAP - LE - short cmd: %d", cmd_len); @@ -988,13 +1014,20 @@ BOOLEAN l2cble_init_direct_conn (tL2C_LCB *p_lcb) uint32_t link_timeout = L2CAP_BLE_LINK_CONNECT_TOUT; if(GATTC_CONNECT_RETRY_COUNT) { if(!p_lcb->retry_create_con) { - p_lcb->start_time_s = (esp_system_get_time()/1000); + p_lcb->start_time_s = esp_system_get_time() / 1000; } - uint32_t current_time = (esp_system_get_time()/1000); - link_timeout = (L2CAP_BLE_LINK_CONNECT_TOUT*1000 - (current_time - p_lcb->start_time_s))/1000; + int64_t current_time = esp_system_get_time() / 1000; + int64_t elapsed_ms = current_time - p_lcb->start_time_s; + int64_t timeout_ms = (int64_t)L2CAP_BLE_LINK_CONNECT_TOUT * 1000; + int64_t remaining_ms = timeout_ms - elapsed_ms; - if(link_timeout == 0 || link_timeout > L2CAP_BLE_LINK_CONNECT_TOUT) { + if (remaining_ms <= 0 || remaining_ms > timeout_ms) { link_timeout = L2CAP_BLE_LINK_CONNECT_TOUT; + } else { + link_timeout = (uint32_t)(remaining_ms / 1000); + if (link_timeout == 0) { + link_timeout = 1; + } } } diff --git a/components/bt/host/bluedroid/stack/l2cap/l2c_link.c b/components/bt/host/bluedroid/stack/l2cap/l2c_link.c index 9eefc8c075..347e220a41 100644 --- a/components/bt/host/bluedroid/stack/l2cap/l2c_link.c +++ b/components/bt/host/bluedroid/stack/l2cap/l2c_link.c @@ -379,6 +379,9 @@ BOOLEAN l2c_link_hci_disc_comp (UINT16 handle, UINT8 reason) } else { #if (BLE_INCLUDED == TRUE) tL2C_LINK_STATE link_state_temp = p_lcb->link_state; +#if ((BLE_42_ADV_EN == TRUE) || (GATTC_CONNECT_RETRY_EN == TRUE) || (BLE_50_EXTEND_ADV_EN == TRUE)) + UINT8 link_role_temp = p_lcb->link_role; +#endif #endif // (BLE_INCLUDED == TRUE) /* There can be a case when we rejected PIN code authentication */ /* otherwise save a new reason */ @@ -469,6 +472,7 @@ BOOLEAN l2c_link_hci_disc_comp (UINT16 handle, UINT8 reason) } #endif } + /* l2cu_create_conn must not release the LCB on failure */ if (l2cu_create_conn(p_lcb, transport)) { lcb_is_free = FALSE; /* still using this lcb */ } @@ -476,13 +480,15 @@ BOOLEAN l2c_link_hci_disc_comp (UINT16 handle, UINT8 reason) p_lcb->p_pending_ccb = NULL; #if (BLE_INCLUDED == TRUE) - if(p_lcb->transport == BT_TRANSPORT_LE) { + /* Use p_lcb->transport so BLE reconnection (slave adv restart / master retry) runs even when + * there was no pending CCB; 'transport' is only set when (p_first_ccb || p_pending_ccb). */ + if (p_lcb->transport == BT_TRANSPORT_LE) { // for legacy adv, adv restart in gatt_le_connect_cback->gatt_cleanup_upon_disc->BTM_Recovery_Pre_State if (reason == HCI_ERR_CONN_FAILED_ESTABLISHMENT) { #if (BLE_42_FEATURE_SUPPORT == TRUE) #if (BLE_42_ADV_EN == TRUE) - if(!btm_ble_inter_get() && p_lcb->link_role == HCI_ROLE_SLAVE) { + if(!btm_ble_inter_get() && link_role_temp == HCI_ROLE_SLAVE) { L2CAP_TRACE_DEBUG("slave resatrt adv, retry count %d reason 0x%x\n", p_lcb->retry_create_con, reason); tBTM_STATUS start_adv_status = btm_ble_start_adv(); if (start_adv_status != BTM_SUCCESS) { @@ -496,7 +502,7 @@ BOOLEAN l2c_link_hci_disc_comp (UINT16 handle, UINT8 reason) if ((reason == HCI_ERR_CONN_FAILED_ESTABLISHMENT) || (link_state_temp == LST_CONNECTING)) { #if (GATTC_CONNECT_RETRY_EN == TRUE) - if(p_lcb->link_role == HCI_ROLE_MASTER && p_lcb->retry_create_con < GATTC_CONNECT_RETRY_COUNT) { + if(link_role_temp == HCI_ROLE_MASTER && p_lcb->retry_create_con < GATTC_CONNECT_RETRY_COUNT) { L2CAP_TRACE_DEBUG("master retry connect, retry count %d reason 0x%x\n", p_lcb->retry_create_con, reason); p_lcb->retry_create_con ++; // create connection retry @@ -513,7 +519,7 @@ BOOLEAN l2c_link_hci_disc_comp (UINT16 handle, UINT8 reason) if ((reason == HCI_ERR_CONN_FAILED_ESTABLISHMENT) || (link_state_temp == LST_DISCONNECTED)) { #if (BLE_50_FEATURE_SUPPORT == TRUE) #if (BLE_50_EXTEND_ADV_EN == TRUE) - if(btm_ble_inter_get() && p_lcb->link_role == HCI_ROLE_SLAVE) { + if(btm_ble_inter_get() && link_role_temp == HCI_ROLE_SLAVE) { L2CAP_TRACE_DEBUG("slave restart extend adv, retry count %d reason 0x%x\n", p_lcb->retry_create_con, reason); tBTM_STATUS start_adv_status = BTM_BleStartExtAdvRestart(handle); if (start_adv_status != BTM_SUCCESS) { @@ -626,6 +632,7 @@ void l2c_link_timeout (tL2C_LCB *p_lcb) #endif /* Release the LCB */ l2cu_release_lcb (p_lcb); + return; } /* If link is connected, check for inactivity timeout */ @@ -1314,6 +1321,13 @@ static BOOLEAN l2c_link_send_to_lower (tL2C_LCB *p_lcb, BT_HDR *p_buf) acl_data_size = controller->get_acl_data_size_classic(); xmit_window = l2cb.controller_xmit_window; } + + if (p_buf->len <= HCI_DATA_PREAMBLE_SIZE) { + L2CAP_TRACE_ERROR ("l2c_link_send_to_lower - bad buffer len: %u", p_buf->len); + osi_free(p_buf); + return FALSE; + } + num_segs = (p_buf->len - HCI_DATA_PREAMBLE_SIZE + acl_data_size - 1) / acl_data_size; @@ -1395,15 +1409,26 @@ static BOOLEAN l2c_link_send_to_lower (tL2C_LCB *p_lcb, BT_HDR *p_buf) ** Returns void ** *******************************************************************************/ -void l2c_link_process_num_completed_pkts (UINT8 *p) +void l2c_link_process_num_completed_pkts (UINT8 *p, UINT8 evt_len) { UINT8 num_handles, xx; UINT16 handle; UINT16 num_sent; tL2C_LCB *p_lcb; + if (evt_len < 1) { + L2CAP_TRACE_ERROR ("l2c_link_process_num_completed_pkts: evt too short (len=%u)", evt_len); + return; + } + STREAM_TO_UINT8 (num_handles, p); + if (num_handles > (evt_len - 1) / 4) { + L2CAP_TRACE_ERROR ("l2c_link_process_num_completed_pkts: num_handles %u exceeds evt_len %u, truncating", + num_handles, evt_len); + num_handles = (evt_len - 1) / 4; + } + for (xx = 0; xx < num_handles; xx++) { STREAM_TO_UINT16 (handle, p); STREAM_TO_UINT16 (num_sent, p); diff --git a/components/bt/host/bluedroid/stack/l2cap/l2c_main.c b/components/bt/host/bluedroid/stack/l2cap/l2c_main.c index c2c3150b2d..1a58b69277 100644 --- a/components/bt/host/bluedroid/stack/l2cap/l2c_main.c +++ b/components/bt/host/bluedroid/stack/l2cap/l2c_main.c @@ -150,7 +150,6 @@ void l2c_rcv_acl_data (BT_HDR *p_msg) #if (!CONFIG_BT_STACK_NO_LOG) UINT16 psm; #endif - UINT16 credit; /* Extract the handle */ STREAM_TO_UINT16 (handle, p); @@ -292,6 +291,9 @@ void l2c_rcv_acl_data (BT_HDR *p_msg) if (p_ccb->peer_cfg.fcr.mode != L2CAP_FCR_BASIC_MODE) { #if (CLASSIC_BT_INCLUDED == TRUE) l2c_fcr_proc_pdu (p_ccb, p_msg); +#else + /* Classic FCR not compiled (e.g. BLE-only); free p_msg to avoid leak */ + osi_free (p_msg); #endif ///CLASSIC_BT_INCLUDED == TRUE } else { (*l2cb.fixed_reg[rcv_cid - L2CAP_FIRST_FIXED_CHNL].pL2CA_FixedData_Cb) @@ -310,31 +312,24 @@ void l2c_rcv_acl_data (BT_HDR *p_msg) osi_free (p_msg); } else { if (p_lcb->transport == BT_TRANSPORT_LE) { - // Got a pkt, valid send out credits to the peer device - credit = L2CAP_LE_DEFAULT_CREDIT; - L2CAP_TRACE_DEBUG("%s Credits received %d",__func__, credit); - if((p_ccb->peer_conn_cfg.credits + credit) > L2CAP_LE_MAX_CREDIT) { - /* we have received credits more than max coc credits, - * so disconnecting the Le Coc Channel - */ -#if (BLE_INCLUDED == TRUE) - l2cble_send_peer_disc_req (p_ccb); -#endif ///BLE_INCLUDED == TRUE - } else { - p_ccb->peer_conn_cfg.credits += credit; - l2c_link_check_send_pkts (p_ccb->p_lcb, NULL, NULL); - } + l2c_link_check_send_pkts (p_ccb->p_lcb, NULL, NULL); } /* Basic mode packets go straight to the state machine */ if (p_ccb->peer_cfg.fcr.mode == L2CAP_FCR_BASIC_MODE) { -#if (CLASSIC_BT_INCLUDED == TRUE) +#if (L2CAP_COC_INCLUDED == TRUE) l2c_csm_execute (p_ccb, L2CEVT_L2CAP_DATA, p_msg); -#endif ///CLASSIC_BT_INCLUDED == TRUE +#else + /* COC not included: state machine not compiled; free p_msg to avoid leak */ + osi_free (p_msg); +#endif } else { /* eRTM or streaming mode, so we need to validate states first */ if ((p_ccb->chnl_state == CST_OPEN) || (p_ccb->chnl_state == CST_CONFIG)) { #if (CLASSIC_BT_INCLUDED == TRUE) l2c_fcr_proc_pdu (p_ccb, p_msg); +#else + /* Classic FCR not compiled (e.g. BLE-only); free p_msg to avoid leak */ + osi_free (p_msg); #endif ///CLASSIC_BT_INCLUDED == TRUE } else { osi_free (p_msg); @@ -423,14 +418,26 @@ static void process_l2cap_cmd (tL2C_LCB *p_lcb, UINT8 *p, UINT16 pkt_len) switch (cmd_code) { case L2CAP_CMD_REJECT: + if (cmd_len < L2CAP_CMD_REJECT_LEN) { + L2CAP_TRACE_WARNING ("L2CAP - cmd reject too short, cmd_len: %d", cmd_len); + break; + } STREAM_TO_UINT16 (rej_reason, p); if (rej_reason == L2CAP_CMD_REJ_MTU_EXCEEDED) { + if (cmd_len < L2CAP_CMD_REJECT_LEN + 2) { + L2CAP_TRACE_WARNING ("L2CAP - MTU rej too short, cmd_len: %d", cmd_len); + break; + } STREAM_TO_UINT16 (rej_mtu, p); /* What to do with the MTU reject ? We have negotiated an MTU. For now */ /* we will ignore it and let a higher protocol timeout take care of it */ L2CAP_TRACE_WARNING ("L2CAP - MTU rej Handle: %d MTU: %d", p_lcb->handle, rej_mtu); } if (rej_reason == L2CAP_CMD_REJ_INVALID_CID) { + if (cmd_len < L2CAP_CMD_REJECT_LEN + 4) { + L2CAP_TRACE_WARNING ("L2CAP - CID rej too short, cmd_len: %d", cmd_len); + break; + } STREAM_TO_UINT16 (rcid, p); STREAM_TO_UINT16 (lcid, p); diff --git a/components/bt/host/bluedroid/stack/l2cap/l2c_utils.c b/components/bt/host/bluedroid/stack/l2cap/l2c_utils.c index 3d3c1cc3ff..ad996c18da 100644 --- a/components/bt/host/bluedroid/stack/l2cap/l2c_utils.c +++ b/components/bt/host/bluedroid/stack/l2cap/l2c_utils.c @@ -1899,15 +1899,31 @@ void l2cu_disconnect_chnl (tL2C_CCB *p_ccb) UINT16 local_cid = p_ccb->local_cid; if (local_cid >= L2CAP_BASE_APPL_CID) { - tL2CA_DISCONNECT_IND_CB *p_disc_cb = p_ccb->p_rcb->api.pL2CA_DisconnectInd_Cb; + tL2CA_DISCONNECT_IND_CB *p_disc_cb = NULL; + + if (p_ccb->p_rcb != NULL) { + p_disc_cb = p_ccb->p_rcb->api.pL2CA_DisconnectInd_Cb; + } L2CAP_TRACE_WARNING ("L2CAP - disconnect_chnl CID: 0x%04x", local_cid); l2cu_send_peer_disc_req (p_ccb); + /* + * l2cu_send_peer_disc_req drains xmit_hold_q in basic FCR mode. + * Free the queue and NULL it here to prevent l2cu_release_ccb from + * calling fixed_queue_free again on already-dequeued buffers. + */ + if (p_ccb->xmit_hold_q != NULL) { + fixed_queue_free(p_ccb->xmit_hold_q, osi_free_func); + p_ccb->xmit_hold_q = NULL; + } + l2cu_release_ccb (p_ccb); - (*p_disc_cb)(local_cid, FALSE); + if (p_disc_cb) { + (*p_disc_cb)(local_cid, FALSE); + } } else { /* failure on the AMP channel, probably need to disconnect ACL */ L2CAP_TRACE_ERROR ("L2CAP - disconnect_chnl CID: 0x%04x Ignored", local_cid); From 50747e4f63eea4985fc07156d8675c4d124fc4c8 Mon Sep 17 00:00:00 2001 From: zhiweijian Date: Wed, 18 Mar 2026 16:34:27 +0800 Subject: [PATCH 9/9] fix(ble/bluedroid): Null/range checks, crypto cleanup and API consistency - smp_api.h/smp_int.h: SMP_OPCODE_ARRAY_SIZE and SecureConnectionOobDataReply declaration alignment - p_256_ecc_pp/p_256_multprecision: bounds and overflow fixes in ECC/multiprecision - smp_act: init le_key; p_dev_rec null check in smp_key_distribution; smp_compute_dhkey failure notify in smp_both_have_public_keys - smp_api: early state/cb_evt check in SMP_SecureConnectionOobDataReply - smp_cmac: input/length validation in cmac_aes_k_calculate and aes_cipher_msg_auth_code - smp_keys: smp_gen_p2_4_confirm return and smp_calculate_comfirm_cont; smp_process_private_key/smp_compute_dhkey cleanup and peer_pub_be clear - smp_l2c: fix callback param types with L2CAP - smp_main: event/state bounds in smp_sm_event; smp_get_event_name default string - smp_utils: cmd_code y^2 = (x^2 - 3)*x + b (mod q), so we calculate the x^2 - 3 value here */ - x_x[0] -= 3; + DWORD three[2 * KEY_LENGTH_DWORDS_P256] = {0}; + three[0] = 3; + multiprecision_sub(x_x, x_x, three, 2 * KEY_LENGTH_DWORDS_P256); /* Using math relations. (a*b) % q = ((a%q)*(b%q)) % q ==> (x^2 - 3)*x = (((x^2 - 3) % q) * x % q) % q */ multiprecision_fast_mod_P256(x_x_q, x_x); diff --git a/components/bt/host/bluedroid/stack/smp/p_256_multprecision.c b/components/bt/host/bluedroid/stack/smp/p_256_multprecision.c index 92291d98b3..d0a45a247c 100644 --- a/components/bt/host/bluedroid/stack/smp/p_256_multprecision.c +++ b/components/bt/host/bluedroid/stack/smp/p_256_multprecision.c @@ -262,7 +262,7 @@ void multiprecision_mult(DWORD *c, DWORD *a, DWORD *b, uint32_t keyLength) DWORD V; U = V = W = 0; - multiprecision_init(c, keyLength); + multiprecision_init(c, 2 * keyLength); //assume little endian right now for (uint32_t i = 0; i < keyLength; i++) { @@ -340,7 +340,7 @@ void multiprecision_fast_mod(DWORD *c, DWORD *a) c[2] += V; V = c[2] < V; c[2] += U; - V = c[2] < U; + V += c[2] < U; c[3] += V; V = c[3] < V; c[4] += V; @@ -594,6 +594,7 @@ void multiprecision_fast_mod_P256(DWORD *c, DWORD *a) void multiprecision_inv_mod(DWORD *aminus, DWORD *u, uint32_t keyLength) { + DWORD u_local[KEY_LENGTH_DWORDS_P256]; DWORD v[KEY_LENGTH_DWORDS_P256]; DWORD A[KEY_LENGTH_DWORDS_P256 + 1]; DWORD C[KEY_LENGTH_DWORDS_P256 + 1]; @@ -605,14 +606,15 @@ void multiprecision_inv_mod(DWORD *aminus, DWORD *u, uint32_t keyLength) modp = curve.p; } + multiprecision_copy(u_local, u, keyLength); multiprecision_copy(v, modp, keyLength); multiprecision_init(A, keyLength); multiprecision_init(C, keyLength); A[0] = 1; - while (!multiprecision_iszero(u, keyLength)) { - while (!(u[0] & 0x01)) { // u is even - multiprecision_rshift(u, u, keyLength); + while (!multiprecision_iszero(u_local, keyLength)) { + while (!(u_local[0] & 0x01)) { // u is even + multiprecision_rshift(u_local, u_local, keyLength); if (!(A[0] & 0x01)) { // A is even multiprecision_rshift(A, A, keyLength); } else { @@ -633,11 +635,11 @@ void multiprecision_inv_mod(DWORD *aminus, DWORD *u, uint32_t keyLength) } } - if (multiprecision_compare(u, v, keyLength) >= 0) { - multiprecision_sub(u, u, v, keyLength); + if (multiprecision_compare(u_local, v, keyLength) >= 0) { + multiprecision_sub(u_local, u_local, v, keyLength); multiprecision_sub_mod(A, A, C, keyLength); } else { - multiprecision_sub(v, v, u, keyLength); + multiprecision_sub(v, v, u_local, keyLength); multiprecision_sub_mod(C, C, A, keyLength); } } diff --git a/components/bt/host/bluedroid/stack/smp/smp_act.c b/components/bt/host/bluedroid/stack/smp/smp_act.c index 4dbb0fa6c8..5b3d76ddd3 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_act.c +++ b/components/bt/host/bluedroid/stack/smp/smp_act.c @@ -406,7 +406,7 @@ void smp_send_id_info(tSMP_CB *p_cb, tSMP_INT_DATA *p_data) smp_send_cmd(SMP_OPCODE_ID_ADDR, p_cb); #if (BLE_INCLUDED == TRUE) - tBTM_LE_KEY_VALUE le_key; + tBTM_LE_KEY_VALUE le_key = {0}; if ((p_cb->peer_auth_req & SMP_AUTH_BOND) && (p_cb->loc_auth_req & SMP_AUTH_BOND)) { btm_sec_save_le_key(p_cb->pairing_bda, BTM_LE_KEY_LID, &le_key, TRUE); @@ -1420,7 +1420,9 @@ void smp_key_distribution(tSMP_CB *p_cb, tSMP_INT_DATA *p_data) if (smp_get_state() == SMP_STATE_BOND_PENDING) { if (p_cb->derive_lk) { tBTM_SEC_DEV_REC* p_dev_rec = btm_find_dev(p_cb->pairing_bda); - if (!(p_dev_rec->sec_flags & BTM_SEC_LE_LINK_KEY_AUTHED) && + if (p_dev_rec == NULL) { + SMP_TRACE_WARNING("%s device record not found, skip LK derivation", __func__); + } else if (!(p_dev_rec->sec_flags & BTM_SEC_LE_LINK_KEY_AUTHED) && (p_dev_rec->sec_flags & BTM_SEC_LINK_KEY_AUTHED)) { SMP_TRACE_DEBUG("%s BREDR key is higher security than existing LE keys, " "don't derive LK from LTK", __func__); @@ -1678,7 +1680,11 @@ void smp_both_have_public_keys(tSMP_CB *p_cb, tSMP_INT_DATA *p_data) SMP_TRACE_DEBUG("%s\n", __func__); /* invokes DHKey computation */ - smp_compute_dhkey(p_cb); + if (!smp_compute_dhkey(p_cb)) { + UINT8 reason = SMP_PAIR_INTERNAL_ERR; + smp_sm_event(p_cb, SMP_AUTH_CMPL_EVT, &reason); + return; + } /* on slave side invokes sending local public key to the peer */ if (p_cb->role == HCI_ROLE_SLAVE) { diff --git a/components/bt/host/bluedroid/stack/smp/smp_api.c b/components/bt/host/bluedroid/stack/smp/smp_api.c index 0b00bf151c..7a592b775d 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_api.c +++ b/components/bt/host/bluedroid/stack/smp/smp_api.c @@ -483,6 +483,10 @@ void SMP_SecureConnectionOobDataReply(UINT8 *p_data) return; } + if (p_cb->state != SMP_STATE_WAIT_APP_RSP || p_cb->cb_evt != SMP_SC_OOB_REQ_EVT) { + return; + } + /* Set local oob data when req_oob_type = SMP_OOB_BOTH */ memcpy(&p_oob->loc_oob_data, smp_get_local_oob_data(), sizeof(tSMP_LOC_OOB_DATA)); @@ -491,10 +495,6 @@ void SMP_SecureConnectionOobDataReply(UINT8 *p_data) __FUNCTION__, p_cb->req_oob_type, p_oob->loc_oob_data.present, p_oob->peer_oob_data.present); - if (p_cb->state != SMP_STATE_WAIT_APP_RSP || p_cb->cb_evt != SMP_SC_OOB_REQ_EVT) { - return; - } - BOOLEAN data_missing = FALSE; switch (p_cb->req_oob_type) { case SMP_OOB_PEER: diff --git a/components/bt/host/bluedroid/stack/smp/smp_cmac.c b/components/bt/host/bluedroid/stack/smp/smp_cmac.c index fd10e50760..d060dd7033 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_cmac.c +++ b/components/bt/host/bluedroid/stack/smp/smp_cmac.c @@ -153,6 +153,11 @@ static BOOLEAN cmac_aes_k_calculate(BT_OCTET16 key, UINT8 *p_signature, UINT16 t SMP_TRACE_EVENT ("cmac_aes_k_calculate "); + if (cmac_cb.round == 0) { + SMP_TRACE_ERROR("%s round is 0", __func__); + return FALSE; + } + while (i <= cmac_cb.round) { smp_xor_128(&cmac_cb.text[(cmac_cb.round - i)*BT_OCTET16_LEN], x); /* Mi' := Mi (+) X */ @@ -304,6 +309,11 @@ BOOLEAN aes_cipher_msg_auth_code(BT_OCTET16 key, UINT8 *input, UINT16 length, SMP_TRACE_EVENT ("%s", __func__); + if (tlen == 0 || tlen > BT_OCTET16_LEN) { + SMP_TRACE_ERROR("%s invalid tlen=%d", __func__, tlen); + return FALSE; + } + #if (SMP_CRYPTO_MBEDTLS == TRUE) { /* diff --git a/components/bt/host/bluedroid/stack/smp/smp_keys.c b/components/bt/host/bluedroid/stack/smp/smp_keys.c index 30c602b4b1..0ed04c0b33 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_keys.c +++ b/components/bt/host/bluedroid/stack/smp/smp_keys.c @@ -672,10 +672,10 @@ BOOLEAN smp_gen_p1_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p1) ** Description Generate Confirm/Compare Step2: ** p2 = padding || ia || ra ** -** Returns void +** Returns FALSE if remote address unavailable, TRUE otherwise ** *******************************************************************************/ -void smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2) +BOOLEAN smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2) { UINT8 *p = (UINT8 *)p2; BD_ADDR remote_bda; @@ -683,7 +683,7 @@ void smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2) SMP_TRACE_DEBUG ("smp_gen_p2_4_confirm\n"); if (!BTM_ReadRemoteConnectionAddr(p_cb->pairing_bda, remote_bda, &addr_type)) { SMP_TRACE_ERROR("can not generate confirm p2 for unknown device\n"); - return; + return FALSE; } SMP_TRACE_DEBUG ("smp_gen_p2_4_confirm\n"); @@ -705,6 +705,7 @@ void smp_gen_p2_4_confirm( tSMP_CB *p_cb, BT_OCTET16 p2) SMP_TRACE_DEBUG("p2 = padding || ia || ra"); smp_debug_print_nbyte_little_endian(p2, (const UINT8 *)"p2", 16); #endif + return TRUE; } /******************************************************************************* @@ -768,7 +769,10 @@ static void smp_calculate_comfirm_cont(tSMP_CB *p_cb, tSMP_ENC *p) smp_debug_print_nbyte_little_endian (p->param_buf, (const UINT8 *)"C1", 16); #endif - smp_gen_p2_4_confirm(p_cb, p2); + if (!smp_gen_p2_4_confirm(p_cb, p2)) { + smp_sm_event(p_cb, SMP_AUTH_CMPL_EVT, &status); + return; + } /* calculate p2 = (p1' XOR p2) */ smp_xor_128(p2, p->param_buf); @@ -1217,6 +1221,7 @@ void smp_process_private_key(tSMP_CB *p_cb) UINT8 priv_be[BT_OCTET32_LEN]; UINT8 pub_be[BT_OCTET32_LEN + BT_OCTET32_LEN + 1]; /* 0x04 || X (32 bytes) || Y (32 bytes) */ size_t pub_len = 0; + BOOLEAN psa_ok = FALSE; /* Convert private key from little-endian to big-endian */ for (int i = 0; i < BT_OCTET32_LEN; i++) { @@ -1250,11 +1255,16 @@ void smp_process_private_key(tSMP_CB *p_cb) p_cb->loc_publ_key.x[i] = pub_be[1 + BT_OCTET32_LEN - 1 - i]; p_cb->loc_publ_key.y[i] = pub_be[33 + BT_OCTET32_LEN - 1 - i]; } + psa_ok = TRUE; psa_pubkey_cleanup: psa_destroy_key(key_id); /* Clear sensitive data from stack */ memset(priv_be, 0, sizeof(priv_be)); + memset(pub_be, 0, sizeof(pub_be)); + if (!psa_ok) { + return; + } #elif (SMP_CRYPTO_TINYCRYPT == TRUE) { UINT8 pub_key[64]; /* TinyCrypt format: X (32 bytes) || Y (32 bytes), no prefix */ @@ -1315,10 +1325,10 @@ psa_pubkey_cleanup: ** key and peer public key; ** - saves the new public key x-coordinate as DHKey. ** -** Returns void +** Returns TRUE if DHKey was computed successfully, FALSE otherwise. ** *******************************************************************************/ -void smp_compute_dhkey (tSMP_CB *p_cb) +BOOLEAN smp_compute_dhkey (tSMP_CB *p_cb) { SMP_TRACE_DEBUG ("%s\n", __FUNCTION__); @@ -1330,6 +1340,7 @@ void smp_compute_dhkey (tSMP_CB *p_cb) UINT8 peer_pub_be[BT_OCTET32_LEN + BT_OCTET32_LEN + 1]; /* 0x04 || X (32 bytes) || Y (32 bytes) */ UINT8 shared_secret[BT_OCTET32_LEN]; size_t output_len = 0; + BOOLEAN psa_ok = FALSE; /* Convert private key from little-endian to big-endian */ for (int i = 0; i < BT_OCTET32_LEN; i++) { @@ -1369,12 +1380,17 @@ void smp_compute_dhkey (tSMP_CB *p_cb) for (int i = 0; i < BT_OCTET32_LEN; i++) { p_cb->dhkey[i] = shared_secret[BT_OCTET32_LEN - 1 - i]; } + psa_ok = TRUE; psa_dhkey_cleanup: psa_destroy_key(key_id); /* Clear sensitive data from stack */ memset(priv_be, 0, sizeof(priv_be)); + memset(peer_pub_be, 0, sizeof(peer_pub_be)); memset(shared_secret, 0, sizeof(shared_secret)); + if (!psa_ok) { + return FALSE; + } #elif (SMP_CRYPTO_TINYCRYPT == TRUE) { UINT8 priv_be[BT_OCTET32_LEN]; @@ -1395,11 +1411,11 @@ psa_dhkey_cleanup: /* Validate peer public key */ /* uECC_valid_public_key returns 0 if valid, negative value if invalid */ - if (uECC_valid_public_key(peer_pub_be, uECC_secp256r1()) < 0) { + if (uECC_valid_public_key(peer_pub_be, uECC_secp256r1()) != 0) { SMP_TRACE_ERROR("%s Invalid peer public key\n", __FUNCTION__); memset(priv_be, 0, sizeof(priv_be)); memset(peer_pub_be, 0, sizeof(peer_pub_be)); - return; + return FALSE; } /* Compute ECDH shared secret */ @@ -1409,7 +1425,7 @@ psa_dhkey_cleanup: memset(priv_be, 0, sizeof(priv_be)); memset(peer_pub_be, 0, sizeof(peer_pub_be)); memset(shared_secret, 0, sizeof(shared_secret)); - return; + return FALSE; } /* Convert shared secret from big-endian to little-endian for DHKey */ @@ -1430,9 +1446,16 @@ psa_dhkey_cleanup: memcpy(peer_publ_key.x, p_cb->peer_publ_key.x, BT_OCTET32_LEN); memcpy(peer_publ_key.y, p_cb->peer_publ_key.y, BT_OCTET32_LEN); + if (!ECC_CheckPointIsInElliCur_P256(&peer_publ_key)) { + SMP_TRACE_ERROR("%s Invalid peer public key\n", __FUNCTION__); + memset(private_key, 0, sizeof(private_key)); + return FALSE; + } + ECC_PointMult(&new_publ_key, &peer_publ_key, (DWORD *) private_key, KEY_LENGTH_DWORDS_P256); memcpy(p_cb->dhkey, new_publ_key.x, BT_OCTET32_LEN); + memset(private_key, 0, sizeof(private_key)); #endif /* SMP_CRYPTO_MBEDTLS */ smp_debug_print_nbyte_little_endian (p_cb->dhkey, (const UINT8 *)"DHKey", @@ -1444,6 +1467,7 @@ psa_dhkey_cleanup: BT_OCTET32_LEN); smp_debug_print_nbyte_little_endian (p_cb->peer_publ_key.y, (const UINT8 *)"rem public(y)", BT_OCTET32_LEN); + return TRUE; } /******************************************************************************* diff --git a/components/bt/host/bluedroid/stack/smp/smp_l2c.c b/components/bt/host/bluedroid/stack/smp/smp_l2c.c index c109b9d002..9b019ae73f 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_l2c.c +++ b/components/bt/host/bluedroid/stack/smp/smp_l2c.c @@ -118,8 +118,8 @@ static void smp_connect_callback (UINT16 channel, BD_ADDR bd_addr, BOOLEAN conne if (memcmp(bd_addr, p_cb->pairing_bda, BD_ADDR_LEN) == 0) { SMP_TRACE_EVENT ("%s() for pairing BDA: %08x%04x Event: %s\n", __FUNCTION__, - (bd_addr[0] << 24) + (bd_addr[1] << 16) + (bd_addr[2] << 8) + bd_addr[3], - (bd_addr[4] << 8) + bd_addr[5], + ((UINT32)bd_addr[0] << 24) + ((UINT32)bd_addr[1] << 16) + ((UINT32)bd_addr[2] << 8) + bd_addr[3], + ((UINT32)bd_addr[4] << 8) + bd_addr[5], (connected) ? "connected" : "disconnected"); if (connected) { @@ -283,8 +283,8 @@ static void smp_br_connect_callback(UINT16 channel, BD_ADDR bd_addr, BOOLEAN con SMP_TRACE_EVENT ("%s for pairing BDA: %08x%04x Event: %s\n", __func__, - (bd_addr[0] << 24) + (bd_addr[1] << 16) + (bd_addr[2] << 8) + bd_addr[3], - (bd_addr[4] << 8) + bd_addr[5], + ((UINT32)bd_addr[0] << 24) + ((UINT32)bd_addr[1] << 16) + ((UINT32)bd_addr[2] << 8) + bd_addr[3], + ((UINT32)bd_addr[4] << 8) + bd_addr[5], (connected) ? "connected" : "disconnected"); if (connected) { diff --git a/components/bt/host/bluedroid/stack/smp/smp_main.c b/components/bt/host/bluedroid/stack/smp/smp_main.c index fa4d5ffbe3..fd9bc0c6d7 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_main.c +++ b/components/bt/host/bluedroid/stack/smp/smp_main.c @@ -746,7 +746,7 @@ void smp_sm_event(tSMP_CB *p_cb, tSMP_EVENT event, void *p_data) /* lookup entry /w event & curr_state */ /* If entry is ignore, return. * Otherwise, get state table (according to curr_state or all_state) */ - if ((event <= SMP_MAX_EVT) && ( (entry = entry_table[event - 1][curr_state]) != SMP_SM_IGNORE )) { + if ((event >= 1 && event <= SMP_MAX_EVT) && ( (entry = entry_table[event - 1][curr_state]) != SMP_SM_IGNORE )) { if (entry & SMP_ALL_TBL_MASK) { entry &= ~SMP_ALL_TBL_MASK; state_table = smp_all_table; @@ -803,7 +803,7 @@ const char *smp_get_event_name(tSMP_EVENT event) { const char *p_str = smp_event_name[SMP_MAX_EVT]; - if (event <= SMP_MAX_EVT) { + if (event >= 1 && event <= SMP_MAX_EVT) { p_str = smp_event_name[event - 1]; } return p_str; diff --git a/components/bt/host/bluedroid/stack/smp/smp_utils.c b/components/bt/host/bluedroid/stack/smp/smp_utils.c index 1660d78fb1..0276f8b7e2 100644 --- a/components/bt/host/bluedroid/stack/smp/smp_utils.c +++ b/components/bt/host/bluedroid/stack/smp/smp_utils.c @@ -351,7 +351,7 @@ BOOLEAN smp_send_cmd(UINT8 cmd_code, tSMP_CB *p_cb) BOOLEAN sent = FALSE; UINT8 failure = SMP_PAIR_INTERNAL_ERR; SMP_TRACE_EVENT("smp_send_cmd on l2cap cmd_code=0x%x\n", cmd_code); - if ( cmd_code <= (SMP_OPCODE_MAX + 1 /* for SMP_OPCODE_PAIR_COMMITM */) && + if ( cmd_code < SMP_OPCODE_ARRAY_SIZE && smp_cmd_build_act[cmd_code] != NULL) { p_buf = (*smp_cmd_build_act[cmd_code])(cmd_code, p_cb); @@ -861,6 +861,11 @@ void smp_convert_string_to_tk(BT_OCTET16 tk, UINT32 passkey) void smp_mask_enc_key(UINT8 loc_enc_size, UINT8 *p_data) { SMP_TRACE_EVENT("smp_mask_enc_key\n"); + if (loc_enc_size < SMP_ENCR_KEY_SIZE_MIN) { + SMP_TRACE_ERROR("smp_mask_enc_key: loc_enc_size %d below minimum %d\n", + loc_enc_size, SMP_ENCR_KEY_SIZE_MIN); + return; + } if (loc_enc_size < BT_OCTET16_LEN) { for (; loc_enc_size < BT_OCTET16_LEN; loc_enc_size ++) { * (p_data + loc_enc_size) = 0; @@ -1060,7 +1065,7 @@ BOOLEAN smp_command_has_invalid_parameters(tSMP_CB *p_cb) SMP_TRACE_DEBUG("%s for cmd code 0x%02x\n", __func__, cmd_code); - if ((cmd_code > (SMP_OPCODE_MAX + 1 /* for SMP_OPCODE_PAIR_COMMITM */)) || + if ((cmd_code >= SMP_OPCODE_ARRAY_SIZE) || (cmd_code < SMP_OPCODE_MIN)) { SMP_TRACE_WARNING("Somehow received command with the RESERVED code 0x%02x\n", cmd_code); return TRUE;