From 131f55f91fa06cb6feec2ffa8bdd734811d53ba0 Mon Sep 17 00:00:00 2001 From: Zhi Wei Jian Date: Wed, 25 Mar 2026 10:27:39 +0800 Subject: [PATCH] fix(ble/bluedroid): fix memory safety and state issues in BTA GATT layer - Fix use-after-free and double-free in bta_gattc_update_include_service - Fix heap buffer overflow in GATT database operations - Fix GATTC cache load attr length check and NVS handle leak - Fix parameter validation in bta_gattc_uuid_compare - Ensure all CLCBs are cleaned up on deregister - Remove unused bta_gattc_open_error - Unify GATT db count/fill by declaration handle range - Fix return status in gatts_set_attribute_value (cherry picked from commit d4f3517da4a81144eaa3f091848e61ec68ab3700) Co-authored-by: zhiweijian --- .../host/bluedroid/bta/gatt/bta_gatt_common.c | 4 + .../host/bluedroid/bta/gatt/bta_gattc_act.c | 170 ++++++++------ .../host/bluedroid/bta/gatt/bta_gattc_api.c | 27 ++- .../host/bluedroid/bta/gatt/bta_gattc_cache.c | 222 +++++++++++------- .../bt/host/bluedroid/bta/gatt/bta_gattc_co.c | 64 +++-- .../host/bluedroid/bta/gatt/bta_gattc_main.c | 7 +- .../host/bluedroid/bta/gatt/bta_gattc_utils.c | 73 ++---- .../host/bluedroid/bta/gatt/bta_gatts_act.c | 11 +- .../host/bluedroid/bta/gatt/bta_gatts_api.c | 2 + .../host/bluedroid/bta/gatt/bta_gatts_main.c | 8 + .../host/bluedroid/bta/gatt/bta_gatts_utils.c | 67 +----- .../bta/gatt/include/bta_gattc_int.h | 7 +- .../bta/gatt/include/bta_gatts_int.h | 1 - 13 files changed, 342 insertions(+), 321 deletions(-) diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gatt_common.c b/components/bt/host/bluedroid/bta/gatt/bta_gatt_common.c index 81cee87767..2b8ecd72c9 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gatt_common.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gatt_common.c @@ -16,6 +16,10 @@ void BTA_GATT_SetLocalMTU(uint16_t mtu) { + if (mtu < GATT_DEF_BLE_MTU_SIZE || mtu > GATT_MAX_MTU_SIZE) { + APPL_TRACE_ERROR("%s: invalid mtu=%u", __func__, mtu); + return; + } gatt_set_local_mtu(mtu); } diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gattc_act.c b/components/bt/host/bluedroid/bta/gatt/bta_gattc_act.c index e808dc9f54..61983b3c4c 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_act.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_act.c @@ -38,10 +38,6 @@ #include "bta_hh_int.h" #include "btm_int.h" -#if (defined BTA_HH_LE_INCLUDED && BTA_HH_LE_INCLUDED == TRUE) -#include "bta_hh_int.h" -#endif - // #include "btif/include/btif_debug_conn.h" #include @@ -290,12 +286,21 @@ void bta_gattc_deregister(tBTA_GATTC_CB *p_cb, tBTA_GATTC_RCB *p_clreg) /* remove bg connection associated with this rcb */ for (i = 0; i < BTA_GATTC_KNOWN_SR_MAX; i ++) { if (p_cb->bg_track[i].in_use) { - if (p_cb->bg_track[i].cif_mask & (1 << (p_clreg->client_if - 1))) { - bta_gattc_mark_bg_conn(p_clreg->client_if, p_cb->bg_track[i].remote_bda, FALSE, FALSE); - GATT_CancelConnect(p_clreg->client_if, p_cb->bg_track[i].remote_bda, FALSE); + if (p_cb->bg_track[i].cif_mask & BTA_GATTC_CIF_MASK_BIT(p_clreg->client_if)) { + /* bta_gattc_mark_bg_conn is called to remove the application's bit from the mask. + If this was the last application interested in the device, + bta_gattc_mark_bg_conn zeroes out the entire track entry using memset + */ + BD_ADDR remote_bda; + bdcpy(remote_bda, p_cb->bg_track[i].remote_bda); + bta_gattc_mark_bg_conn(p_clreg->client_if, remote_bda, FALSE, FALSE); + GATT_CancelConnect(p_clreg->client_if, remote_bda, FALSE); } - if (p_cb->bg_track[i].cif_adv_mask & (1 << (p_clreg->client_if - 1))) { - bta_gattc_mark_bg_conn(p_clreg->client_if, p_cb->bg_track[i].remote_bda, FALSE, TRUE); + if (p_cb->bg_track[i].cif_adv_mask & BTA_GATTC_CIF_MASK_BIT(p_clreg->client_if)) { + BD_ADDR remote_bda_adv; + bdcpy(remote_bda_adv, p_cb->bg_track[i].remote_bda); + bta_gattc_mark_bg_conn(p_clreg->client_if, remote_bda_adv, FALSE, TRUE); + GATT_CancelConnect(p_clreg->client_if, remote_bda_adv, FALSE); } } } @@ -450,28 +455,6 @@ void bta_gattc_cancel_open_error(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_dat } } -/******************************************************************************* -** -** Function bta_gattc_open_error -** -** Description -** -** Returns void -** -*******************************************************************************/ -void bta_gattc_open_error(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) -{ - UNUSED(p_data); - - APPL_TRACE_ERROR("Connection already opened. wrong state"); - - bta_gattc_send_open_cback(p_clcb->p_rcb, - BTA_GATT_OK, - p_clcb->bda, - p_clcb->bta_conn_id, - p_clcb->transport, - 0); -} /******************************************************************************* ** ** Function bta_gattc_open_fail @@ -713,6 +696,18 @@ void bta_gattc_cancel_open(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) void bta_gattc_conn(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) { tBTA_GATTC_IF gatt_if; + if (p_clcb->p_srcb == NULL) { + APPL_TRACE_ERROR("%s, p_clcb->p_srcb is NULL", __func__); + if (p_clcb->p_rcb) { + bta_gattc_send_open_cback(p_clcb->p_rcb, + BTA_GATT_ERROR, + p_clcb->bda, + p_clcb->bta_conn_id, + p_clcb->transport, + GATT_DEF_BLE_MTU_SIZE); + } + return; + } APPL_TRACE_DEBUG("bta_gattc_conn server cache state=%d", p_clcb->p_srcb->state); if (p_data != NULL) { @@ -861,6 +856,10 @@ void bta_gattc_close_fail(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) *******************************************************************************/ void bta_gattc_close(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) { + if (!p_clcb || !p_clcb->p_rcb) { + APPL_TRACE_ERROR("%s, p_clcb or p_clcb->p_rcb is NULL", __func__); + return; + } tBTA_GATTC_CBACK *p_cback = p_clcb->p_rcb->p_cback; tBTA_GATTC_RCB *p_clreg = p_clcb->p_rcb; tBTA_GATTC cb_data; @@ -1096,6 +1095,11 @@ void bta_gattc_disc_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) APPL_TRACE_DEBUG("bta_gattc_disc_cmpl conn_id=%d, status = %d", p_clcb->bta_conn_id, p_clcb->status); + if (p_clcb->p_srcb == NULL) { + APPL_TRACE_ERROR("%s, p_clcb->p_srcb is NULL", __func__); + return; + } + p_clcb->p_srcb->state = BTA_GATTC_SERV_IDLE; p_clcb->disc_active = FALSE; @@ -1214,16 +1218,13 @@ void bta_gattc_read_multi(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) if (bta_gattc_enqueue(p_clcb, p_data)) { memset(&read_param, 0, sizeof(tGATT_READ_PARAM)); - if (status == BTA_GATT_OK) { - read_param.read_multiple.num_handles = p_data->api_read_multi.num_attr; - read_param.read_multiple.auth_req = p_data->api_read_multi.auth_req; - memcpy(&read_param.read_multiple.handles, p_data->api_read_multi.handles, - sizeof(UINT16) * p_data->api_read_multi.num_attr); + read_param.read_multiple.num_handles = p_data->api_read_multi.num_attr; + read_param.read_multiple.auth_req = p_data->api_read_multi.auth_req; + memcpy(&read_param.read_multiple.handles, p_data->api_read_multi.handles, + sizeof(UINT16) * p_data->api_read_multi.num_attr); - status = GATTC_Read(p_clcb->bta_conn_id, GATT_READ_MULTIPLE, &read_param); - } + status = GATTC_Read(p_clcb->bta_conn_id, GATT_READ_MULTIPLE, &read_param); - /* read fail */ if (status != BTA_GATT_OK) { bta_gattc_cmpl_sendmsg(p_clcb->bta_conn_id, GATTC_OPTYPE_READ, status, NULL); } @@ -1245,16 +1246,13 @@ void bta_gattc_read_multi_var(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) if (bta_gattc_enqueue(p_clcb, p_data)) { memset(&read_param, 0, sizeof(tGATT_READ_PARAM)); - if (status == BTA_GATT_OK) { - read_param.read_multiple.num_handles = p_data->api_read_multi.num_attr; - read_param.read_multiple.auth_req = p_data->api_read_multi.auth_req; - memcpy(&read_param.read_multiple.handles, p_data->api_read_multi.handles, - sizeof(UINT16) * p_data->api_read_multi.num_attr); + read_param.read_multiple.num_handles = p_data->api_read_multi.num_attr; + read_param.read_multiple.auth_req = p_data->api_read_multi.auth_req; + memcpy(&read_param.read_multiple.handles, p_data->api_read_multi.handles, + sizeof(UINT16) * p_data->api_read_multi.num_attr); - status = GATTC_Read(p_clcb->bta_conn_id, GATT_READ_MULTIPLE_VAR, &read_param); - } + status = GATTC_Read(p_clcb->bta_conn_id, GATT_READ_MULTIPLE_VAR, &read_param); - /* read fail */ if (status != BTA_GATT_OK) { bta_gattc_cmpl_sendmsg(p_clcb->bta_conn_id, GATTC_OPTYPE_READ, status, NULL); } @@ -1364,6 +1362,11 @@ void bta_gattc_read_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) tBTA_GATTC cb_data; tBTA_GATT_UNFMT read_value; + if (p_clcb->p_q_cmd == NULL) { + APPL_TRACE_ERROR("%s, p_clcb->p_q_cmd is NULL", __func__); + return; + } + memset(&cb_data, 0, sizeof(tBTA_GATTC)); memset(&read_value, 0, sizeof(tBTA_GATT_UNFMT)); @@ -1406,6 +1409,18 @@ void bta_gattc_write_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) { tBTA_GATTC cb_data = {0}; UINT8 event; + if (p_data->p_cmpl == NULL) { + APPL_TRACE_ERROR("%s, p_data->p_cmpl is NULL", __func__); + UINT16 handle = p_clcb->p_q_cmd->api_write.handle; + tBTA_GATTC_EVT cmpl_evt = p_clcb->p_q_cmd->api_write.cmpl_evt; + bta_gattc_free_command_data(p_clcb); + bta_gattc_pop_command_to_send(p_clcb); + cb_data.write.status = BTA_GATT_ERROR; + cb_data.write.handle = handle; + cb_data.write.conn_id = p_clcb->bta_conn_id; + ( *p_clcb->p_rcb->p_cback)(cmpl_evt, (tBTA_GATTC *)&cb_data); + return; + } tBTA_GATTC_CONN *p_conn = bta_gattc_conn_find(p_clcb->bda); memset(&cb_data, 0, sizeof(tBTA_GATTC)); @@ -1414,6 +1429,17 @@ void bta_gattc_write_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) cb_data.write.handle = p_data->p_cmpl->att_value.handle; if (p_clcb->p_q_cmd->api_write.hdr.event == BTA_GATTC_API_WRITE_EVT && p_clcb->p_q_cmd->api_write.write_type == BTA_GATTC_WRITE_PREPARE) { + // Check if the parameters are valid + // should not happen, but just in case + if (p_clcb->p_q_cmd->api_write.p_value == NULL) { + APPL_TRACE_ERROR("%s, p_clcb->p_q_cmd->api_write.p_value is NULL", __func__); + bta_gattc_free_command_data(p_clcb); + bta_gattc_pop_command_to_send(p_clcb); + cb_data.write.status = BTA_GATT_ERROR; + cb_data.write.conn_id = p_clcb->bta_conn_id; + ( *p_clcb->p_rcb->p_cback)(BTA_GATTC_PREP_WRITE_EVT, (tBTA_GATTC *)&cb_data); + return; + } // Should check the value received from the peer device is correct or not. if (memcmp(p_clcb->p_q_cmd->api_write.p_value, p_data->p_cmpl->att_value.value, p_data->p_cmpl->att_value.len) != 0) { @@ -1509,6 +1535,13 @@ void bta_gattc_op_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) APPL_TRACE_DEBUG("bta_gattc_op_cmpl op = %d", op); + // Check if the operation is valid + // should not happen, but just in case + if (op > GATTC_OPTYPE_INDICATION) { + APPL_TRACE_ERROR("unexpected operation, ignored"); + return; + } + if (op == GATTC_OPTYPE_INDICATION || op == GATTC_OPTYPE_NOTIFICATION) { APPL_TRACE_ERROR("unexpected operation, ignored"); } else if (op >= GATTC_OPTYPE_READ) { @@ -1893,49 +1926,30 @@ void bta_gattc_process_api_cache_assoc(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_m { tBTA_GATTC gattc_cb = {0}; gattc_cb.set_assoc.client_if = p_msg->api_assoc.client_if; + gattc_cb.set_assoc.status = BTA_GATT_OK; BOOLEAN state = FALSE; tBTA_GATTC_CLCB *p_assoc_clcb = bta_gattc_find_clcb_by_cif(p_msg->api_assoc.client_if, p_msg->api_assoc.assoc_addr, BTA_TRANSPORT_LE); tBTA_GATTC_RCB *p_clrcb = bta_gattc_cl_get_regcb(p_msg->api_assoc.client_if); - if (p_assoc_clcb != NULL) { - if (p_assoc_clcb->state == BTA_GATTC_CONN_ST || p_assoc_clcb->state == BTA_GATTC_DISCOVER_ST) { - gattc_cb.set_assoc.status = BTA_GATT_BUSY; - if (p_clrcb != NULL) { - (*p_clrcb->p_cback)(BTA_GATTC_ASSOC_EVT, &gattc_cb); - return; - } - } - } - if (p_msg->api_assoc.is_assoc) { - if ((state = bta_gattc_co_cache_append_assoc_addr(p_msg->api_assoc.src_addr, p_msg->api_assoc.assoc_addr)) == TRUE) { - gattc_cb.set_assoc.status = BTA_GATT_OK; - - } else { + if (p_assoc_clcb != NULL && + (p_assoc_clcb->state == BTA_GATTC_CONN_ST || p_assoc_clcb->state == BTA_GATTC_DISCOVER_ST)) { + gattc_cb.set_assoc.status = BTA_GATT_BUSY; + } else if (p_msg->api_assoc.is_assoc) { + state = bta_gattc_co_cache_append_assoc_addr(p_msg->api_assoc.src_addr, p_msg->api_assoc.assoc_addr); + if (!state) { gattc_cb.set_assoc.status = BTA_GATT_ERROR; - if (p_clrcb != NULL) { - (*p_clrcb->p_cback)(BTA_GATTC_ASSOC_EVT, &gattc_cb); - return; - } } } else { - if (( state = bta_gattc_co_cache_remove_assoc_addr(p_msg->api_assoc.src_addr, p_msg->api_assoc.assoc_addr)) == TRUE) { - gattc_cb.set_assoc.status = BTA_GATT_OK; - } else { + state = bta_gattc_co_cache_remove_assoc_addr(p_msg->api_assoc.src_addr, p_msg->api_assoc.assoc_addr); + if (!state) { gattc_cb.set_assoc.status = BTA_GATT_ERROR; - if (p_clrcb != NULL) { - (*p_clrcb->p_cback)(BTA_GATTC_ASSOC_EVT, &gattc_cb); - return; - } } } - if (p_clrcb != NULL) { + if (p_clrcb != NULL && p_clrcb->p_cback != NULL) { (*p_clrcb->p_cback)(BTA_GATTC_ASSOC_EVT, &gattc_cb); } - - return; - } void bta_gattc_process_api_cache_get_addr_list(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_msg) { @@ -1957,7 +1971,7 @@ void bta_gattc_process_api_cache_get_addr_list(tBTA_GATTC_CB *p_cb, tBTA_GATTC_D gattc_cb.get_addr_list.status = BTA_GATT_NOT_FOUND; } - if (p_clrcb != NULL) { + if (p_clrcb != NULL && p_clrcb->p_cback != NULL) { (* p_clrcb->p_cback)(BTA_GATTC_GET_ADDR_LIST_EVT, &gattc_cb); } @@ -2363,7 +2377,7 @@ tBTA_GATTC_FIND_SERVICE_CB bta_gattc_register_service_change_notify(UINT16 conn_ tBTA_GATTC_SERVICE *p_service = NULL; tBTA_GATTC_CHARACTERISTIC *p_char = NULL; tBTA_GATTC_DESCRIPTOR *p_desc = NULL; - tBTA_GATTC_FIND_SERVICE_CB result; + tBTA_GATTC_FIND_SERVICE_CB result = SERVICE_CHANGE_CACHE_NOT_FOUND; BOOLEAN gatt_cache_found = FALSE; BOOLEAN gatt_service_found = FALSE; BOOLEAN gatt_service_change_found = FALSE; diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gattc_api.c b/components/bt/host/bluedroid/bta/gatt/bta_gattc_api.c index 69738647ba..29df677274 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_api.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_api.c @@ -151,6 +151,7 @@ void BTA_GATTC_Enh_Open(tBTA_GATTC_IF client_if, BD_ADDR remote_bda, tBTA_ADDR_T tBTA_GATTC_API_OPEN *p_buf; if ((p_buf = (tBTA_GATTC_API_OPEN *) osi_malloc(sizeof(tBTA_GATTC_API_OPEN))) != NULL) { + memset(p_buf, 0, sizeof(tBTA_GATTC_API_OPEN)); p_buf->hdr.event = BTA_GATTC_API_OPEN_EVT; p_buf->client_if = client_if; @@ -365,7 +366,7 @@ void BTA_GATTC_GetServiceWithUUID(UINT16 conn_id, tBT_UUID *svc_uuid, void BTA_GATTC_GetAllChar(UINT16 conn_id, UINT16 start_handle, UINT16 end_handle, btgatt_db_element_t **db, UINT16 *count) { - bta_gattc_get_db_with_opration(conn_id, + bta_gattc_get_db_with_operation(conn_id, GATT_OP_GET_ALL_CHAR, 0, NULL, @@ -380,7 +381,7 @@ void BTA_GATTC_GetAllChar(UINT16 conn_id, UINT16 start_handle, UINT16 end_handle void BTA_GATTC_GetAllDescriptor(UINT16 conn_id, UINT16 char_handle, btgatt_db_element_t **db, UINT16 *count) { - bta_gattc_get_db_with_opration(conn_id, + bta_gattc_get_db_with_operation(conn_id, GATT_OP_GET_ALL_DESCRI, char_handle, NULL, @@ -395,7 +396,7 @@ void BTA_GATTC_GetAllDescriptor(UINT16 conn_id, UINT16 char_handle, void BTA_GATTC_GetCharByUUID(UINT16 conn_id, UINT16 start_handle, UINT16 end_handle, tBT_UUID char_uuid, btgatt_db_element_t **db, UINT16 *count) { - bta_gattc_get_db_with_opration(conn_id, + bta_gattc_get_db_with_operation(conn_id, GATT_OP_GET_CHAR_BY_UUID, 0, NULL, @@ -411,7 +412,7 @@ void BTA_GATTC_GetDescrByUUID(UINT16 conn_id, uint16_t start_handle, uint16_t en tBT_UUID char_uuid, tBT_UUID descr_uuid, btgatt_db_element_t **db, UINT16 *count) { - bta_gattc_get_db_with_opration(conn_id, + bta_gattc_get_db_with_operation(conn_id, GATT_OP_GET_DESCRI_BY_UUID, 0, NULL, @@ -426,7 +427,7 @@ void BTA_GATTC_GetDescrByUUID(UINT16 conn_id, uint16_t start_handle, uint16_t en void BTA_GATTC_GetDescrByCharHandle(UINT16 conn_id, UINT16 char_handle, tBT_UUID descr_uuid, btgatt_db_element_t **db, UINT16 *count) { - bta_gattc_get_db_with_opration(conn_id, + bta_gattc_get_db_with_operation(conn_id, GATT_OP_GET_DESCRI_BY_HANDLE, char_handle, NULL, @@ -441,7 +442,7 @@ void BTA_GATTC_GetDescrByCharHandle(UINT16 conn_id, UINT16 char_handle, tBT_UUID void BTA_GATTC_GetIncludeService(UINT16 conn_id, UINT16 start_handle, UINT16 end_handle, tBT_UUID *incl_uuid, btgatt_db_element_t **db, UINT16 *count) { - bta_gattc_get_db_with_opration(conn_id, + bta_gattc_get_db_with_operation(conn_id, GATT_OP_GET_INCLUDE_SVC, 0, incl_uuid, @@ -528,9 +529,8 @@ void BTA_GATTC_ReadCharacteristic(UINT16 conn_id, UINT16 handle, tBTA_GATT_AUTH_ void BTA_GATTC_ReadCharDescr (UINT16 conn_id, UINT16 handle, tBTA_GATT_AUTH_REQ auth_req) { tBTA_GATTC_API_READ *p_buf; - UINT16 len = (UINT16)(sizeof(tBTA_GATT_ID) + sizeof(tBTA_GATTC_API_READ)); - if ((p_buf = (tBTA_GATTC_API_READ *) osi_malloc(len)) != NULL) { + if ((p_buf = (tBTA_GATTC_API_READ *) osi_malloc(sizeof(tBTA_GATTC_API_READ))) != NULL) { memset(p_buf, 0, sizeof(tBTA_GATTC_API_READ)); p_buf->hdr.event = BTA_GATTC_API_READ_EVT; @@ -560,6 +560,7 @@ void BTA_GATTC_ReadCharDescr (UINT16 conn_id, UINT16 handle, tBTA_GATT_AUTH_REQ void BTA_GATTC_ReadMultiple(UINT16 conn_id, tBTA_GATTC_MULTI *p_read_multi, tBTA_GATT_AUTH_REQ auth_req) { + // p_read_multi should not be NULL tBTA_GATTC_API_READ_MULTI *p_buf; //tBTA_GATTC_API_READ_MULTI *p_value; UINT16 len = (UINT16)(sizeof(tBTA_GATTC_API_READ_MULTI)); @@ -921,7 +922,7 @@ tBTA_GATT_STATUS BTA_GATTC_RegisterForNotifications (tBTA_GATTC_IF client_if, if (!handle) { - APPL_TRACE_ERROR("deregistration failed, handle is 0"); + APPL_TRACE_ERROR("registration failed, handle is 0"); return status; } @@ -1117,10 +1118,15 @@ typedef struct { } uuid; } __attribute__((packed)) tAPP_UUID; -uint8_t BTA_GATTC_Discover(uint8_t gatt_if, uint16_t conn_id, void *uuid, uint8_t disc_type, uint16_t s_handle, uint16_t e_handle) +int BTA_GATTC_Discover(uint8_t gatt_if, uint16_t conn_id, void *uuid, uint8_t disc_type, uint16_t s_handle, uint16_t e_handle) { tGATT_STATUS status; tGATT_DISC_PARAM param; + + if (uuid == NULL && disc_type == GATT_DISC_SRVC_BY_UUID) { + APPL_TRACE_ERROR("%s invalid parameters", __func__); + return -1; + } tAPP_UUID *app_uuid = (tAPP_UUID *)uuid; conn_id = (UINT16)((((UINT8)conn_id) << 8) | gatt_if); @@ -1144,6 +1150,7 @@ uint8_t BTA_GATTC_Discover(uint8_t gatt_if, uint16_t conn_id, void *uuid, uint8_ memcpy(param.service.uu.uuid128, app_uuid->uuid.uuid128, LEN_UUID_128); } else { APPL_TRACE_ERROR("%s invalid uuid len %u", __func__, app_uuid->len); + return -1; } } diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gattc_cache.c b/components/bt/host/bluedroid/bta/gatt/bta_gattc_cache.c index 7af5e6beee..aebffce50a 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_cache.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_cache.c @@ -40,6 +40,7 @@ #include "stack/l2c_api.h" #include "btm_int.h" #include "errno.h" +#include "gatt_int.h" // #include "osi/include/log.h" @@ -287,6 +288,12 @@ static tBTA_GATT_STATUS bta_gattc_add_srvc_to_cache(tBTA_GATTC_SERV *p_srvc_cb, if(!p_srvc_cb->p_srvc_cache) { APPL_TRACE_WARNING("%s(), no resource.", __func__); + if (p_new_srvc->characteristics) { + list_free(p_new_srvc->characteristics); + } + if (p_new_srvc->included_svc) { + list_free(p_new_srvc->included_svc); + } osi_free(p_new_srvc); return BTA_GATT_NO_RESOURCES; } @@ -336,7 +343,14 @@ static tBTA_GATT_STATUS bta_gattc_add_char_to_cache(tBTA_GATTC_SERV *p_srvc_cb, memcpy(&characteristic->uuid, p_uuid, sizeof(tBT_UUID)); characteristic->service = service; - list_append(service->characteristics, characteristic); + if (!list_append(service->characteristics, characteristic)) { + APPL_TRACE_WARNING("%s(), no resource.", __func__); + if (characteristic->descriptors) { + list_free(characteristic->descriptors); + } + osi_free(characteristic); + return BTA_GATT_NO_RESOURCES; + } return BTA_GATT_OK; } @@ -556,7 +570,8 @@ void bta_gattc_update_include_service(const list_t *services) { for (list_node_t *sn = list_begin(services); sn != list_end(services); sn = list_next(sn)) { tBTA_GATTC_SERVICE *service = list_node(sn); if(!service || !service->included_svc || list_is_empty(service->included_svc)) break; - for (list_node_t *sn = list_begin(service->included_svc); sn != list_end(service->included_svc); sn = list_next(sn)) { + for (list_node_t *sn = list_begin(service->included_svc); sn != list_end(service->included_svc);) { + list_node_t *sn_next = list_next(sn); tBTA_GATTC_INCLUDED_SVC *include_service = list_node(sn); if(include_service && !include_service->included_service) { //update @@ -564,9 +579,10 @@ void bta_gattc_update_include_service(const list_t *services) { if(!include_service->included_service) { //not match, free it list_remove(service->included_svc, include_service); - osi_free(include_service); + // osi_free(include_service); } } + sn = sn_next; } } @@ -594,7 +610,7 @@ static void bta_gattc_explore_srvc(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb) APPL_TRACE_ERROR("unknown connection ID"); return; } - /* start expore a service if there is service not been explored */ + /* start explore a service if there is service not been explored */ if (p_srvc_cb->cur_srvc_idx < p_srvc_cb->total_srvc) { /* add the first service into cache */ if (bta_gattc_add_srvc_to_cache (p_srvc_cb, @@ -817,7 +833,7 @@ static tBTA_GATT_STATUS bta_gattc_add_char_to_list(tBTA_GATTC_SERV *p_srvc_cb, p_rec->e_handle = (p_srvc_cb->p_srvc_list + p_srvc_cb->cur_srvc_idx)->e_handle; memcpy(&p_rec->uuid, &uuid, sizeof(tBT_UUID)); - /* update the endind handle of pervious characteristic if available */ + /* update the endind handle of previous characteristic if available */ if (p_srvc_cb->total_char > 1) { p_rec -= 1; p_rec->e_handle = decl_handle - 1; @@ -1319,7 +1335,7 @@ void bta_gattc_fill_gatt_db_el(btgatt_db_element_t *p_attr, bta_to_btif_uuid(&p_attr->uuid, &uuid); } -void bta_gattc_get_db_with_opration(UINT16 conn_id, +void bta_gattc_get_db_with_operation(UINT16 conn_id, bt_gatt_get_db_op_t op, UINT16 char_handle, tBT_UUID *incl_uuid, @@ -1502,10 +1518,9 @@ static size_t bta_gattc_get_db_size_with_type(list_t *services, } size_t db_size = 0; - UINT16 svc_length = list_length(services) - 1; for (list_node_t *sn = list_begin(services); - sn != list_end(services); sn = list_next(sn), svc_length--) { + sn != list_end(services); sn = list_next(sn)) { tBTA_GATTC_SERVICE *p_cur_srvc = list_node(sn); if (p_cur_srvc->e_handle < start_handle) { @@ -1519,12 +1534,8 @@ static size_t bta_gattc_get_db_size_with_type(list_t *services, if (type == BTGATT_DB_PRIMARY_SERVICE || type == BTGATT_DB_SECONDARY_SERVICE) { if ((type == BTGATT_DB_PRIMARY_SERVICE && p_cur_srvc->is_primary) || (type == BTGATT_DB_SECONDARY_SERVICE && !p_cur_srvc->is_primary)) { - // if the current service is the last service in the db, need to ensure the current service start handle is not less than the start_handle. - if (!svc_length) { - if (p_cur_srvc->s_handle >= start_handle) { - db_size++; - } - } else { + /* Count only when declaration handle s_handle is within [start_handle, end_handle] (GATT spec). */ + if (p_cur_srvc->s_handle >= start_handle) { db_size++; } } @@ -1620,9 +1631,8 @@ static size_t bta_gattc_get_db_size(list_t *services, } size_t db_size = 0; - UINT16 svc_length = list_length(services) - 1; for (list_node_t *sn = list_begin(services); - sn != list_end(services); sn = list_next(sn), svc_length--) { + sn != list_end(services); sn = list_next(sn)) { tBTA_GATTC_SERVICE *p_cur_srvc = list_node(sn); if (p_cur_srvc->e_handle < start_handle) { @@ -1633,41 +1643,38 @@ static size_t bta_gattc_get_db_size(list_t *services, break; } - if (!svc_length) { - if (p_cur_srvc->s_handle >= start_handle) { - db_size++; - } - } else { - db_size++; - } - - if (!p_cur_srvc->characteristics || list_is_empty(p_cur_srvc->characteristics)) { + /* Count service only when declaration handle s_handle is within [start_handle, end_handle] (GATT spec). + * Skip counting this service and all its contents when s_handle < start_handle (consistent with bta_gattc_get_gatt_db_impl). */ + if (p_cur_srvc->s_handle < start_handle) { continue; } + db_size++; - for (list_node_t *cn = list_begin(p_cur_srvc->characteristics); - cn != list_end(p_cur_srvc->characteristics); cn = list_next(cn)) { - tBTA_GATTC_CHARACTERISTIC *p_char = list_node(cn); + if (p_cur_srvc->characteristics && !list_is_empty(p_cur_srvc->characteristics)) { + for (list_node_t *cn = list_begin(p_cur_srvc->characteristics); + cn != list_end(p_cur_srvc->characteristics); cn = list_next(cn)) { + tBTA_GATTC_CHARACTERISTIC *p_char = list_node(cn); - if (p_char->handle < start_handle) { - continue; - } - if (p_char->handle > end_handle) { - return db_size; - } - db_size++; + if (p_char->handle < start_handle) { + continue; + } + if (p_char->handle > end_handle) { + return db_size; + } + db_size++; - if (p_char->descriptors) { - for (list_node_t *dn = list_begin(p_char->descriptors); - dn != list_end(p_char->descriptors); dn = list_next(dn)) { - tBTA_GATTC_DESCRIPTOR *p_desc = list_node(dn); - if (p_desc->handle < start_handle) { - continue; + if (p_char->descriptors) { + for (list_node_t *dn = list_begin(p_char->descriptors); + dn != list_end(p_char->descriptors); dn = list_next(dn)) { + tBTA_GATTC_DESCRIPTOR *p_desc = list_node(dn); + if (p_desc->handle < start_handle) { + continue; + } + if (p_desc->handle > end_handle) { + return db_size; + } + db_size++; } - if (p_desc->handle > end_handle) { - return db_size; - } - db_size++; } } } @@ -1702,7 +1709,7 @@ void bta_gattc_get_db_size_handle(UINT16 conn_id, UINT16 start_handle, UINT16 en } tBTA_GATTC_SERV *p_srcb = p_clcb->p_srcb; - if (!p_srcb->p_srvc_cache || list_is_empty(p_srcb->p_srvc_cache)) { + if ((p_srcb == NULL) || !p_srcb->p_srvc_cache || list_is_empty(p_srcb->p_srvc_cache)) { *count = 0; return; } @@ -1792,6 +1799,11 @@ static void bta_gattc_get_gatt_db_impl(tBTA_GATTC_SERV *p_srvc_cb, break; } + /* Output service only when declaration handle s_handle is within [start_handle, end_handle] (consistent with count logic). */ + if (p_cur_srvc->s_handle < start_handle) { + continue; + } + bta_gattc_fill_gatt_db_el(curr_db_attr, p_cur_srvc->is_primary ? BTGATT_DB_PRIMARY_SERVICE : @@ -1879,8 +1891,8 @@ static void bta_gattc_get_gatt_db_impl(tBTA_GATTC_SERV *p_srvc_cb, bta_gattc_fill_gatt_db_el(curr_db_attr, BTGATT_DB_INCLUDED_SERVICE, p_isvc->handle, - 0 /* s_handle */, - 0 /* e_handle */, + p_isvc->incl_srvc_s_handle, + p_isvc->incl_srvc_e_handle, p_isvc->handle, p_isvc->uuid, 0 /* property */); @@ -1912,17 +1924,26 @@ void bta_gattc_get_gatt_db(UINT16 conn_id, UINT16 start_handle, UINT16 end_handl if (p_clcb == NULL) { APPL_TRACE_ERROR("Unknown conn ID: %d", conn_id); + *count = 0; + *db = NULL; return; } if (p_clcb->state != BTA_GATTC_CONN_ST) { APPL_TRACE_ERROR("server cache not available, CLCB state = %d", p_clcb->state); + *count = 0; + *db = NULL; return; } - if (!p_clcb->p_srcb || p_clcb->p_srcb->p_srvc_list || /* no active discovery */ + /* Reject if no server context, discovery still in progress (p_srvc_list in use), or cache not ready + p_srvc_list must set to NULL after osi_free; + */ + if (!p_clcb->p_srcb || p_clcb->p_srcb->p_srvc_list || !p_clcb->p_srcb->p_srvc_cache) { + *count = 0; + *db = NULL; APPL_TRACE_ERROR("No server cache available"); return; } @@ -2032,10 +2053,11 @@ void bta_gattc_cache_save(tBTA_GATTC_SERV *p_srvc_cb, UINT16 conn_id) return; } - int i = 0; + /* i: current write index, equals actual count after loops; db_size: allocated slots from cache count */ + size_t i = 0; size_t db_size = bta_gattc_get_db_size(p_srvc_cb->p_srvc_cache, 0x0000, 0xFFFF); tBTA_GATTC_NV_ATTR *nv_attr = osi_malloc(db_size * sizeof(tBTA_GATTC_NV_ATTR)); - // This step is very importent, if not clear the memory, the hasy key base on the attribute case will be not corret. + // This step is very important, if not clear the memory, the hasy key base on the attribute case will be not correct. if (nv_attr != NULL) { memset(nv_attr, 0, db_size * sizeof(tBTA_GATTC_NV_ATTR)); } @@ -2048,6 +2070,11 @@ void bta_gattc_cache_save(tBTA_GATTC_SERV *p_srvc_cb, UINT16 conn_id) sn != list_end(p_srvc_cb->p_srvc_cache); sn = list_next(sn)) { tBTA_GATTC_SERVICE *p_cur_srvc = list_node(sn); + /* Bounds check: avoid buffer overflow if cache is modified and count exceeds db_size */ + if (i >= db_size) { + APPL_TRACE_ERROR("%s(), db_size exceeded (services), skip saving.", __func__); + goto save_error; + } bta_gattc_fill_nv_attr(&nv_attr[i++], BTA_GATTC_ATTR_TYPE_SRVC, p_cur_srvc->s_handle, @@ -2063,41 +2090,49 @@ void bta_gattc_cache_save(tBTA_GATTC_SERV *p_srvc_cb, UINT16 conn_id) sn != list_end(p_srvc_cb->p_srvc_cache); sn = list_next(sn)) { tBTA_GATTC_SERVICE *p_cur_srvc = list_node(sn); - if (!p_cur_srvc->characteristics || list_is_empty(p_cur_srvc->characteristics)) { - continue; - } - - for (list_node_t *cn = list_begin(p_cur_srvc->characteristics); - cn != list_end(p_cur_srvc->characteristics); cn = list_next(cn)) { - tBTA_GATTC_CHARACTERISTIC *p_char = list_node(cn); - - bta_gattc_fill_nv_attr(&nv_attr[i++], - BTA_GATTC_ATTR_TYPE_CHAR, - p_char->handle, - 0, - p_char->uuid, - p_char->properties, - 0 /* incl_srvc_s_handle */, - 0 /* incl_srvc_e_handle */, - FALSE); - - if (!p_char->descriptors || list_is_empty(p_char->descriptors)) { - continue; - } - - for (list_node_t *dn = list_begin(p_char->descriptors); - dn != list_end(p_char->descriptors); dn = list_next(dn)) { - tBTA_GATTC_DESCRIPTOR *p_desc = list_node(dn); + if (p_cur_srvc->characteristics && !list_is_empty(p_cur_srvc->characteristics)) { + for (list_node_t *cn = list_begin(p_cur_srvc->characteristics); + cn != list_end(p_cur_srvc->characteristics); cn = list_next(cn)) { + tBTA_GATTC_CHARACTERISTIC *p_char = list_node(cn); + /* Bounds check: avoid buffer overflow if count exceeds db_size */ + if (i >= db_size) { + APPL_TRACE_ERROR("%s(), db_size exceeded (characteristics), skip saving.", __func__); + goto save_error; + } bta_gattc_fill_nv_attr(&nv_attr[i++], - BTA_GATTC_ATTR_TYPE_CHAR_DESCR, - p_desc->handle, + BTA_GATTC_ATTR_TYPE_CHAR, + p_char->handle, 0, - p_desc->uuid, - 0 /* properties */, + p_char->uuid, + p_char->properties, 0 /* incl_srvc_s_handle */, 0 /* incl_srvc_e_handle */, FALSE); + + if (!p_char->descriptors || list_is_empty(p_char->descriptors)) { + continue; + } + + for (list_node_t *dn = list_begin(p_char->descriptors); + dn != list_end(p_char->descriptors); dn = list_next(dn)) { + tBTA_GATTC_DESCRIPTOR *p_desc = list_node(dn); + + /* Bounds check: avoid buffer overflow if count exceeds db_size */ + if (i >= db_size) { + APPL_TRACE_ERROR("%s(), db_size exceeded (descriptors), skip saving.", __func__); + goto save_error; + } + bta_gattc_fill_nv_attr(&nv_attr[i++], + BTA_GATTC_ATTR_TYPE_CHAR_DESCR, + p_desc->handle, + 0, + p_desc->uuid, + 0 /* properties */, + 0 /* incl_srvc_s_handle */, + 0 /* incl_srvc_e_handle */, + FALSE); + } } } @@ -2108,21 +2143,32 @@ void bta_gattc_cache_save(tBTA_GATTC_SERV *p_srvc_cb, UINT16 conn_id) for (list_node_t *an = list_begin(p_cur_srvc->included_svc); an != list_end(p_cur_srvc->included_svc); an = list_next(an)) { tBTA_GATTC_INCLUDED_SVC *p_isvc = list_node(an); + UINT16 incl_s_handle = p_isvc->included_service ? p_isvc->included_service->s_handle : p_isvc->incl_srvc_s_handle; + UINT16 incl_e_handle = p_isvc->included_service ? p_isvc->included_service->e_handle : p_isvc->incl_srvc_e_handle; + /* Bounds check: avoid buffer overflow if count exceeds db_size */ + if (i >= db_size) { + APPL_TRACE_ERROR("%s(), db_size exceeded (included_svc), skip saving.", __func__); + goto save_error; + } bta_gattc_fill_nv_attr(&nv_attr[i++], BTA_GATTC_ATTR_TYPE_INCL_SRVC, p_isvc->handle, 0, p_isvc->uuid, 0 /* properties */, - p_isvc->included_service->s_handle, - p_isvc->included_service->e_handle, + incl_s_handle, + incl_e_handle, FALSE); } } - /* TODO: Gattc cache write/read need to be added in IDF 3.1*/ - bta_gattc_cache_write(p_srvc_cb->server_bda, db_size, nv_attr); + bta_gattc_cache_write(p_srvc_cb->server_bda, (UINT16)db_size, nv_attr); + osi_free(nv_attr); + return; + +save_error: + APPL_TRACE_ERROR("%s(), cache inconsistency detected, not saving partial cache.", __func__); osi_free(nv_attr); } @@ -2149,11 +2195,14 @@ bool bta_gattc_cache_load(tBTA_GATTC_CLCB *p_clcb) return false; } - size_t num_attr = bta_gattc_get_cache_attr_length(index) / sizeof(tBTA_GATTC_NV_ATTR); + size_t length = bta_gattc_get_cache_attr_length(index); - if (!num_attr) { + if (length == 0 || (length % sizeof(tBTA_GATTC_NV_ATTR)) != 0) { + APPL_TRACE_ERROR("%s, Invalid cache length %d", __func__, length); return false; } + + size_t num_attr = length / sizeof(tBTA_GATTC_NV_ATTR); //don't forget to set the total attribute number. p_clcb->p_srcb->total_attr = num_attr; APPL_TRACE_DEBUG("%s(), index = %x, num_attr = %d", __func__, index, num_attr); @@ -2163,6 +2212,7 @@ bool bta_gattc_cache_load(tBTA_GATTC_CLCB *p_clcb) } if ((status = bta_gattc_co_cache_load(attr, index)) != BTA_GATT_OK) { APPL_TRACE_DEBUG("%s(), gattc cache load fail, status = %x", __func__, status); + osi_free(attr); return false; } p_clcb->searched_service_source = BTA_GATTC_SERVICE_INFO_FROM_NVS_FLASH; diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gattc_co.c b/components/bt/host/bluedroid/bta/gatt/bta_gattc_co.c index d6ca7bca47..80a10a8f2a 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_co.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_co.c @@ -172,6 +172,14 @@ static void cacheReset(BD_ADDR bda, BOOLEAN update) return; } + // Free the assoc_addr list_t structure before array shift to prevent memory leak + // Note: bta_gattc_co_cache_clear_assoc_addr only calls list_clear which clears nodes + // but doesn't free the list_t structure itself + if (cache_env->cache_addr[index].assoc_addr != NULL) { + list_free(cache_env->cache_addr[index].assoc_addr); + cache_env->cache_addr[index].assoc_addr = NULL; + } + UINT8 num = cache_env->num_addr; //delete the server_bda in the addr_info list. for(UINT8 i = index; i < (num - 1); i++) { @@ -435,6 +443,8 @@ void bta_gattc_co_cache_addr_init(void) } else { APPL_TRACE_ERROR("%s, Line = %d, nvs flash open fail, err_code = %x", __func__, __LINE__, err_code); osi_free(p_buf); + osi_free(cache_env); + cache_env = NULL; return; } @@ -444,11 +454,18 @@ void bta_gattc_co_cache_addr_init(void) void bta_gattc_co_cache_addr_deinit(void) { - APPL_TRACE_DEBUG("%s is_open=%d", __func__, cache_env->is_open); - - if(!cache_env->is_open) { + if(cache_env == NULL) { return; } + + APPL_TRACE_DEBUG("%s is_open=%d", __func__, cache_env->is_open); + + if (!cache_env->is_open) { + osi_free(cache_env); + cache_env = NULL; + return; + } + nvs_close(cache_env->addr_fp); cache_env->is_open = false; @@ -608,9 +625,30 @@ BOOLEAN bta_gattc_co_cache_append_assoc_addr(BD_ADDR src_addr, BD_ADDR assoc_add if ((addr_index = bta_gattc_co_find_addr_in_cache(src_addr)) != INVALID_ADDR_NUM) { addr_info = &cache_env->cache_addr[addr_index]; if (addr_info->assoc_addr == NULL) { - addr_info->assoc_addr =list_new(NULL); + addr_info->assoc_addr =list_new(osi_free_func); } - return list_append(addr_info->assoc_addr, p_assoc_buf); + if (addr_info->assoc_addr == NULL) { + APPL_TRACE_ERROR("assoc_addr list creation failed"); + osi_free(p_assoc_buf); + return FALSE; + } + + for (list_node_t *sn = list_begin(addr_info->assoc_addr); + sn != list_end(addr_info->assoc_addr); sn = list_next(sn)) { + if (!memcmp(list_node(sn), assoc_addr, sizeof(BD_ADDR))) { + APPL_TRACE_WARNING("Association already exists"); + osi_free(p_assoc_buf); + return TRUE; + } + } + + if (!list_append(addr_info->assoc_addr, p_assoc_buf)) { + APPL_TRACE_ERROR("Failed to append to assoc_addr list"); + osi_free(p_assoc_buf); + return FALSE; + + } + return TRUE; } else { osi_free(p_assoc_buf); } @@ -643,16 +681,15 @@ BOOLEAN bta_gattc_co_cache_remove_assoc_addr(BD_ADDR src_addr, BD_ADDR assoc_add UINT8* bta_gattc_co_cache_find_src_addr(BD_ADDR assoc_addr, UINT8 *index) { - UINT8 num = cache_env->num_addr; + UINT8 num = (cache_env->num_addr > MAX_DEVICE_IN_CACHE) ? MAX_DEVICE_IN_CACHE : cache_env->num_addr; cache_addr_info_t *addr_info = &cache_env->cache_addr[0]; UINT8 *addr_data; - //Check the assoc_addr list is NULL or not - if (addr_info->assoc_addr == NULL) { - *index = INVALID_ADDR_NUM; - return NULL; - } for (int i = 0; i < num; i++) { + if (addr_info->assoc_addr == NULL) { + addr_info++; + continue; + } for (const list_node_t *node = list_begin(addr_info->assoc_addr); node != list_end(addr_info->assoc_addr); node = list_next(node)) { addr_data = (UINT8 *)list_node(node); @@ -662,11 +699,6 @@ UINT8* bta_gattc_co_cache_find_src_addr(BD_ADDR assoc_addr, UINT8 *index) } } addr_info++; - - if (addr_info->assoc_addr == NULL) { - *index = INVALID_ADDR_NUM; - return NULL; - } } *index = INVALID_ADDR_NUM; 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 c1d1e72033..09e1aea2fa 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_main.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_main.c @@ -41,7 +41,6 @@ enum { BTA_GATTC_OPEN, BTA_GATTC_OPEN_FAIL, - BTA_GATTC_OPEN_ERROR, BTA_GATTC_CANCEL_OPEN, BTA_GATTC_CANCEL_OPEN_OK, BTA_GATTC_CANCEL_OPEN_ERROR, @@ -77,7 +76,6 @@ typedef void (*tBTA_GATTC_ACTION)(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_da const tBTA_GATTC_ACTION bta_gattc_action[] = { bta_gattc_open, bta_gattc_open_fail, - bta_gattc_open_error, bta_gattc_cancel_open, bta_gattc_cancel_open_ok, bta_gattc_cancel_open_error, @@ -289,6 +287,11 @@ BOOLEAN bta_gattc_sm_execute(tBTA_GATTC_CLCB *p_clcb, UINT16 event, tBTA_GATTC_D gattc_evt_code(in_event)); #endif + // should not happen, but just in case + if (!p_clcb) { + APPL_TRACE_ERROR("p_clcb or p_data is NULL, event is %d", event); + return rt; + } /* look up the state table for the current state */ state_table = bta_gattc_st_tbl[p_clcb->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 6f3f3c0a4c..bc329b775b 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_utils.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_utils.c @@ -35,37 +35,16 @@ #include "bta_gattc_int.h" #include "stack/l2c_api.h" #include "osi/allocator.h" +#include "gatt_int.h" /***************************************************************************** ** Constants *****************************************************************************/ - -static const UINT8 base_uuid[LEN_UUID_128] = {0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80, - 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 - }; - static const BD_ADDR dummy_bda = {0, 0, 0, 0, 0, 0}; #define GATTC_COMMAND_QUEUE_SIZE_MAX 30 -/******************************************************************************* -** -** Function bta_gatt_convert_uuid16_to_uuid128 -** -** Description Convert a 16 bits UUID to be an standard 128 bits one. -** -** Returns TRUE if two uuid match; FALSE otherwise. -** -*******************************************************************************/ -void bta_gatt_convert_uuid16_to_uuid128(UINT8 uuid_128[LEN_UUID_128], UINT16 uuid_16) -{ - UINT8 *p = &uuid_128[LEN_UUID_128 - 4]; - - memcpy (uuid_128, base_uuid, LEN_UUID_128); - - UINT16_TO_STREAM(p, uuid_16); -} /******************************************************************************* ** ** Function bta_gattc_uuid_compare @@ -77,11 +56,8 @@ void bta_gatt_convert_uuid16_to_uuid128(UINT8 uuid_128[LEN_UUID_128], UINT16 uui *******************************************************************************/ BOOLEAN bta_gattc_uuid_compare (const tBT_UUID *p_src, const tBT_UUID *p_tar, BOOLEAN is_precise) { - UINT8 su[LEN_UUID_128], tu[LEN_UUID_128]; - const UINT8 *ps, *pt; - /* any of the UUID is unspecified */ - if (p_src == 0 || p_tar == 0) { + if (p_src == 0 || p_tar == 0 || p_src->len == 0 || p_tar->len == 0) { if (is_precise) { return FALSE; } else { @@ -89,29 +65,7 @@ BOOLEAN bta_gattc_uuid_compare (const tBT_UUID *p_src, const tBT_UUID *p_tar, BO } } - /* If both are 16-bit, we can do a simple compare */ - if (p_src->len == 2 && p_tar->len == 2) { - return p_src->uu.uuid16 == p_tar->uu.uuid16; - } - - /* One or both of the UUIDs is 128-bit */ - if (p_src->len == LEN_UUID_16) { - /* convert a 16 bits UUID to 128 bits value */ - bta_gatt_convert_uuid16_to_uuid128(su, p_src->uu.uuid16); - ps = su; - } else { - ps = p_src->uu.uuid128; - } - - if (p_tar->len == LEN_UUID_16) { - /* convert a 16 bits UUID to 128 bits value */ - bta_gatt_convert_uuid16_to_uuid128(tu, p_tar->uu.uuid16); - pt = tu; - } else { - pt = p_tar->uu.uuid128; - } - - return (memcmp(ps, pt, LEN_UUID_128) == 0); + return gatt_uuid_compare(*p_src, *p_tar); } /******************************************************************************* @@ -242,6 +196,10 @@ tBTA_GATTC_CLCB *bta_gattc_clcb_alloc(tBTA_GATTC_IF client_if, BD_ADDR remote_bd p_clcb->p_srcb->num_clcb ++; p_clcb->p_rcb->num_clcb ++; } else { + if (p_clcb->p_cmd_list) { + list_free(p_clcb->p_cmd_list); + p_clcb->p_cmd_list = NULL; + } /* release this clcb if clcb or srcb allocation failed */ p_clcb->in_use = FALSE; p_clcb = NULL; @@ -305,6 +263,9 @@ void bta_gattc_clcb_dealloc(tBTA_GATTC_CLCB *p_clcb) list_free(p_srcb->p_srvc_cache); p_srcb->p_srvc_cache = NULL; } +#if (GATTC_CACHE_NVS == TRUE) + bta_gattc_co_cache_close(p_srcb->server_bda, 0); +#endif } if ( p_clcb->p_q_cmd != NULL && !list_contains(p_clcb->p_cmd_list, p_clcb->p_q_cmd)){ @@ -740,12 +701,12 @@ BOOLEAN bta_gattc_mark_bg_conn (tBTA_GATTC_IF client_if, BD_ADDR_PTR remote_bda if (add) /* mask on the cif bit */ { - *p_cif_mask |= (1 << (client_if - 1)); + *p_cif_mask |= BTA_GATTC_CIF_MASK_BIT(client_if); } else { - if (client_if != 0) { - *p_cif_mask &= (~(1 << (client_if - 1))); - } else { + if (client_if == 0) { *p_cif_mask = 0; + } else { + *p_cif_mask &= (~BTA_GATTC_CIF_MASK_BIT(client_if)); } } /* no BG connection for this device, make it available */ @@ -777,7 +738,7 @@ BOOLEAN bta_gattc_mark_bg_conn (tBTA_GATTC_IF client_if, BD_ADDR_PTR remote_bda p_cif_mask = is_listen ? &p_bg_tck->cif_adv_mask : &p_bg_tck->cif_mask; - *p_cif_mask = (1 << (client_if - 1)); + *p_cif_mask = BTA_GATTC_CIF_MASK_BIT(client_if); return TRUE; } } @@ -804,12 +765,12 @@ BOOLEAN bta_gattc_check_bg_conn (tBTA_GATTC_IF client_if, BD_ADDR remote_bda, U if (p_bg_tck->in_use && (bdcmp(p_bg_tck->remote_bda, remote_bda) == 0 || bdcmp(p_bg_tck->remote_bda, dummy_bda) == 0)) { - if (((p_bg_tck->cif_mask & (1 << (client_if - 1))) != 0) && + if (((p_bg_tck->cif_mask & BTA_GATTC_CIF_MASK_BIT(client_if)) != 0) && role == HCI_ROLE_MASTER) { is_bg_conn = TRUE; } - if (((p_bg_tck->cif_adv_mask & (1 << (client_if - 1))) != 0) && + if (((p_bg_tck->cif_adv_mask & BTA_GATTC_CIF_MASK_BIT(client_if)) != 0) && role == HCI_ROLE_SLAVE) { is_bg_conn = TRUE; } 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 4e786bc8a1..04bcb697f8 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gatts_act.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gatts_act.c @@ -36,6 +36,7 @@ #include #include "osi/allocator.h" #include "l2c_int.h" +#include "gatt_int.h" static void bta_gatts_nv_save_cback(BOOLEAN is_saved, tGATTS_HNDL_RANGE *p_hndl_range); static BOOLEAN bta_gatts_nv_srv_chg_cback(tGATTS_SRV_CHG_CMD cmd, tGATTS_SRV_CHG_REQ *p_req, @@ -176,13 +177,15 @@ void bta_gatts_register(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg) tBTA_GATT_STATUS status = BTA_GATT_OK; UINT8 i, first_unuse = 0xff; + memset(&cb_data, 0, sizeof(tBTA_GATTS)); + if (p_cb->enabled == FALSE) { bta_gatts_enable(p_cb); } for (i = 0; i < BTA_GATTS_MAX_APP_NUM; i ++) { if (p_cb->rcb[i].in_use) { - if (bta_gatts_uuid_compare(p_cb->rcb[i].app_uuid, p_msg->api_reg.app_uuid)) { + if (gatt_uuid_compare(p_cb->rcb[i].app_uuid, p_msg->api_reg.app_uuid)) { APPL_TRACE_ERROR("application already registered.\n"); status = BTA_GATT_DUP_REG; break; @@ -199,9 +202,7 @@ void bta_gatts_register(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg) } cb_data.reg_oper.server_if = BTA_GATTS_INVALID_IF; -// btla-specific ++ memcpy(&cb_data.reg_oper.uuid, &p_msg->api_reg.app_uuid, sizeof(tBT_UUID)); -// btla-specific -- if (first_unuse != 0xff) { APPL_TRACE_VERBOSE("register application first_unuse rcb_idx = %d", first_unuse); @@ -213,6 +214,7 @@ void bta_gatts_register(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg) GATT_Register(&p_msg->api_reg.app_uuid, &bta_gatts_cback); if ( !p_cb->rcb[first_unuse].gatt_if) { status = BTA_GATT_NO_RESOURCES; + memset( &p_cb->rcb[first_unuse], 0 , sizeof(tBTA_GATTS_RCB)); } else { if ((p_buf = (tBTA_GATTS_INT_START_IF *) osi_malloc(sizeof(tBTA_GATTS_INT_START_IF))) != NULL) { @@ -222,6 +224,7 @@ void bta_gatts_register(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg) bta_sys_sendmsg(p_buf); } else { status = BTA_GATT_NO_RESOURCES; + GATT_Deregister(p_cb->rcb[first_unuse].gatt_if); memset( &p_cb->rcb[first_unuse], 0 , sizeof(tBTA_GATTS_RCB)); } } @@ -309,7 +312,7 @@ void bta_gatts_deregister(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg) void bta_gatts_create_srvc(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg) { UINT8 rcb_idx; - tBTA_GATTS cb_data; + tBTA_GATTS cb_data = {0}; UINT8 srvc_idx; UINT16 service_id = 0; 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 b712da9b79..06667a415b 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gatts_api.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gatts_api.c @@ -302,6 +302,8 @@ void BTA_GATTS_AddCharDescriptor (UINT16 service_id, memcpy(p_buf->attr_val.attr_val, attr_val->attr_val, value_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__); } diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gatts_main.c b/components/bt/host/bluedroid/bta/gatt/bta_gatts_main.c index bf1174c5f7..6444321437 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gatts_main.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gatts_main.c @@ -109,6 +109,14 @@ BOOLEAN bta_gatts_hdl_event(BT_HDR *p_msg) case BTA_GATTS_API_SET_ATTR_VAL_EVT:{ UINT16 attr_id = ((tBTA_GATTS_DATA *) p_msg)->api_set_val.hdr.layer_specific; p_srvc_cb = bta_gatts_find_srvc_cb_by_attr_id(p_cb, attr_id); + if (p_srvc_cb == NULL) { + if (((tBTA_GATTS_DATA *) p_msg)->api_set_val.value != NULL) { + osi_free(((tBTA_GATTS_DATA *) p_msg)->api_set_val.value); + } + APPL_TRACE_ERROR("Service not found for attribute ID: %d", attr_id); + break; + } + bta_gatts_set_attr_value(p_srvc_cb, (tBTA_GATTS_DATA *) p_msg); break; } diff --git a/components/bt/host/bluedroid/bta/gatt/bta_gatts_utils.c b/components/bt/host/bluedroid/bta/gatt/bta_gatts_utils.c index 224e887334..d697f0dc19 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gatts_utils.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gatts_utils.c @@ -31,27 +31,7 @@ #include "bta/bta_sys.h" #include "bta_gatts_int.h" -static const UINT8 base_uuid[LEN_UUID_128] = {0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80, - 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 - }; -/******************************************************************************* -** -** Function bta_gatt_convert_uuid16_to_uuid128 -** -** Description Convert a 16 bits UUID to be an standard 128 bits one. -** -** Returns TRUE if two uuid match; FALSE otherwise. -** -*******************************************************************************/ -static void bta_gatt_convert_uuid16_to_uuid128(UINT8 uuid_128[LEN_UUID_128], UINT16 uuid_16) -{ - UINT8 *p = &uuid_128[LEN_UUID_128 - 4]; - - memcpy (uuid_128, base_uuid, LEN_UUID_128); - - UINT16_TO_STREAM(p, uuid_16); -} /******************************************************************************* ** ** Function bta_gatts_alloc_srvc_cb @@ -167,6 +147,7 @@ tBTA_GATTS_SRVC_CB *bta_gatts_find_srvc_cb_by_attr_id(tBTA_GATTS_CB *p_cb, UINT1 attr_id >= p_cb->srvc_cb[i].service_id) || /* last service incb */ (i == (BTA_GATTS_MAX_SRVC_NUM - 1) && + p_cb->srvc_cb[i].in_use && attr_id >= p_cb->srvc_cb[i].service_id) ) { return &p_cb->srvc_cb[i]; @@ -174,51 +155,5 @@ tBTA_GATTS_SRVC_CB *bta_gatts_find_srvc_cb_by_attr_id(tBTA_GATTS_CB *p_cb, UINT1 } return NULL; } -/******************************************************************************* -** -** Function bta_gatts_uuid_compare -** -** Description Compare two UUID to see if they are the same. -** -** Returns TRUE if two uuid match; FALSE otherwise. -** -*******************************************************************************/ -BOOLEAN bta_gatts_uuid_compare(tBT_UUID tar, tBT_UUID src) -{ - UINT8 su[LEN_UUID_128], tu[LEN_UUID_128]; - UINT8 *ps, *pt; - - /* any of the UUID is unspecified */ - if (src.len == 0 || tar.len == 0) { - return TRUE; - } - - /* If both are 16-bit, we can do a simple compare */ - if (src.len == 2 && tar.len == 2) { - return src.uu.uuid16 == tar.uu.uuid16; - } - - /* One or both of the UUIDs is 128-bit */ - if (src.len == LEN_UUID_16) { - /* convert a 16 bits UUID to 128 bits value */ - bta_gatt_convert_uuid16_to_uuid128(su, src.uu.uuid16); - ps = su; - } else { - ps = src.uu.uuid128; - } - - if (tar.len == LEN_UUID_16) { - /* convert a 16 bits UUID to 128 bits value */ - bta_gatt_convert_uuid16_to_uuid128(tu, tar.uu.uuid16); - pt = tu; - } else { - pt = tar.uu.uuid128; - } - - return (memcmp(ps, pt, LEN_UUID_128) == 0); -} - - - #endif /* GATTS_INCLUDED */ diff --git a/components/bt/host/bluedroid/bta/gatt/include/bta_gattc_int.h b/components/bt/host/bluedroid/bta/gatt/include/bta_gattc_int.h index ac42de6973..e74b5759ee 100644 --- a/components/bt/host/bluedroid/bta/gatt/include/bta_gattc_int.h +++ b/components/bt/host/bluedroid/bta/gatt/include/bta_gattc_int.h @@ -383,6 +383,10 @@ typedef UINT16 tBTA_GATTC_CIF_MASK; typedef UINT32 tBTA_GATTC_CIF_MASK; #endif +/* Safe bit for client_if in cif_mask; avoids UB when client_if is 0 or > GATT_MAX_APPS */ +#define BTA_GATTC_CIF_MASK_BIT(cif) \ + ((tBTA_GATTC_CIF_MASK)(((cif) >= 1 && (cif) <= (unsigned)GATT_MAX_APPS) ? (1U << ((cif) - 1)) : 0U)) + typedef struct { BOOLEAN in_use; BD_ADDR remote_bda; @@ -456,7 +460,6 @@ extern void bta_gattc_process_enc_cmpl(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_m /* function within state machine */ extern void bta_gattc_open(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data); extern void bta_gattc_open_fail(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data); -extern void bta_gattc_open_error(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data); extern void bta_gattc_cancel_open(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data); extern void bta_gattc_cancel_open_ok(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data); @@ -542,7 +545,7 @@ extern void bta_gattc_get_service_with_uuid(UINT16 conn_id, tBT_UUID *svc_uuid, btgatt_db_element_t **svc_db, UINT16 *count); -extern void bta_gattc_get_db_with_opration(UINT16 conn_id, +extern void bta_gattc_get_db_with_operation(UINT16 conn_id, bt_gatt_get_db_op_t op, UINT16 char_handle, tBT_UUID *incl_uuid, diff --git a/components/bt/host/bluedroid/bta/gatt/include/bta_gatts_int.h b/components/bt/host/bluedroid/bta/gatt/include/bta_gatts_int.h index 854a28a208..8a5e4a7749 100644 --- a/components/bt/host/bluedroid/bta/gatt/include/bta_gatts_int.h +++ b/components/bt/host/bluedroid/bta/gatt/include/bta_gatts_int.h @@ -243,7 +243,6 @@ extern void bta_gatts_close (tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg); extern void bta_gatts_send_service_change_indication (tBTA_GATTS_DATA *p_msg); extern void bta_gatts_show_local_database (void); -extern BOOLEAN bta_gatts_uuid_compare(tBT_UUID tar, tBT_UUID src); extern tBTA_GATTS_RCB *bta_gatts_find_app_rcb_by_app_if(tBTA_GATTS_IF server_if); extern UINT8 bta_gatts_find_app_rcb_idx_by_app_if(tBTA_GATTS_CB *p_cb, tBTA_GATTS_IF server_if); extern UINT8 bta_gatts_alloc_srvc_cb(tBTA_GATTS_CB *p_cb, UINT8 rcb_idx);