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 afc6c821588..9dce7417070 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_act.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_act.c @@ -189,7 +189,7 @@ void bta_gattc_disable(tBTA_GATTC_CB *p_cb) *******************************************************************************/ void bta_gattc_register(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_data) { - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; UINT8 i; tBT_UUID *p_app_uuid = &p_data->api_reg.app_uuid; tBTA_GATTC_INT_START_IF *p_buf; @@ -383,7 +383,7 @@ void bta_gattc_process_api_open_cancel (tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_ UINT16 event = ((BT_HDR *)p_msg)->event; tBTA_GATTC_CLCB *p_clcb = NULL; tBTA_GATTC_RCB *p_clreg; - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; UNUSED(p_cb); if (p_msg->api_cancel_conn.is_direct) { @@ -425,7 +425,7 @@ void bta_gattc_process_api_open_cancel (tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_ void bta_gattc_process_enc_cmpl(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_msg) { tBTA_GATTC_RCB *p_clreg; - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; UNUSED(p_cb); p_clreg = bta_gattc_cl_get_regcb(p_msg->enc_cmpl.client_if); @@ -451,7 +451,7 @@ void bta_gattc_process_enc_cmpl(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_msg) *******************************************************************************/ void bta_gattc_cancel_open_error(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) { - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; UNUSED(p_data); memset(&cb_data, 0, sizeof(cb_data)); @@ -513,6 +513,8 @@ void bta_gattc_open(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) return; } + memset(&gattc_data, 0, sizeof(gattc_data)); + p_tcb = gatt_find_tcb_by_addr(p_data->api_conn.remote_bda, BT_TRANSPORT_LE); if(p_tcb) { found_app = gatt_find_specific_app_in_hold_link(p_tcb, p_clcb->p_rcb->client_if); @@ -584,6 +586,8 @@ void bta_gattc_init_bk_conn(tBTA_GATTC_API_OPEN *p_data, tBTA_GATTC_RCB *p_clreg tBTA_GATTC_CLCB *p_clcb; tBTA_GATTC_DATA gattc_data; + memset(&gattc_data, 0, sizeof(gattc_data)); + if (bta_gattc_mark_bg_conn(p_data->client_if, p_data->remote_bda, TRUE, FALSE)) { /* always call open to hold a connection */ if (!GATT_Connect(p_data->client_if, p_data->remote_bda, @@ -634,8 +638,7 @@ void bta_gattc_init_bk_conn(tBTA_GATTC_API_OPEN *p_data, tBTA_GATTC_RCB *p_clreg void bta_gattc_cancel_bk_conn(tBTA_GATTC_API_CANCEL_OPEN *p_data) { tBTA_GATTC_RCB *p_clreg; - tBTA_GATTC cb_data; - + tBTA_GATTC cb_data = {0}; memset(&cb_data, 0, sizeof(cb_data)); cb_data.cancel_open.status = BTA_GATT_ERROR; cb_data.cancel_open.client_if = p_data->client_if; @@ -667,7 +670,7 @@ void bta_gattc_cancel_bk_conn(tBTA_GATTC_API_CANCEL_OPEN *p_data) *******************************************************************************/ void bta_gattc_cancel_open_ok(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) { - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; UNUSED(p_data); if ( p_clcb->p_rcb->p_cback ) { @@ -691,8 +694,7 @@ void bta_gattc_cancel_open_ok(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) *******************************************************************************/ void bta_gattc_cancel_open(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) { - tBTA_GATTC cb_data; - + tBTA_GATTC cb_data = {0}; if (GATT_CancelConnect(p_clcb->p_rcb->client_if, p_data->api_cancel_conn.remote_bda, TRUE)) { bta_gattc_sm_execute(p_clcb, BTA_GATTC_INT_CANCEL_OPEN_OK_EVT, p_data); } else { @@ -759,12 +761,18 @@ void bta_gattc_conn(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) bta_gattc_register_service_change_notify(p_clcb->bta_conn_id, p_clcb->bda); } else #endif - { /* cache is building */ - APPL_TRACE_DEBUG("%s cache not found, start discovery %u", __func__, bta_gattc_cb.auto_disc); + { /* cache miss or cache load failed */ + APPL_TRACE_DEBUG("%s cache not found, auto_disc=%u", __func__, bta_gattc_cb.auto_disc); if (bta_gattc_cb.auto_disc) { p_clcb->p_srcb->state = BTA_GATTC_SERV_DISC; /* cache load failure, start discovery */ bta_gattc_start_discover(p_clcb, NULL); + } else { + /* Auto discovery is disabled: roll the SRCB back to + * SERV_IDLE so it is not stuck in SERV_LOAD. The app is + * expected to drive service discovery explicitly via + * BTA_GATTC_ServiceSearchRequest() once it is ready. */ + p_clcb->p_srcb->state = BTA_GATTC_SERV_IDLE; } } } else { /* cache is building */ @@ -852,8 +860,7 @@ void bta_gattc_disconncback(tBTA_GATTC_RCB *p_rcb, tBTA_GATTC_DATA *p_data) *******************************************************************************/ void bta_gattc_close_fail(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) { - tBTA_GATTC cb_data; - + tBTA_GATTC cb_data = {0}; if ( p_clcb->p_rcb->p_cback ) { memset(&cb_data, 0, sizeof(tBTA_GATTC)); cb_data.close.client_if = p_clcb->p_rcb->client_if; @@ -883,8 +890,7 @@ void bta_gattc_close(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) } tBTA_GATTC_CBACK *p_cback = p_clcb->p_rcb->p_cback; tBTA_GATTC_RCB *p_clreg = p_clcb->p_rcb; - tBTA_GATTC cb_data; - + tBTA_GATTC cb_data = {0}; APPL_TRACE_DEBUG("bta_gattc_close conn_id=%d", p_clcb->bta_conn_id); cb_data.close.client_if = p_clcb->p_rcb->client_if; @@ -908,9 +914,13 @@ void bta_gattc_close(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) (* p_cback)(BTA_GATTC_CLOSE_EVT, (tBTA_GATTC *)&cb_data); } - // Please note that BTA_GATTC_CLOSE_EVT will run in the BTC task. - // because bta_gattc_deregister_cmpl did not execute as expected(this is a known issue), - // we will run it again in bta_gattc_clcb_dealloc_by_conn_id. + /* + * Free the CLCB in the BTA task after the close callback has been copied to + * BTC. This keeps the original close-callback semantics while avoiding BTC + * task access to core stack control blocks without locking. + */ + bta_gattc_clcb_dealloc(p_clcb); + if (p_clreg->num_clcb == 0 && p_clreg->dereg_pending) { bta_gattc_deregister_cmpl(p_clreg); } @@ -1078,7 +1088,7 @@ void bta_gattc_start_discover(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) APPL_TRACE_ERROR("discovery on server failed"); bta_gattc_reset_discover_st(p_clcb->p_srcb, p_clcb->status); //discover service complete, trigger callback - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; cb_data.dis_cmpl.status = p_clcb->status; cb_data.dis_cmpl.conn_id = p_clcb->bta_conn_id; ( *p_clcb->p_rcb->p_cback)(BTA_GATTC_DIS_SRVC_CMPL_EVT, &cb_data); @@ -1380,7 +1390,7 @@ void bta_gattc_confirm(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) void bta_gattc_read_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) { UINT8 event; - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; tBTA_GATT_UNFMT read_value; if (p_clcb->p_q_cmd == NULL) { @@ -1434,8 +1444,21 @@ void bta_gattc_write_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) 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; + tBTA_GATTC_CONN *p_conn = bta_gattc_conn_find(p_clcb->bda); bta_gattc_free_command_data(p_clcb); bta_gattc_pop_command_to_send(p_clcb); + /* If this completion belongs to the internal service-change CCC write, + * clear the in-progress flag and swallow the event so the application + * does not see a spurious BTA_GATTC_WRITE_DESCR_EVT, and a future + * bta_gattc_register_service_change_notify() can retry. */ + if (p_conn && + p_conn->write_remote_svc_change_ccc_in_progress && + p_conn->svc_change_descr_handle == handle) { + p_conn->write_remote_svc_change_ccc_in_progress = FALSE; + p_conn->write_remote_svc_change_ccc_done = FALSE; + APPL_TRACE_ERROR("svc chg ccc: p_cmpl NULL"); + return; + } cb_data.write.status = BTA_GATT_ERROR; cb_data.write.handle = handle; cb_data.write.conn_id = p_clcb->bta_conn_id; @@ -1452,7 +1475,7 @@ void bta_gattc_write_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) 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) { + if (p_clcb->p_q_cmd->api_write.p_value == NULL && p_clcb->p_q_cmd->api_write.len != 0) { 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); @@ -1461,10 +1484,21 @@ void bta_gattc_write_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) ( *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) { - cb_data.write.status = BTA_GATT_INVALID_PDU; + /* Rsp value is one ATT chunk (<= MTU-5), not necessarily full api_write.len. */ + { + UINT16 rsp_len = p_data->p_cmpl->att_value.len; + UINT16 req_len = p_clcb->p_q_cmd->api_write.len; + tGATT_VALUE *a = &p_data->p_cmpl->att_value; + tBTA_GATTC_API_WRITE *w = &p_clcb->p_q_cmd->api_write; + + if (a->handle != w->handle || a->offset != w->offset || rsp_len > req_len || + (req_len > 0 && rsp_len == 0) || + (rsp_len > 0 && w->p_value != NULL && + memcmp(w->p_value, a->value, rsp_len) != 0)) { + APPL_TRACE_ERROR("%s prep_write rsp bad h %u/%u o %u/%u len %u/%u", __func__, + a->handle, w->handle, a->offset, w->offset, rsp_len, req_len); + cb_data.write.status = BTA_GATT_INVALID_PDU; + } } event = BTA_GATTC_PREP_WRITE_EVT; @@ -1475,12 +1509,17 @@ void bta_gattc_write_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) bta_gattc_free_command_data(p_clcb); bta_gattc_pop_command_to_send(p_clcb); cb_data.write.conn_id = p_clcb->bta_conn_id; - if (p_conn && p_conn->svc_change_descr_handle == cb_data.write.handle) { - if(cb_data.write.status != BTA_GATT_OK) { + if (p_conn && + p_conn->write_remote_svc_change_ccc_in_progress && + p_conn->svc_change_descr_handle == cb_data.write.handle) { + p_conn->write_remote_svc_change_ccc_in_progress = FALSE; + if (cb_data.write.status == BTA_GATT_OK) { + p_conn->write_remote_svc_change_ccc_done = TRUE; + } else { p_conn->write_remote_svc_change_ccc_done = FALSE; APPL_TRACE_ERROR("service change write ccc failed"); } - return; + return; /* internal CCC write: don't forward to application */ } /* write complete, callback */ ( *p_clcb->p_rcb->p_cback)(event, (tBTA_GATTC *)&cb_data); @@ -1497,7 +1536,7 @@ void bta_gattc_write_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) *******************************************************************************/ void bta_gattc_exec_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) { - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; //free the command data store in the queue. bta_gattc_free_command_data(p_clcb); bta_gattc_pop_command_to_send(p_clcb); @@ -1522,7 +1561,7 @@ void bta_gattc_exec_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) *******************************************************************************/ void bta_gattc_cfg_mtu_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data) { - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; //free the command data store in the queue. bta_gattc_free_command_data(p_clcb); bta_gattc_pop_command_to_send(p_clcb); @@ -1650,7 +1689,7 @@ void bta_gattc_ignore_op_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) void bta_gattc_search(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) { tBTA_GATT_STATUS status = GATT_INTERNAL_ERROR; - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; APPL_TRACE_DEBUG("bta_gattc_search conn_id=%d", p_clcb->bta_conn_id); if (p_clcb->p_srcb && p_clcb->p_srcb->p_srvc_cache) { status = BTA_GATT_OK; @@ -1782,7 +1821,7 @@ void bta_gattc_deregister_cmpl(tBTA_GATTC_RCB *p_clreg) { tBTA_GATTC_CB *p_cb = &bta_gattc_cb; tBTA_GATTC_IF client_if = p_clreg->client_if; - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; tBTA_GATTC_CBACK *p_cback = p_clreg->p_cback; memset(&cb_data, 0, sizeof(tBTA_GATTC)); @@ -1910,7 +1949,7 @@ static void bta_gattc_enc_cmpl_cback(tGATT_IF gattc_if, BD_ADDR bda) *******************************************************************************/ void bta_gattc_process_api_refresh(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_msg) { - tBTA_GATTC_SERV *p_srvc_cb = bta_gattc_find_srvr_cache(p_msg->api_refresh.remote_bda); + tBTA_GATTC_SERV *p_srvc_cb; tBTA_GATTC_CLCB *p_clcb = &bta_gattc_cb.clcb[0]; BOOLEAN found = FALSE; UINT8 i; @@ -1918,6 +1957,14 @@ void bta_gattc_process_api_refresh(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_msg) APPL_TRACE_DEBUG("%s", __func__); +#if (GATTC_CACHE_NVS == TRUE) + if (p_msg->api_refresh.erase_flash) { + bta_gattc_cache_reset(p_msg->api_refresh.remote_bda); + } +#endif + + p_srvc_cb = bta_gattc_find_srvr_cache(p_msg->api_refresh.remote_bda); + if (p_srvc_cb != NULL) { /* try to find a CLCB */ if (p_srvc_cb->connected && p_srvc_cb->num_clcb != 0) { @@ -2014,11 +2061,17 @@ void bta_gattc_process_api_cache_get_addr_list(tBTA_GATTC_CB *p_cb, tBTA_GATTC_D *******************************************************************************/ void bta_gattc_process_api_cache_clean(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_msg) { - tBTA_GATTC_SERV *p_srvc_cb = bta_gattc_find_srvr_cache(p_msg->api_clean.remote_bda); + tBTA_GATTC_SERV *p_srvc_cb; UNUSED(p_cb); APPL_TRACE_DEBUG("%s", __func__); +#if (GATTC_CACHE_NVS == TRUE) + bta_gattc_cache_reset(p_msg->api_clean.remote_bda); +#endif + + p_srvc_cb = bta_gattc_find_srvr_cache(p_msg->api_clean.remote_bda); + if (p_srvc_cb != NULL && p_srvc_cb->p_srvc_cache != NULL) { //mark it and delete the cache */ list_free(p_srvc_cb->p_srvc_cache); @@ -2094,6 +2147,8 @@ BOOLEAN bta_gattc_process_srvc_chg_ind(UINT16 conn_id, tBTA_GATTC_CONN *p_conn = bta_gattc_conn_find(p_clcb->bda); if(p_conn) { p_conn->write_remote_svc_change_ccc_done = FALSE; + p_conn->write_remote_svc_change_ccc_in_progress = FALSE; + p_conn->svc_change_descr_handle = 0; } bta_gattc_sm_execute(p_clcb, BTA_GATTC_INT_DISCOVER_EVT, NULL); } @@ -2302,7 +2357,7 @@ static void bta_gattc_cmpl_sendmsg(UINT16 conn_id, tGATTC_OPTYPE op, ********************************************************************************/ static void bta_gattc_cong_cback (UINT16 conn_id, BOOLEAN congested) { - tBTA_GATTC cb_data; + tBTA_GATTC cb_data = {0}; cb_data.congest.conn_id = conn_id; cb_data.congest.congested = congested; btc_gattc_congest_callback(&cb_data); @@ -2345,6 +2400,8 @@ void bta_gattc_init_clcb_conn(UINT8 cif, BD_ADDR remote_bda) tBTA_GATTC_DATA gattc_data; UINT16 conn_id; + memset(&gattc_data, 0, sizeof(gattc_data)); + /* should always get the connection ID */ if (GATT_GetConnIdIfConnected(cif, remote_bda, &conn_id, BTA_GATT_TRANSPORT_LE) == FALSE) { APPL_TRACE_ERROR("bta_gattc_init_clcb_conn ERROR: not a connected device"); @@ -2416,7 +2473,8 @@ tBTA_GATTC_FIND_SERVICE_CB bta_gattc_register_service_change_notify(UINT16 conn_ tBT_UUID gatt_service_change_uuid = {LEN_UUID_16, {GATT_UUID_GATT_SRV_CHGD}}; tBT_UUID gatt_ccc_uuid = {LEN_UUID_16, {GATT_UUID_CHAR_CLIENT_CONFIG}}; tBTA_GATTC_CONN *p_conn = bta_gattc_conn_find_alloc(remote_bda); - if(p_conn && p_conn->write_remote_svc_change_ccc_done) { + if (p_conn && (p_conn->write_remote_svc_change_ccc_done || + p_conn->write_remote_svc_change_ccc_in_progress)) { return SERVICE_CHANGE_CCC_WRITTEN_SUCCESS; } @@ -2483,17 +2541,26 @@ tBTA_GATTC_FIND_SERVICE_CB bta_gattc_register_service_change_notify(UINT16 conn_ } if (gatt_ccc_found == TRUE){ - if (p_conn) { + /* + * The in-progress flag is required so that bta_gattc_write_cmpl can + * recognize this internal CCC write and avoid forwarding the response + * to the application as a spurious BTA_GATTC_WRITE_DESCR_EVT. If the + * connection tracking slot could not be obtained (e.g. conn_track is + * full), skip the write entirely instead of issuing an untracked one. + */ + if (p_conn == NULL) { + APPL_TRACE_ERROR("%s: no conn_track, skip ccc", __func__); + result = SERVICE_CHANGE_WRITE_CCC_FAILED; + } else { p_conn->svc_change_descr_handle = p_desc->handle; - p_conn->write_remote_svc_change_ccc_done = TRUE; + p_conn->write_remote_svc_change_ccc_in_progress = TRUE; + result = SERVICE_CHANGE_CCC_WRITTEN_SUCCESS; + uint16_t indicate_value = GATT_CLT_CONFIG_INDICATION; + tBTA_GATT_UNFMT indicate_v; + indicate_v.len = 2; + indicate_v.p_value = (uint8_t *)&indicate_value; + BTA_GATTC_WriteCharDescr (conn_id, p_desc->handle, BTA_GATTC_TYPE_WRITE, &indicate_v, BTA_GATT_AUTH_REQ_NONE); } - result = SERVICE_CHANGE_CCC_WRITTEN_SUCCESS; - uint16_t indicate_value = GATT_CLT_CONFIG_INDICATION; - tBTA_GATT_UNFMT indicate_v; - indicate_v.len = 2; - indicate_v.p_value = (uint8_t *)&indicate_value; - BTA_GATTC_WriteCharDescr (conn_id, p_desc->handle, BTA_GATTC_TYPE_WRITE, &indicate_v, BTA_GATT_AUTH_REQ_NONE); - } else if (gatt_service_change_found == TRUE) { /* Gatt service char found, but service change char ccc not found, 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 b72c62c9ec2..6eb81b24311 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_api.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_api.c @@ -91,6 +91,7 @@ void BTA_GATTC_AppRegister(tBT_UUID *p_app_uuid, tBTA_GATTC_CBACK *p_client_cb) } if ((p_buf = (tBTA_GATTC_API_REG *) osi_malloc(sizeof(tBTA_GATTC_API_REG))) != NULL) { + memset(p_buf, 0, sizeof(*p_buf)); p_buf->hdr.event = BTA_GATTC_API_REG_EVT; if (p_app_uuid != NULL) { memcpy(&p_buf->app_uuid, p_app_uuid, sizeof(tBT_UUID)); @@ -206,6 +207,7 @@ void BTA_GATTC_CancelOpen(tBTA_GATTC_IF client_if, BD_ADDR remote_bda, BOOLEAN i tBTA_GATTC_API_CANCEL_OPEN *p_buf; if ((p_buf = (tBTA_GATTC_API_CANCEL_OPEN *) osi_malloc(sizeof(tBTA_GATTC_API_CANCEL_OPEN))) != NULL) { + memset(p_buf, 0, sizeof(tBTA_GATTC_API_CANCEL_OPEN)); p_buf->hdr.event = BTA_GATTC_API_CANCEL_OPEN_EVT; p_buf->client_if = client_if; @@ -780,6 +782,7 @@ void BTA_GATTC_PrepareWrite (UINT16 conn_id, UINT16 handle, p_buf->handle = handle; p_buf->write_type = BTA_GATTC_WRITE_PREPARE; + p_buf->cmpl_evt = BTA_GATTC_PREP_WRITE_EVT; p_buf->offset = offset; p_buf->len = len; @@ -827,6 +830,7 @@ void BTA_GATTC_PrepareWriteCharDescr (UINT16 conn_id, UINT16 handle, p_buf->auth_req = auth_req; p_buf->handle = handle; p_buf->write_type = BTA_GATTC_WRITE_PREPARE; + p_buf->cmpl_evt = BTA_GATTC_PREP_WRITE_EVT; p_buf->offset = offset; if (p_data && p_data->len != 0) { @@ -1043,12 +1047,6 @@ tBTA_GATT_STATUS BTA_GATTC_DeregisterForNotifications (tBTA_GATTC_IF client_if, *******************************************************************************/ void BTA_GATTC_Refresh(BD_ADDR remote_bda, bool erase_flash) { -#if(GATTC_CACHE_NVS == TRUE) - if(erase_flash) { - /* used to reset cache in application */ - bta_gattc_cache_reset(remote_bda); - } -#endif //If the registration callback is NULL, return if(bta_sys_is_register(BTA_ID_GATTC) == FALSE) { return; @@ -1056,8 +1054,10 @@ void BTA_GATTC_Refresh(BD_ADDR remote_bda, bool erase_flash) tBTA_GATTC_API_CACHE_REFRESH *p_buf; if ((p_buf = (tBTA_GATTC_API_CACHE_REFRESH *) osi_malloc(sizeof(tBTA_GATTC_API_CACHE_REFRESH))) != NULL) { + memset(p_buf, 0, sizeof(tBTA_GATTC_API_CACHE_REFRESH)); p_buf->hdr.event = BTA_GATTC_API_REFRESH_EVT; memcpy(p_buf->remote_bda, remote_bda, BD_ADDR_LEN); + p_buf->erase_flash = erase_flash ? TRUE : FALSE; bta_sys_sendmsg(p_buf); } @@ -1106,11 +1106,6 @@ void BTA_GATTC_CacheGetAddrList(tBTA_GATTC_IF client_if) *******************************************************************************/ void BTA_GATTC_Clean(BD_ADDR remote_bda) { -#if(GATTC_CACHE_NVS == TRUE) - /* used to reset cache in application */ - bta_gattc_cache_reset(remote_bda); -#endif - tBTA_GATTC_API_CACHE_CLEAN *p_buf; if ((p_buf = (tBTA_GATTC_API_CACHE_CLEAN *) osi_malloc(sizeof(tBTA_GATTC_API_CACHE_CLEAN))) != NULL) { 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 aebffce50a8..76da820b701 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_cache.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_cache.c @@ -29,6 +29,7 @@ //#if( defined GATTC_CACHE_NVS ) && (GATTC_CACHE_NVS == TRUE) #include +#include #include "bta/utl.h" #include "bta/bta_sys.h" #include "stack/sdp_api.h" @@ -45,6 +46,7 @@ // #include "osi/include/log.h" static void bta_gattc_char_dscpt_disc_cmpl(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb); +static tBTA_GATT_STATUS bta_gattc_incl_srvc_disc_cmpl(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb); extern void bta_to_btif_uuid(bt_uuid_t *p_dest, tBT_UUID *p_src); static size_t bta_gattc_get_db_size_with_type(list_t *services, bt_gatt_db_attribute_type_t type, @@ -66,15 +68,8 @@ void bta_gattc_fill_gatt_db_el(btgatt_db_element_t *p_attr, static tBTA_GATT_STATUS bta_gattc_sdp_service_disc(UINT16 conn_id, tBTA_GATTC_SERV *p_server_cb); #define BTA_GATT_SDP_DB_SIZE 4096 #endif ///SDP_INCLUDED == TRUE -#define GATT_CACHE_PREFIX "/data/misc/bluetooth/gatt_cache_" #define GATT_CACHE_VERSION 2 -static void bta_gattc_generate_cache_file_name(char *buffer, BD_ADDR bda) -{ - sprintf(buffer, "%s%02x%02x%02x%02x%02x%02x", GATT_CACHE_PREFIX, - bda[0], bda[1], bda[2], bda[3], bda[4], bda[5]); -} - /***************************************************************************** ** Constants and data types *****************************************************************************/ @@ -131,6 +126,10 @@ bool display_cache_service(void *data, void *context) *******************************************************************************/ static void bta_gattc_display_cache_server(list_t *p_cache) { + if (p_cache == NULL) { + APPL_TRACE_ERROR("Server Cache: (null)"); + return; + } APPL_TRACE_ERROR("<================Start Server Cache =============>"); list_foreach(p_cache, display_cache_service, NULL); APPL_TRACE_ERROR("<================End Server Cache =============>"); @@ -146,15 +145,57 @@ static void bta_gattc_display_cache_server(list_t *p_cache) ** Returns none. ** *******************************************************************************/ -static void bta_gattc_display_explore_record(tBTA_GATTC_ATTR_REC *p_rec, UINT8 num_rec) +static void bta_gattc_uuid_to_str(const tBT_UUID *uuid, char *buf, size_t buf_len) { - UINT8 i; + int x = 0; + + if (buf == NULL || buf_len == 0) { + return; + } + + if (uuid == NULL) { + snprintf(buf, buf_len, "NULL"); + return; + } + + if (uuid->len == LEN_UUID_16) { + snprintf(buf, buf_len, "0x%04x", uuid->uu.uuid16); + } else if (uuid->len == LEN_UUID_32) { + snprintf(buf, buf_len, "0x%08x", (unsigned int)uuid->uu.uuid32); + } else if (uuid->len == LEN_UUID_128) { + x += snprintf(&buf[x], buf_len - (size_t)x, + "0x%02x%02x%02x%02x%02x%02x%02x%02x", + uuid->uu.uuid128[15], uuid->uu.uuid128[14], + uuid->uu.uuid128[13], uuid->uu.uuid128[12], + uuid->uu.uuid128[11], uuid->uu.uuid128[10], + uuid->uu.uuid128[9], uuid->uu.uuid128[8]); + if ((size_t)x < buf_len) { + (void)snprintf(&buf[x], buf_len - (size_t)x, + "%02x%02x%02x%02x%02x%02x%02x%02x", + uuid->uu.uuid128[7], uuid->uu.uuid128[6], + uuid->uu.uuid128[5], uuid->uu.uuid128[4], + uuid->uu.uuid128[3], uuid->uu.uuid128[2], + uuid->uu.uuid128[1], uuid->uu.uuid128[0]); + } + } else { + snprintf(buf, buf_len, "Unknown UUID len=%u", (unsigned)uuid->len); + } +} + +static void bta_gattc_display_explore_record(tBTA_GATTC_ATTR_REC *p_rec, UINT16 num_rec) +{ + UINT16 i; tBTA_GATTC_ATTR_REC *pp = p_rec; APPL_TRACE_ERROR("<================Start Explore Queue =============>"); for (i = 0; i < num_rec; i ++, pp ++) { - APPL_TRACE_ERROR("\t rec[%d] uuid[0x%04x] s_handle[%d] e_handle[%d] is_primary[%d]", - i + 1, pp->uuid.uu.uuid16, pp->s_handle, pp->e_handle, pp->is_primary); + char uuid_buf[50]; + bta_gattc_uuid_to_str(&pp->uuid, uuid_buf, sizeof(uuid_buf)); + + APPL_TRACE_ERROR("\t rec[%u] uuid[%s] s_handle[%u] e_handle[%u] is_primary[%u]", + (unsigned)(i + 1), uuid_buf, + (unsigned)pp->s_handle, (unsigned)pp->e_handle, + (unsigned)pp->is_primary); } APPL_TRACE_ERROR("<================ End Explore Queue =============>"); APPL_TRACE_ERROR(" "); @@ -215,38 +256,66 @@ static void bta_gattc_free(void *ptr) osi_free(ptr); } -void bta_gattc_insert_sec_service_to_cache(list_t *services, tBTA_GATTC_SERVICE *p_new_srvc) +/* Bluedroid host operates under the design assumption that it must not run + * out of memory; OOM is treated as an unrecoverable condition handled + * upstream by the allocator policy. The list_append / list_prepend / + * list_insert_after results are therefore forwarded as-is via the bool + * return value (NULL preconditions also surface as `false`), leaving the + * decision to roll back the orphaned service or to propagate the failure + * up the call stack to the caller. */ +bool bta_gattc_insert_sec_service_to_cache(list_t *services, tBTA_GATTC_SERVICE *p_new_srvc) { - // services/p_new_srvc is NULL if (!services || !p_new_srvc) { APPL_TRACE_ERROR("%s services/p_new_srvc is NULL", __func__); - return; + return false; } - //list is empty + + /* Order by s_handle so traversals (e.g. bta_gattc_get_db_with_operation) may rely on + * monotonic s_handle; gap-based insert failed when ranges overlapped or nested. */ if (list_is_empty(services)) { - list_append(services, p_new_srvc); - } else { - //check the first service - list_node_t *sn = list_begin(services); + return list_append(services, p_new_srvc); + } + + list_node_t *prev = NULL; + 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 && p_new_srvc->e_handle < service->s_handle) { - list_prepend(services, p_new_srvc); - } else { - for (list_node_t *sn = list_begin(services); sn != list_end(services); sn = list_next(sn)) { - list_node_t *next_sn = list_next(sn); - if(next_sn == list_end(services)) { - list_append(services, p_new_srvc); - return; - } - tBTA_GATTC_SERVICE *service = list_node(sn); - tBTA_GATTC_SERVICE *next_service = list_node(next_sn); - if (p_new_srvc->s_handle > service->e_handle && p_new_srvc->e_handle < next_service->s_handle) { - list_insert_after(services, sn, p_new_srvc); - return; - } + if (p_new_srvc->s_handle < service->s_handle) { + if (prev == NULL) { + return list_prepend(services, p_new_srvc); + } else { + return list_insert_after(services, prev, p_new_srvc); } } + prev = sn; } + return list_append(services, p_new_srvc); +} + +/******************************************************************************* +** +** Function bta_gattc_next_service_in_list +** +** Description Return the service node after |cur| in |services| list order. +** +** Returns Next service, or NULL if none. +** +*******************************************************************************/ +static tBTA_GATTC_SERVICE *bta_gattc_next_service_in_list(const list_t *services, + const tBTA_GATTC_SERVICE *cur) +{ + if (!services) { + return NULL; + } + for (list_node_t *sn = list_begin(services); sn != list_end(services); sn = list_next(sn)) { + if (list_node(sn) == cur) { + list_node_t *next_sn = list_next(sn); + if (next_sn == list_end(services)) { + return NULL; + } + return (tBTA_GATTC_SERVICE *)list_node(next_sn); + } + } + return NULL; } /******************************************************************************* @@ -282,6 +351,18 @@ static tBTA_GATT_STATUS bta_gattc_add_srvc_to_cache(tBTA_GATTC_SERV *p_srvc_cb, p_new_srvc->characteristics = list_new(characteristic_free); p_new_srvc->included_svc = list_new(bta_gattc_free); + if (!p_new_srvc->characteristics || !p_new_srvc->included_svc) { + 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; + } + if (p_srvc_cb->p_srvc_cache == NULL) { p_srvc_cb->p_srvc_cache = list_new(service_free); } @@ -298,11 +379,22 @@ static tBTA_GATT_STATUS bta_gattc_add_srvc_to_cache(tBTA_GATTC_SERV *p_srvc_cb, return BTA_GATT_NO_RESOURCES; } - if(is_primary) { - list_append(p_srvc_cb->p_srvc_cache, p_new_srvc); + bool inserted; + if (is_primary) { + inserted = list_append(p_srvc_cb->p_srvc_cache, p_new_srvc); } else { //add secondary service into list - bta_gattc_insert_sec_service_to_cache(p_srvc_cb->p_srvc_cache, p_new_srvc); + inserted = bta_gattc_insert_sec_service_to_cache(p_srvc_cb->p_srvc_cache, p_new_srvc); + } + if (!inserted) { + /* p_new_srvc still owns its characteristics/included_svc lists; release + * them here since service_free will not run for a node that never + * entered p_srvc_cache. */ + APPL_TRACE_WARNING("%s(), no resource.", __func__); + list_free(p_new_srvc->characteristics); + list_free(p_new_srvc->included_svc); + osi_free(p_new_srvc); + return BTA_GATT_NO_RESOURCES; } return BTA_GATT_OK; } @@ -329,7 +421,15 @@ static tBTA_GATT_STATUS bta_gattc_add_char_to_cache(tBTA_GATTC_SERV *p_srvc_cb, This is just a temporary workaround. */ if (service->e_handle < value_handle) { - service->e_handle = value_handle; + UINT16 new_e = value_handle; + tBTA_GATTC_SERVICE *next_srvc = bta_gattc_next_service_in_list(p_srvc_cb->p_srvc_cache, service); + if (next_srvc != NULL && next_srvc->s_handle > service->s_handle + && new_e >= next_srvc->s_handle && next_srvc->s_handle > 0) { + new_e = next_srvc->s_handle - 1; + } + if (new_e > service->e_handle) { + service->e_handle = new_e; + } } tBTA_GATTC_CHARACTERISTIC *characteristic = osi_malloc(sizeof(tBTA_GATTC_CHARACTERISTIC)); @@ -343,6 +443,11 @@ 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; + if (!characteristic->descriptors) { + APPL_TRACE_WARNING("%s(), no resource.", __func__); + osi_free(characteristic); + return BTA_GATT_NO_RESOURCES; + } if (!list_append(service->characteristics, characteristic)) { APPL_TRACE_WARNING("%s(), no resource.", __func__); if (characteristic->descriptors) { @@ -402,7 +507,11 @@ static tBTA_GATT_STATUS bta_gattc_add_attr_to_cache(tBTA_GATTC_SERV *p_srvc_cb, p_srvc_cb->update_incl_srvc = true; } - list_append(service->included_svc, isvc); + if (!service->included_svc || !list_append(service->included_svc, isvc)) { + APPL_TRACE_WARNING("%s(), no resource.", __func__); + osi_free(isvc); + return BTA_GATT_NO_RESOURCES; + } } else if (type == BTA_GATTC_ATTR_TYPE_CHAR_DESCR) { tBTA_GATTC_DESCRIPTOR *descriptor = osi_malloc(sizeof(tBTA_GATTC_DESCRIPTOR)); @@ -424,7 +533,11 @@ static tBTA_GATT_STATUS bta_gattc_add_attr_to_cache(tBTA_GATTC_SERV *p_srvc_cb, tBTA_GATTC_CHARACTERISTIC *char_node = list_back(service->characteristics); descriptor->characteristic = char_node; - list_append(char_node->descriptors, descriptor); + if (!char_node->descriptors || !list_append(char_node->descriptors, descriptor)) { + APPL_TRACE_WARNING("%s(), no resource.", __func__); + osi_free(descriptor); + return BTA_GATT_NO_RESOURCES; + } } return BTA_GATT_OK; } @@ -447,6 +560,17 @@ void bta_gattc_get_disc_range(tBTA_GATTC_SERV *p_srvc_cb, UINT16 *p_s_hdl, UINT1 *p_s_hdl = p_rec->s_handle; } else { p_rec = p_srvc_cb->p_srvc_list + p_srvc_cb->cur_char_idx; + if (p_rec->s_handle == 0xFFFF) { + /* Per BT Core Spec (Vol 3, Part G, 3.3): a Characteristic + * Declaration cannot reside at handle 0xFFFF because its + * mandatory Value Declaration must immediately follow it. + * Treat this as corrupted/unexpected cache data and signal + * an invalid range to the caller. */ + APPL_TRACE_ERROR("%s: char s_handle 0xFFFF", __func__); + *p_s_hdl = 0; + *p_e_hdl = 0; + return; + } *p_s_hdl = p_rec->s_handle + 1; } @@ -510,7 +634,9 @@ tBTA_GATT_STATUS bta_gattc_discover_procedure(UINT16 conn_id, tBTA_GATTC_SERV *p bta_gattc_get_disc_range(p_server_cb, ¶m.s_handle, ¶m.e_handle, is_service); - if (param.s_handle > param.e_handle) { + if (!GATT_HANDLE_IS_VALID(param.s_handle) || + !GATT_HANDLE_IS_VALID(param.e_handle) || + (param.s_handle > param.e_handle)) { return GATT_ERROR; } } @@ -569,7 +695,9 @@ 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; + if (!service || !service->included_svc || list_is_empty(service->included_svc)) { + continue; + } 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); @@ -619,7 +747,38 @@ static void bta_gattc_explore_srvc(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb) &p_rec->uuid, p_rec->is_primary) == 0) { /* start discovering included services */ - bta_gattc_start_disc_include_srvc(conn_id, p_srvc_cb); + tBTA_GATT_STATUS status = bta_gattc_start_disc_include_srvc(conn_id, p_srvc_cb); + if (status == 0) { + return; + } + + /* + * If included service discovery cannot be started (e.g. busy/error), + * try to advance the state machine by directly kicking off + * characteristic discovery. If that also succeeds, the discovery + * will be driven forward by its async completion callback and we + * MUST NOT fall through to the finalization code below (doing so + * would reset the state machine while an async procedure is still + * pending). + */ + tBTA_GATT_STATUS disc_status = bta_gattc_incl_srvc_disc_cmpl(conn_id, p_srvc_cb); + if (disc_status == BTA_GATT_OK) { + return; + } + + /* + * Both included-service and characteristic discovery failed to + * start synchronously. Falling through to the finalization path + * below would (a) deliver BTA_GATTC_DIS_SRVC_CMPL_EVT with a + * stale (likely SUCCESS) status, (b) persist an incomplete cache + * to NVS when GATTC_CACHE_NVS is enabled, and (c) reset the SRCB + * with BTA_GATT_OK, masking the real failure. Mirror the handling + * in bta_gattc_disc_cmpl_cback() and surface the actual error to + * the upper layer immediately. + */ + APPL_TRACE_ERROR("%s: incl/char disc start failed, st=%d/%d", + __func__, status, disc_status); + bta_gattc_reset_discover_st(p_clcb->p_srcb, disc_status); return; } } @@ -661,15 +820,20 @@ static void bta_gattc_explore_srvc(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb) ** ** Description process the relationship discovery complete event ** -** Returns status +** Returns GATT_SUCCESS if the next discovery step (characteristic +** discovery) was successfully kicked off; otherwise the +** error returned by bta_gattc_start_disc_char so that the +** caller can decide whether to fall back to the discovery +** finalization path instead of stalling on a callback that +** will never come. ** *******************************************************************************/ -static void bta_gattc_incl_srvc_disc_cmpl(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb) +static tBTA_GATT_STATUS bta_gattc_incl_srvc_disc_cmpl(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb) { p_srvc_cb->cur_char_idx = p_srvc_cb->total_srvc; /* start discoverying characteristic */ - bta_gattc_start_disc_char(conn_id, p_srvc_cb); + return bta_gattc_start_disc_char(conn_id, p_srvc_cb); } /******************************************************************************* ** @@ -687,11 +851,24 @@ static void bta_gattc_char_disc_cmpl(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb) /* if there are characteristic needs to be explored */ if (p_srvc_cb->total_char > 0) { /* add the first characteristic into cache */ - bta_gattc_add_char_to_cache (p_srvc_cb, - p_rec->char_decl_handle, - p_rec->s_handle, - &p_rec->uuid, - p_rec->property); + tBTA_GATT_STATUS add_status = bta_gattc_add_char_to_cache(p_srvc_cb, + p_rec->char_decl_handle, + p_rec->s_handle, + &p_rec->uuid, + p_rec->property); + if (add_status != BTA_GATT_OK) { + /* Failed to cache the characteristic (e.g. OOM). Kicking off + * descriptor discovery now would either silently drop the + * descriptors or attach them to the previous characteristic + * via list_back() in bta_gattc_add_attr_to_cache(), leaving the + * cache structurally inconsistent. Abort the procedure so the + * application sees the failure rather than a partially + * populated cache. */ + APPL_TRACE_ERROR("%s: add_char_to_cache fail st=%d", + __func__, add_status); + bta_gattc_reset_discover_st(p_srvc_cb, add_status); + return; + } /* start discoverying characteristic descriptor , if failed, disc for next char */ bta_gattc_start_disc_char_dscp(conn_id, p_srvc_cb); @@ -713,17 +890,31 @@ static void bta_gattc_char_disc_cmpl(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb) static void bta_gattc_char_dscpt_disc_cmpl(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb) { tBTA_GATTC_ATTR_REC *p_rec = NULL; + tBTA_GATT_STATUS add_status = BTA_GATT_OK; /* Recursive function will cause BTU stack overflow when there are a large number of characteristic * without descriptor to discover. So replace it with while function */ while (--p_srvc_cb->total_char > 0) { p_rec = p_srvc_cb->p_srvc_list + (++ p_srvc_cb->cur_char_idx); /* add the next characteristic into cache */ - bta_gattc_add_char_to_cache (p_srvc_cb, - p_rec->char_decl_handle, - p_rec->s_handle, - &p_rec->uuid, - p_rec->property); + add_status = bta_gattc_add_char_to_cache(p_srvc_cb, + p_rec->char_decl_handle, + p_rec->s_handle, + &p_rec->uuid, + p_rec->property); + if (add_status != BTA_GATT_OK) { + /* If we failed to cache the current characteristic, any + * descriptors discovered for it would be appended to the + * previous characteristic (bta_gattc_add_attr_to_cache uses + * list_back() on the service's characteristic list), corrupting + * the cache. Stop the discovery state machine and surface the + * error to the upper layer so the application can react + * instead of silently using an inconsistent cache. */ + APPL_TRACE_ERROR("%s: add_char_to_cache fail st=%d", + __func__, add_status); + bta_gattc_reset_discover_st(p_srvc_cb, add_status); + return; + } /* start to discover next characteristic for descriptor */ if (bta_gattc_discover_procedure(conn_id, p_srvc_cb, GATT_DISC_CHAR_DSCPT) == 0) { /* send att req and wait for att rsp */ @@ -992,7 +1183,10 @@ void bta_gattc_disc_res_cback (UINT16 conn_id, tGATT_DISC_TYPE disc_type, tGATT_ p_srvc_cb = bta_gattc_find_scb_by_cid(conn_id); if (p_srvc_cb != NULL && p_clcb != NULL && p_clcb->state == BTA_GATTC_DISCOVER_ST) { - p_srvc_cb->total_attr++; + /* total_attr is UINT16; saturate so discovery cannot wrap the counter */ + if (p_srvc_cb->total_attr < UINT16_MAX) { + p_srvc_cb->total_attr++; + } switch (disc_type) { case GATT_DISC_SRVC_ALL: /* discover services result, add services into a service list */ @@ -1085,10 +1279,24 @@ void bta_gattc_disc_cmpl_cback (UINT16 conn_id, tGATT_DISC_TYPE disc_type, tGATT bta_gattc_explore_srvc(conn_id, p_srvc_cb); break; - case GATT_DISC_INC_SRVC: - bta_gattc_incl_srvc_disc_cmpl(conn_id, p_srvc_cb); - + case GATT_DISC_INC_SRVC: { + /* + * If chained characteristic discovery cannot be started + * synchronously (e.g. GATT_BUSY / GATT_NO_RESOURCES, or invalid + * handle range returned by bta_gattc_discover_procedure), no + * async completion will ever come back to drive the state + * machine forward. Mirror the handling in bta_gattc_explore_srvc() + * and finalize discovery now (propagating the actual error code) + * so the SRCB does not get stuck in BTA_GATTC_SERV_DISC_ACT until + * the link supervision timeout fires. + */ + tBTA_GATT_STATUS disc_status = bta_gattc_incl_srvc_disc_cmpl(conn_id, p_srvc_cb); + if (disc_status != GATT_SUCCESS) { + APPL_TRACE_ERROR("%s: char disc fail after incl, st=%d", __func__, disc_status); + bta_gattc_reset_discover_st(p_srvc_cb, disc_status); + } break; + } case GATT_DISC_CHAR: #if (defined BTA_GATT_DEBUG && BTA_GATT_DEBUG == TRUE) @@ -1209,6 +1417,9 @@ tBTA_GATTC_CHARACTERISTIC* bta_gattc_get_characteristic_srcb(tBTA_GATTC_SERV *p return NULL; } + if (!service->characteristics) { + return NULL; + } for (list_node_t *cn = list_begin(service->characteristics); cn != list_end(service->characteristics); cn = list_next(cn)) { tBTA_GATTC_CHARACTERISTIC *p_char = list_node(cn); @@ -1240,9 +1451,15 @@ tBTA_GATTC_DESCRIPTOR* bta_gattc_get_descriptor_srcb(tBTA_GATTC_SERV *p_srcb, U return NULL; } + if (!service->characteristics) { + return NULL; + } for (list_node_t *cn = list_begin(service->characteristics); cn != list_end(service->characteristics); cn = list_next(cn)) { tBTA_GATTC_CHARACTERISTIC *p_char = list_node(cn); + if (!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); @@ -1335,6 +1552,39 @@ void bta_gattc_fill_gatt_db_el(btgatt_db_element_t *p_attr, bta_to_btif_uuid(&p_attr->uuid, &uuid); } +/******************************************************************************* +** +** Function bta_gattc_get_db_chk_room +** +** Description bta_gattc_get_db_with_operation allocates using a heuristic +** upper bound; the walk can still emit more elements than that +** bound. If filled >= max_elems, free the buffer and clear +** outputs so the caller returns an empty DB (same shape as no +** matches) instead of heap overflow. +** +** Returns TRUE if there is room for one more element, else FALSE. +** +*******************************************************************************/ +/* Static trip counter: incremented every time the DB element buffer would + * have overflowed. Useful as a "did this ever happen in the field?" smoke + * signal in addition to the ERROR log emitted on each trip. Not an API. */ +static unsigned int s_get_db_chk_room_trips = 0; + +static BOOLEAN bta_gattc_get_db_chk_room(size_t filled, size_t max_elems, void *buffer, + btgatt_db_element_t **char_db, UINT16 *count) +{ + if (filled >= max_elems) { + s_get_db_chk_room_trips++; + APPL_TRACE_ERROR("%s: cap exceeded filled=%u cap=%u trips=%u", + __func__, (unsigned)filled, (unsigned)max_elems, s_get_db_chk_room_trips); + osi_free(buffer); + *char_db = NULL; + *count = 0; + return FALSE; + } + return TRUE; +} + void bta_gattc_get_db_with_operation(UINT16 conn_id, bt_gatt_get_db_op_t op, UINT16 char_handle, @@ -1361,15 +1611,19 @@ void bta_gattc_get_db_with_operation(UINT16 conn_id, return; } - size_t db_size = ((end_handle - start_handle + 1) < p_srcb->total_attr) ? (end_handle - start_handle + 1) : p_srcb->total_attr; - if (!db_size) { + /* Allocation cap: min(handle span, total_attr). May be lower than actual + * matching rows; bta_gattc_get_db_chk_room guards each append. */ + const size_t max_elems = ((end_handle - start_handle + 1) < p_srcb->total_attr) + ? (end_handle - start_handle + 1) + : p_srcb->total_attr; + if (!max_elems) { APPL_TRACE_DEBUG("the db size is 0."); *count = 0; *char_db = NULL; return; } - void *buffer = osi_malloc(db_size*sizeof(btgatt_db_element_t)); + void *buffer = osi_malloc(max_elems * sizeof(btgatt_db_element_t)); if (!buffer) { APPL_TRACE_DEBUG("the buffer is NULL."); @@ -1378,7 +1632,7 @@ void bta_gattc_get_db_with_operation(UINT16 conn_id, return; } btgatt_db_element_t *curr_db_attr = buffer; - db_size = 0; + size_t filled = 0; /* number of elements written into buffer so far */ for (list_node_t *sn = list_begin(p_srcb->p_srvc_cache); sn != list_end(p_srcb->p_srvc_cache); sn = list_next(sn)) { tBTA_GATTC_SERVICE *p_cur_srvc = list_node(sn); @@ -1405,10 +1659,13 @@ void bta_gattc_get_db_with_operation(UINT16 conn_id, if (p_isvc->handle > end_handle) { *char_db = buffer; - *count = db_size; + *count = filled; return; } if (!incl_uuid || bta_gattc_uuid_compare(&p_isvc->uuid, incl_uuid, TRUE)) { + if (!bta_gattc_get_db_chk_room(filled, max_elems, buffer, char_db, count)) { + return; + } bta_gattc_fill_gatt_db_el(curr_db_attr, BTGATT_DB_INCLUDED_SERVICE, p_isvc->handle, @@ -1418,7 +1675,7 @@ void bta_gattc_get_db_with_operation(UINT16 conn_id, p_isvc->uuid, 0 /* property */); curr_db_attr++; - db_size++; + filled++; } } continue; @@ -1439,12 +1696,15 @@ void bta_gattc_get_db_with_operation(UINT16 conn_id, if (p_char->handle > end_handle) { *char_db = buffer; - *count = db_size; + *count = filled; return; } if ((op == GATT_OP_GET_ALL_CHAR || op == GATT_OP_GET_CHAR_BY_UUID) && (char_uuid == NULL || bta_gattc_uuid_compare(&p_char->uuid, char_uuid, TRUE))) { APPL_TRACE_DEBUG("%s(), uuid match.", __func__); + if (!bta_gattc_get_db_chk_room(filled, max_elems, buffer, char_db, count)) { + return; + } bta_gattc_fill_gatt_db_el(curr_db_attr, BTGATT_DB_CHARACTERISTIC, p_char->handle, @@ -1454,7 +1714,7 @@ void bta_gattc_get_db_with_operation(UINT16 conn_id, p_char->uuid, p_char->properties); curr_db_attr++; - db_size++; + filled++; continue; } @@ -1481,12 +1741,15 @@ void bta_gattc_get_db_with_operation(UINT16 conn_id, } if (p_desc->handle > end_handle) { *char_db = buffer; - *count = db_size; + *count = filled; return; } if (((op == GATT_OP_GET_ALL_DESCRI || op == GATT_OP_GET_DESCRI_BY_UUID) && (descr_uuid == NULL || bta_gattc_uuid_compare(&p_desc->uuid, descr_uuid, TRUE))) || (op == GATT_OP_GET_DESCRI_BY_HANDLE && bta_gattc_uuid_compare(&p_desc->uuid, descr_uuid, TRUE))) { + if (!bta_gattc_get_db_chk_room(filled, max_elems, buffer, char_db, count)) { + return; + } bta_gattc_fill_gatt_db_el(curr_db_attr, BTGATT_DB_DESCRIPTOR, p_desc->handle, @@ -1496,7 +1759,7 @@ void bta_gattc_get_db_with_operation(UINT16 conn_id, p_desc->uuid, 0 /* property */); curr_db_attr++; - db_size++; + filled++; } } } @@ -1505,7 +1768,7 @@ void bta_gattc_get_db_with_operation(UINT16 conn_id, } *char_db = buffer; - *count = db_size; + *count = filled; } static size_t bta_gattc_get_db_size_with_type(list_t *services, @@ -1568,19 +1831,23 @@ static size_t bta_gattc_get_db_size_with_type(list_t *services, 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; } - if ((type == BTGATT_DB_CHARACTERISTIC) && bta_gattc_uuid_compare(&p_char->uuid, char_uuid, TRUE)) { + /* Count the characteristic only when its handle is in range. */ + if ((p_char->handle >= start_handle) && + (type == BTGATT_DB_CHARACTERISTIC) && + bta_gattc_uuid_compare(&p_char->uuid, char_uuid, TRUE)) { db_size++; continue; } + /* Descriptor handles are strictly greater than their parent characteristic + * declaration handle, so an in-range descriptor may still exist when the + * parent's handle is below start_handle. Iterate descriptors regardless of + * p_char->handle vs start_handle to stay consistent with the paired filler + * (bta_gattc_get_db_with_operation) which already iterates descriptors in that case. */ if (p_char->descriptors && (type == BTGATT_DB_DESCRIPTOR) && bta_gattc_uuid_compare(&p_char->uuid, char_uuid, TRUE)) { for (list_node_t *dn = list_begin(p_char->descriptors); dn != list_end(p_char->descriptors); dn = list_next(dn)) { @@ -1643,43 +1910,14 @@ static size_t bta_gattc_get_db_size(list_t *services, break; } - /* 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++; - - 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->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++; - } - } - } + /* Count service declaration only when its handle lies in the requested range. Still count + * included services, characteristics, and descriptors in-range when s_handle is before start_handle. */ + if (p_cur_srvc->s_handle >= start_handle && p_cur_srvc->s_handle <= end_handle) { + db_size++; } - if (p_cur_srvc->included_svc) { + /* GATT attribute order: included service declarations before characteristic declarations. */ + if (p_cur_srvc->included_svc && !list_is_empty(p_cur_srvc->included_svc)) { for (list_node_t *isn = list_begin(p_cur_srvc->included_svc); isn != list_end(p_cur_srvc->included_svc); isn = list_next(isn)) { tBTA_GATTC_INCLUDED_SVC *p_isvc = list_node(isn); @@ -1689,11 +1927,45 @@ static size_t bta_gattc_get_db_size(list_t *services, } if (p_isvc->handle > end_handle) { - return db_size; + break; } db_size++; } } + + 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 > end_handle) { + break; + } + + /* Count the characteristic declaration only when its handle is in range. + * Descriptors must still be examined when the parent characteristic's + * handle is below start_handle, because descriptor handles are strictly + * greater than the characteristic declaration handle and may themselves + * fall inside [start_handle, end_handle]. */ + if (p_char->handle >= start_handle) { + 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_desc->handle > end_handle) { + break; + } + db_size++; + } + } + } + } } return db_size; @@ -1778,6 +2050,15 @@ static void bta_gattc_get_gatt_db_impl(tBTA_GATTC_SERV *p_srvc_cb, size_t db_size = bta_gattc_get_db_size(p_srvc_cb->p_srvc_cache, start_handle, end_handle); + /* No attribute falls in the requested handle range. Return an empty result + * without invoking osi_malloc(0), which would yield NULL on ESP-IDF and + * be misreported as an allocation failure. */ + if (db_size == 0) { + *db = NULL; + *count = 0; + return; + } + void* buffer = osi_malloc(db_size * sizeof(btgatt_db_element_t)); if (!buffer) { APPL_TRACE_WARNING("%s(), no resource.", __func__); @@ -1799,47 +2080,69 @@ 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; + /* Emit service declaration only when its handle lies in the requested range. */ + if (p_cur_srvc->s_handle >= start_handle && p_cur_srvc->s_handle <= end_handle) { + bta_gattc_fill_gatt_db_el(curr_db_attr, + p_cur_srvc->is_primary ? + BTGATT_DB_PRIMARY_SERVICE : + BTGATT_DB_SECONDARY_SERVICE, + 0 /* att_handle */, + p_cur_srvc->s_handle, + p_cur_srvc->e_handle, + p_cur_srvc->s_handle, + p_cur_srvc->uuid, + 0 /* prop */); + curr_db_attr++; } - bta_gattc_fill_gatt_db_el(curr_db_attr, - p_cur_srvc->is_primary ? - BTGATT_DB_PRIMARY_SERVICE : - BTGATT_DB_SECONDARY_SERVICE, - 0 /* att_handle */, - p_cur_srvc->s_handle, - p_cur_srvc->e_handle, - p_cur_srvc->s_handle, - p_cur_srvc->uuid, - 0 /* prop */); - curr_db_attr++; + /* GATT attribute order: included service declarations before characteristic declarations. */ + if (p_cur_srvc->included_svc && !list_is_empty(p_cur_srvc->included_svc)) { + for (list_node_t *isn = list_begin(p_cur_srvc->included_svc); + isn != list_end(p_cur_srvc->included_svc); isn = list_next(isn)) { + tBTA_GATTC_INCLUDED_SVC *p_isvc = list_node(isn); + + if (p_isvc->handle < start_handle) { + continue; + } + + if (p_isvc->handle > end_handle) { + break; + } + bta_gattc_fill_gatt_db_el(curr_db_attr, + BTGATT_DB_INCLUDED_SERVICE, + p_isvc->handle, + p_isvc->incl_srvc_s_handle, + p_isvc->incl_srvc_e_handle, + p_isvc->handle, + p_isvc->uuid, + 0 /* property */); + curr_db_attr++; + } + } 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) { + break; } - if (p_char->handle > end_handle) { - *db = buffer; - *count = db_size; - return; + /* Emit the characteristic declaration only when its handle is in range, + * but still emit its in-range descriptors. This mirrors bta_gattc_get_db_size + * so allocated db_size and emitted entries stay consistent. */ + if (p_char->handle >= start_handle) { + bta_gattc_fill_gatt_db_el(curr_db_attr, + BTGATT_DB_CHARACTERISTIC, + p_char->handle, + 0 /* s_handle */, + 0 /* e_handle */, + p_char->handle, + p_char->uuid, + p_char->properties); + curr_db_attr++; } - bta_gattc_fill_gatt_db_el(curr_db_attr, - BTGATT_DB_CHARACTERISTIC, - p_char->handle, - 0 /* s_handle */, - 0 /* e_handle */, - p_char->handle, - p_char->uuid, - p_char->properties); - curr_db_attr++; if (!p_char->descriptors || list_is_empty(p_char->descriptors)) { continue; @@ -1854,9 +2157,7 @@ static void bta_gattc_get_gatt_db_impl(tBTA_GATTC_SERV *p_srvc_cb, } if (p_desc->handle > end_handle) { - *db = buffer; - *count = db_size; - return; + break; } bta_gattc_fill_gatt_db_el(curr_db_attr, BTGATT_DB_DESCRIPTOR, @@ -1870,34 +2171,6 @@ static void bta_gattc_get_gatt_db_impl(tBTA_GATTC_SERV *p_srvc_cb, } } } - - if (!p_cur_srvc->included_svc || list_is_empty(p_cur_srvc->included_svc)) { - continue; - } - - for (list_node_t *isn = list_begin(p_cur_srvc->included_svc); - isn != list_end(p_cur_srvc->included_svc); isn = list_next(isn)) { - tBTA_GATTC_INCLUDED_SVC *p_isvc = list_node(isn); - - if (p_isvc->handle < start_handle) { - continue; - } - - if (p_isvc->handle > end_handle) { - *db = buffer; - *count = db_size; - return; - } - bta_gattc_fill_gatt_db_el(curr_db_attr, - BTGATT_DB_INCLUDED_SERVICE, - p_isvc->handle, - p_isvc->incl_srvc_s_handle, - p_isvc->incl_srvc_e_handle, - p_isvc->handle, - p_isvc->uuid, - 0 /* property */); - curr_db_attr++; - } } *db = buffer; @@ -1959,12 +2232,19 @@ void bta_gattc_get_gatt_db(UINT16 conn_id, UINT16 start_handle, UINT16 end_handl ** ** Parameters ** -** Returns None. +** Returns BTA_GATT_OK on success. On the first failure of any +** bta_gattc_add_*_to_cache() call (e.g. BTA_GATT_NO_RESOURCES +** on OOM, or GATT_WRONG_STATE on inconsistent NV layout), +** the partially built in-memory cache is freed and the +** underlying error is returned so the caller can fall back +** to a fresh discovery. ** *******************************************************************************/ -void bta_gattc_rebuild_cache(tBTA_GATTC_SERV *p_srvc_cb, UINT16 num_attr, - tBTA_GATTC_NV_ATTR *p_attr) +tBTA_GATT_STATUS bta_gattc_rebuild_cache(tBTA_GATTC_SERV *p_srvc_cb, UINT16 num_attr, + tBTA_GATTC_NV_ATTR *p_attr) { + tBTA_GATT_STATUS status = BTA_GATT_OK; + /* first attribute loading, initialize buffer */ APPL_TRACE_DEBUG("%s: bta_gattc_rebuild_cache, num_attr = %d", __func__, num_attr); @@ -1974,44 +2254,58 @@ void bta_gattc_rebuild_cache(tBTA_GATTC_SERV *p_srvc_cb, UINT16 num_attr, while (num_attr > 0 && p_attr != NULL) { switch (p_attr->attr_type) { case BTA_GATTC_ATTR_TYPE_SRVC: - bta_gattc_add_srvc_to_cache(p_srvc_cb, - p_attr->s_handle, - p_attr->e_handle, - &p_attr->uuid, - p_attr->is_primary); + status = bta_gattc_add_srvc_to_cache(p_srvc_cb, + p_attr->s_handle, + p_attr->e_handle, + &p_attr->uuid, + p_attr->is_primary); break; case BTA_GATTC_ATTR_TYPE_CHAR: //TODO(jpawlowski): store decl_handle properly. - bta_gattc_add_char_to_cache(p_srvc_cb, - p_attr->s_handle, - p_attr->s_handle, - &p_attr->uuid, - p_attr->prop); + status = bta_gattc_add_char_to_cache(p_srvc_cb, + p_attr->s_handle, + p_attr->s_handle, + &p_attr->uuid, + p_attr->prop); break; case BTA_GATTC_ATTR_TYPE_CHAR_DESCR: - bta_gattc_add_attr_to_cache(p_srvc_cb, - p_attr->s_handle, - &p_attr->uuid, - p_attr->prop, - p_attr->incl_srvc_s_handle, - p_attr->incl_srvc_e_handle, - p_attr->attr_type); + status = bta_gattc_add_attr_to_cache(p_srvc_cb, + p_attr->s_handle, + &p_attr->uuid, + p_attr->prop, + p_attr->incl_srvc_s_handle, + p_attr->incl_srvc_e_handle, + p_attr->attr_type); break; case BTA_GATTC_ATTR_TYPE_INCL_SRVC: - bta_gattc_add_attr_to_cache(p_srvc_cb, - p_attr->s_handle, - &p_attr->uuid, - p_attr->prop, - p_attr->incl_srvc_s_handle, - p_attr->incl_srvc_e_handle, - p_attr->attr_type); + status = bta_gattc_add_attr_to_cache(p_srvc_cb, + p_attr->s_handle, + &p_attr->uuid, + p_attr->prop, + p_attr->incl_srvc_s_handle, + p_attr->incl_srvc_e_handle, + p_attr->attr_type); break; } + if (status != BTA_GATT_OK) { + /* Partial rebuild leaves the cache in an inconsistent state + * (e.g. a service without its characteristics, or a char whose + * descriptors would now attach to the wrong parent). Drop the + * partially built cache and let the caller fall back to a fresh + * discovery instead of exposing a corrupted view to the app. */ + APPL_TRACE_ERROR("%s: rebuild abort t=%d st=%d", + __func__, p_attr->attr_type, status); + list_free(p_srvc_cb->p_srvc_cache); + p_srvc_cb->p_srvc_cache = NULL; + return status; + } p_attr ++; num_attr --; } + + return BTA_GATT_OK; } /******************************************************************************* @@ -2056,6 +2350,11 @@ void bta_gattc_cache_save(tBTA_GATTC_SERV *p_srvc_cb, UINT16 conn_id) /* 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); + /* Nothing to persist; skip allocating a zero-byte buffer (osi_malloc(0) + * returns NULL on ESP-IDF and would be misreported as "no resource"). */ + if (db_size == 0) { + return; + } tBTA_GATTC_NV_ATTR *nv_attr = osi_malloc(db_size * sizeof(tBTA_GATTC_NV_ATTR)); // 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) { @@ -2203,9 +2502,15 @@ bool bta_gattc_cache_load(tBTA_GATTC_CLCB *p_clcb) } 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); + /* total_attr and rebuild path are UINT16-wide; refuse oversize NV blobs + * (NVS reads the whole blob, and UINT16 cannot represent the count). */ + if (num_attr > UINT16_MAX) { + APPL_TRACE_ERROR("%s: NV attr cnt>%u", + __func__, (unsigned)UINT16_MAX); + return false; + } + p_clcb->p_srcb->total_attr = (UINT16)num_attr; + APPL_TRACE_DEBUG("%s(), index = %x, num_attr = %u", __func__, index, (unsigned)num_attr); if ((attr = osi_malloc(sizeof(tBTA_GATTC_NV_ATTR) * num_attr)) == NULL) { APPL_TRACE_ERROR("%s, No Memory.", __func__); return false; @@ -2216,9 +2521,21 @@ bool bta_gattc_cache_load(tBTA_GATTC_CLCB *p_clcb) return false; } p_clcb->searched_service_source = BTA_GATTC_SERVICE_INFO_FROM_NVS_FLASH; - bta_gattc_rebuild_cache(p_clcb->p_srcb, num_attr, attr); + status = bta_gattc_rebuild_cache(p_clcb->p_srcb, (UINT16)num_attr, attr); //free the attr buffer after used. osi_free(attr); + if (status != BTA_GATT_OK) { + /* The NV cache is either corrupted or we ran out of memory while + * rebuilding it. Either way the in-memory cache has already been + * cleared by bta_gattc_rebuild_cache(); drop the on-flash copy too + * so the next connection re-discovers from scratch instead of + * looping on a bad cache. */ + APPL_TRACE_ERROR("%s: rebuild fail st=%d, reset NV", + __func__, status); + bta_gattc_co_cache_reset(p_clcb->p_srcb->server_bda); + p_clcb->searched_service_source = BTA_GATTC_SERVICE_INFO_FROM_UNKNOWN; + return false; + } return true; } @@ -2256,10 +2573,7 @@ static void bta_gattc_cache_write(BD_ADDR server_bda, UINT16 num_attr, void bta_gattc_cache_reset(BD_ADDR server_bda) { BTIF_TRACE_DEBUG("%s", __func__); - char fname[255] = {0}; - bta_gattc_generate_cache_file_name(fname, server_bda); bta_gattc_co_cache_reset(server_bda); - //unlink(fname); } //#endif /* GATTC_CACHE_NVS */ 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 80a10a8f2ad..9ebac8a30b8 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_co.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_co.c @@ -15,9 +15,6 @@ * limitations under the License. * ******************************************************************************/ -#ifdef BT_SUPPORT_NVM -#include -#endif /* BT_SUPPORT_NVM */ #include #include #include "bta/bta_gattc_co.h" @@ -30,6 +27,8 @@ #include "osi/list.h" #include "esp_err.h" #include "osi/allocator.h" +#include "freertos/FreeRTOS.h" +#include "freertos/semphr.h" #if( defined BLE_INCLUDED ) && (BLE_INCLUDED == TRUE) #if( defined BTA_GATT_INCLUDED ) && (GATTC_INCLUDED == TRUE) @@ -40,42 +39,6 @@ #define MAX_DEVICE_IN_CACHE 50 #define MAX_ADDR_LIST_CACHE_BUF 2048 -#ifdef BT_SUPPORT_NVM -static FILE *sCacheFD = 0; -static void getFilename(char *buffer, BD_ADDR bda) -{ - sprintf(buffer, "%s%02x%02x%02x%02x%02x%02x", GATT_CACHE_PREFIX - , bda[0], bda[1], bda[2], bda[3], bda[4], bda[5]); -} - -static void cacheClose(void) -{ - if (sCacheFD != 0) { - fclose(sCacheFD); - sCacheFD = 0; - } -} - -static bool cacheOpen(BD_ADDR bda, bool to_save) -{ - char fname[255] = {0}; - getFilename(fname, bda); - - cacheClose(); - sCacheFD = fopen(fname, to_save ? "w" : "r"); - - return (sCacheFD != 0); -} - -static void cacheReset(BD_ADDR bda) -{ - char fname[255] = {0}; - getFilename(fname, bda); - unlink(fname); -} - -#else - static const char *cache_key = "gattc_cache_key"; static const char *cache_addr = "cache_addr_tab"; @@ -98,6 +61,60 @@ typedef struct { static cache_env_t *cache_env = NULL; +/* Protect |cache_env|; exported callouts re-enter each other (e.g. cacheOpen -> find_addr). + * + * The mutex is created exactly once from bta_gattc_co_cache_addr_init() on the + * BT host startup task before any other callout becomes reachable, so we do + * NOT need any spinlock around the creation. The *Static variant places the + * storage in .bss and avoids any allocation. */ +static StaticSemaphore_t s_cache_env_mutex_buf; +static SemaphoreHandle_t s_cache_env_mutex = NULL; + +/* Must be called from the BT host startup task (single-threaded context) + * before any other cache_env_* user becomes reachable. Idempotent. */ +static void cache_env_mutex_init_once(void) +{ + if (s_cache_env_mutex == NULL) { + s_cache_env_mutex = xSemaphoreCreateRecursiveMutexStatic(&s_cache_env_mutex_buf); + } +} + +static void cache_env_lock(void) +{ + if (s_cache_env_mutex != NULL) { + (void)xSemaphoreTakeRecursive(s_cache_env_mutex, portMAX_DELAY); + } +} + +static void cache_env_unlock(void) +{ + if (s_cache_env_mutex != NULL) { + xSemaphoreGiveRecursive(s_cache_env_mutex); + } +} + +/* Format a per-device GATT cache NVS namespace name into |buffer|. + * + * Output layout: GATT_CACHE_PREFIX (5 chars: "gatt_") + sizeof(hash_key_t)*2 + * hex chars (currently 8) + NUL = 14 bytes total. + * + * Contract: + * |buffer| MUST be at least NVS_KEY_NAME_MAX_SIZE (16) bytes, which is also + * the NVS limit for a namespace name. Today's 14-byte output leaves only + * 2 bytes of headroom -- if GATT_CACHE_PREFIX is ever lengthened or + * sizeof(hash_key_t) is ever increased so that + * + * strlen(GATT_CACHE_PREFIX) + 2*sizeof(hash_key_t) + 1 > NVS_KEY_NAME_MAX_SIZE + * + * this sprintf() will silently overflow the caller's stack buffer AND + * produce a namespace name that nvs_open() will reject. In that case + * switch to snprintf(buffer, NVS_KEY_NAME_MAX_SIZE, ...) and propagate + * the truncation as an error to callers. + * + * Callers (keep this list in sync if a new one is added): + * - cacheOpen() [open by current hash] + * - bta_gattc_co_cache_addr_save() [erase old namespace on hash change] + */ static void getFilename(char *buffer, hash_key_t hash) { sprintf(buffer, "%s%02x%02x%02x%02x", GATT_CACHE_PREFIX, @@ -107,6 +124,9 @@ static void getFilename(char *buffer, hash_key_t hash) static void cacheClose(BD_ADDR bda) { UINT8 index = 0; + if (cache_env == NULL) { + return; + } if ((index = bta_gattc_co_find_addr_in_cache(bda)) != INVALID_ADDR_NUM) { if (cache_env->cache_addr[index].is_open) { nvs_close(cache_env->cache_addr[index].cache_fp); @@ -117,8 +137,13 @@ static void cacheClose(BD_ADDR bda) static bool cacheOpen(BD_ADDR bda, bool to_save, UINT8 *index) { + if (cache_env == NULL) { + return false; + } UNUSED(to_save); - char fname[255] = {0}; + /* NVS namespace name is limited to (NVS_KEY_NAME_MAX_SIZE - 1) characters. + * getFilename() produces GATT_CACHE_PREFIX (5) + 8 hex chars + NUL = 14 bytes. */ + char fname[NVS_KEY_NAME_MAX_SIZE] = {0}; UINT8 *assoc_addr = NULL; esp_err_t status = ESP_FAIL; hash_key_t hash_key = {0}; @@ -141,8 +166,9 @@ static bool cacheOpen(BD_ADDR bda, bool to_save, UINT8 *index) static void cacheReset(BD_ADDR bda, BOOLEAN update) { - char fname[255] = {0}; - getFilename(fname, bda); + if (cache_env == NULL) { + return; + } UINT8 index = 0; //cache_env->cache_addr if ((index = bta_gattc_co_find_addr_in_cache(bda)) != INVALID_ADDR_NUM) { @@ -153,18 +179,21 @@ static void cacheReset(BD_ADDR bda, BOOLEAN update) nvs_close(cache_env->cache_addr[index].cache_fp); cache_env->cache_addr[index].is_open = FALSE; } else { - cacheOpen(bda, false, &index); - if (index == INVALID_ADDR_NUM) { - APPL_TRACE_ERROR("%s INVALID ADDR NUM", __func__); - return; - } + /* The entry exists but is not currently held open in this session; + * try to (re)open it best-effort so we can erase the on-flash blob. + * The recursive cache_env lock is held throughout, so cacheOpen()'s + * internal find_addr_in_cache(bda) cannot disagree with the outer + * find above, i.e. *index will remain valid here. We deliberately + * do NOT bail out on cacheOpen() failure: the entry must still be + * removed from RAM to keep num_addr/cache_addr[] consistent and to + * avoid OOB in callers. */ + (void)cacheOpen(bda, false, &index); if (cache_env->cache_addr[index].is_open) { nvs_erase_all(cache_env->cache_addr[index].cache_fp); nvs_close(cache_env->cache_addr[index].cache_fp); cache_env->cache_addr[index].is_open = FALSE; } else { - APPL_TRACE_ERROR("%s cacheOpen failed", __func__); - return; + APPL_TRACE_ERROR("%s: cacheOpen fail, evict RAM", __func__); } } if(cache_env->num_addr == 0) { @@ -180,6 +209,10 @@ static void cacheReset(BD_ADDR bda, BOOLEAN update) cache_env->cache_addr[index].assoc_addr = NULL; } + if (cache_env->num_addr > MAX_DEVICE_IN_CACHE) { + APPL_TRACE_WARNING("%s: num_addr %u exceeds max, clamping", __func__, cache_env->num_addr); + cache_env->num_addr = MAX_DEVICE_IN_CACHE; + } UINT8 num = cache_env->num_addr; //delete the server_bda in the addr_info list. for(UINT8 i = index; i < (num - 1); i++) { @@ -231,7 +264,6 @@ static void cacheReset(BD_ADDR bda, BOOLEAN update) } } -#endif /* BT_SUPPORT_NVM */ /***************************************************************************** ** Function Declarations *****************************************************************************/ @@ -253,13 +285,19 @@ static void cacheReset(BD_ADDR bda, BOOLEAN update) *******************************************************************************/ tBTA_GATT_STATUS bta_gattc_co_cache_open(BD_ADDR server_bda, BOOLEAN to_save, UINT8 *index) { + cache_env_lock(); /* open NV cache and send call in */ tBTA_GATT_STATUS status = BTA_GATT_OK; + if (cache_env == NULL) { + cache_env_unlock(); + return BTA_GATT_ERROR; + } if (!cacheOpen(server_bda, to_save, index)) { status = BTA_GATT_ERROR; } APPL_TRACE_DEBUG("%s() - status=%d", __func__, status); + cache_env_unlock(); return status; } @@ -280,11 +318,22 @@ tBTA_GATT_STATUS bta_gattc_co_cache_open(BD_ADDR server_bda, BOOLEAN to_save, UI *******************************************************************************/ tBTA_GATT_STATUS bta_gattc_co_cache_load(tBTA_GATTC_NV_ATTR *attr, UINT8 index) { + cache_env_lock(); #if (!CONFIG_BT_STACK_NO_LOG) UINT16 num_attr = 0; #endif tBTA_GATT_STATUS status = BTA_GATT_ERROR; size_t length = 0; + + if (cache_env == NULL) { + cache_env_unlock(); + return BTA_GATT_ERROR; + } + + if (index >= MAX_DEVICE_IN_CACHE) { + cache_env_unlock(); + return BTA_GATT_ERROR; + } // Read the size of memory space required for blob nvs_get_blob(cache_env->cache_addr[index].cache_fp, cache_key, NULL, &length); // Read previously saved blob if available @@ -296,18 +345,28 @@ tBTA_GATT_STATUS bta_gattc_co_cache_load(tBTA_GATTC_NV_ATTR *attr, UINT8 index) APPL_TRACE_DEBUG("%s() - read=%d, status=%d, err_code = %d", __func__, num_attr, status, err_code); + cache_env_unlock(); return status; } size_t bta_gattc_get_cache_attr_length(UINT8 index) { + cache_env_lock(); size_t length = 0; - if (index == INVALID_ADDR_NUM) { + + if (cache_env == NULL) { + cache_env_unlock(); + return 0; + } + + if (index == INVALID_ADDR_NUM || index >= MAX_DEVICE_IN_CACHE) { + cache_env_unlock(); return 0; } // Read the size of memory space required for blob nvs_get_blob(cache_env->cache_addr[index].cache_fp, cache_key, NULL, &length); + cache_env_unlock(); return length; } @@ -330,6 +389,13 @@ size_t bta_gattc_get_cache_attr_length(UINT8 index) void bta_gattc_co_cache_save (BD_ADDR server_bda, UINT16 num_attr, tBTA_GATTC_NV_ATTR *p_attr_list) { + cache_env_lock(); + + if (cache_env == NULL) { + cache_env_unlock(); + return; + } + tBTA_GATT_STATUS status = BTA_GATT_OK; hash_key_t hash_key = {0}; UINT8 index = INVALID_ADDR_NUM; @@ -350,6 +416,7 @@ void bta_gattc_co_cache_save (BD_ADDR server_bda, UINT16 num_attr, (void) status; #endif APPL_TRACE_DEBUG("%s() wrote hash_key = %x%x%x%x, num_attr = %d, status = %d.", __func__, hash_key[0], hash_key[1], hash_key[2], hash_key[3], num_attr, status); + cache_env_unlock(); } /******************************************************************************* @@ -367,14 +434,14 @@ void bta_gattc_co_cache_save (BD_ADDR server_bda, UINT16 num_attr, *******************************************************************************/ void bta_gattc_co_cache_close(BD_ADDR server_bda, UINT16 conn_id) { + cache_env_lock(); UNUSED(conn_id); -//#ifdef BT_SUPPORT_NVM cacheClose(server_bda); -//#endif /* BT_SUPPORT_NVM */ /* close NV when server cache is done saving or loading, does not need to do anything for now on Insight */ BTIF_TRACE_DEBUG("%s()", __FUNCTION__); + cache_env_unlock(); } /******************************************************************************* @@ -391,18 +458,22 @@ void bta_gattc_co_cache_close(BD_ADDR server_bda, UINT16 conn_id) *******************************************************************************/ void bta_gattc_co_cache_reset(BD_ADDR server_bda) { + cache_env_lock(); cacheReset(server_bda, TRUE); + cache_env_unlock(); } void bta_gattc_co_cache_addr_init(void) { + cache_env_mutex_init_once(); + cache_env_lock(); nvs_handle_t fp; esp_err_t err_code; - UINT8 num_addr; size_t length = MAX_ADDR_LIST_CACHE_BUF; UINT8 *p_buf = osi_malloc(MAX_ADDR_LIST_CACHE_BUF); if (p_buf == NULL) { APPL_TRACE_ERROR("%s malloc failed!", __func__); + cache_env_unlock(); return; } @@ -410,6 +481,7 @@ void bta_gattc_co_cache_addr_init(void) if (cache_env == NULL) { APPL_TRACE_ERROR("%s malloc failed!", __func__); osi_free(p_buf); + cache_env_unlock(); return; } @@ -420,19 +492,53 @@ void bta_gattc_co_cache_addr_init(void) cache_env->is_open = TRUE; // Read previously saved blob if available if ((err_code = nvs_get_blob(fp, cache_key, p_buf, &length)) != ESP_OK) { - if(err_code != ESP_ERR_NVS_NOT_FOUND) { + if (err_code == ESP_ERR_NVS_NOT_FOUND) { + length = 0; + } else { APPL_TRACE_ERROR("%s, Line = %d, nvs flash get blob data fail, err_code = 0x%x", __func__, __LINE__, err_code); + osi_free(p_buf); + if (cache_env->is_open) { + nvs_close(cache_env->addr_fp); + cache_env->is_open = FALSE; + } + osi_free(cache_env); + cache_env = NULL; + cache_env_unlock(); + return; } + } + const size_t rec_sz = sizeof(BD_ADDR) + sizeof(hash_key_t); + if (length == 0) { + cache_env->num_addr = 0; osi_free(p_buf); + cache_env_unlock(); return; } - num_addr = length / (sizeof(BD_ADDR) + sizeof(hash_key_t)); - cache_env->num_addr = num_addr; + if ((length % rec_sz) != 0) { + APPL_TRACE_ERROR("%s: bad blob len %zu", __func__, length); + (void)nvs_erase_key(fp, cache_key); + cache_env->num_addr = 0; + osi_free(p_buf); + if (cache_env->is_open) { + nvs_close(cache_env->addr_fp); + cache_env->is_open = FALSE; + } + osi_free(cache_env); + cache_env = NULL; + cache_env_unlock(); + return; + } + size_t n_entries = length / rec_sz; + const BOOLEAN truncated = (n_entries > (size_t)MAX_DEVICE_IN_CACHE) ? TRUE : FALSE; + if (truncated) { + APPL_TRACE_WARNING("%s: trunc %zu->%d", __func__, n_entries, MAX_DEVICE_IN_CACHE); + n_entries = MAX_DEVICE_IN_CACHE; + } + cache_env->num_addr = (UINT8)n_entries; //read the address from nvs flash to cache address list. - for (UINT8 i = 0; i < num_addr; i++) { - memcpy(cache_env->cache_addr[i].addr, p_buf + i*(sizeof(BD_ADDR) + sizeof(hash_key_t)), sizeof(BD_ADDR)); - memcpy(cache_env->cache_addr[i].hash_key, - p_buf + i*(sizeof(BD_ADDR) + sizeof(hash_key_t)) + sizeof(BD_ADDR), sizeof(hash_key_t)); + for (UINT8 i = 0; i < cache_env->num_addr; i++) { + memcpy(cache_env->cache_addr[i].addr, p_buf + i * rec_sz, sizeof(BD_ADDR)); + memcpy(cache_env->cache_addr[i].hash_key, p_buf + i * rec_sz + sizeof(BD_ADDR), sizeof(hash_key_t)); APPL_TRACE_DEBUG("cache_addr[%x] = %x:%x:%x:%x:%x:%x", i, cache_env->cache_addr[i].addr[0], cache_env->cache_addr[i].addr[1], cache_env->cache_addr[i].addr[2], cache_env->cache_addr[i].addr[3], cache_env->cache_addr[i].addr[4], cache_env->cache_addr[i].addr[5]); @@ -440,21 +546,30 @@ void bta_gattc_co_cache_addr_init(void) cache_env->cache_addr[i].hash_key[2], cache_env->cache_addr[i].hash_key[3]); bta_gattc_co_cache_new_assoc_list(cache_env->cache_addr[i].addr, i); } + if (truncated) { + UINT16 out_len = (UINT16)(cache_env->num_addr * rec_sz); + if (nvs_set_blob(fp, cache_key, p_buf, out_len) != ESP_OK) { + APPL_TRACE_WARNING("%s: nvs trunc fail", __func__); + } + } } 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; + cache_env_unlock(); return; } osi_free(p_buf); - return; + cache_env_unlock(); } void bta_gattc_co_cache_addr_deinit(void) { + cache_env_lock(); if(cache_env == NULL) { + cache_env_unlock(); return; } @@ -463,112 +578,222 @@ void bta_gattc_co_cache_addr_deinit(void) if (!cache_env->is_open) { osi_free(cache_env); cache_env = NULL; + cache_env_unlock(); return; } nvs_close(cache_env->addr_fp); cache_env->is_open = false; - for(UINT8 i = 0; i< cache_env->num_addr; i++) { + UINT8 num = cache_env->num_addr; + if (num > MAX_DEVICE_IN_CACHE) { + num = MAX_DEVICE_IN_CACHE; + } + for (UINT8 i = 0; i < num; i++) { cache_addr_info_t *addr_info = &cache_env->cache_addr[i]; - if(addr_info) { + if (addr_info->is_open) { nvs_close(addr_info->cache_fp); addr_info->is_open = false; - if(addr_info->assoc_addr) { - list_free(addr_info->assoc_addr); - } + } + if (addr_info->assoc_addr != NULL) { + list_free(addr_info->assoc_addr); + addr_info->assoc_addr = NULL; } } osi_free(cache_env); cache_env = NULL; + cache_env_unlock(); } BOOLEAN bta_gattc_co_addr_in_cache(BD_ADDR bda) { + cache_env_lock(); + if (cache_env == NULL) { + cache_env_unlock(); + return FALSE; + } UINT8 addr_index = 0; UINT8 num = cache_env->num_addr; + if (num > MAX_DEVICE_IN_CACHE) { + num = MAX_DEVICE_IN_CACHE; + } cache_addr_info_t *addr_info = &cache_env->cache_addr[0]; - for (addr_index = 0; addr_index < num; addr_index++) { + for (addr_index = 0; addr_index < num; addr_index++, addr_info++) { if (!memcmp(addr_info->addr, bda, sizeof(BD_ADDR))) { + cache_env_unlock(); return TRUE; } } + cache_env_unlock(); return FALSE; } UINT8 bta_gattc_co_find_addr_in_cache(BD_ADDR bda) { + cache_env_lock(); + if (cache_env == NULL) { + cache_env_unlock(); + return INVALID_ADDR_NUM; + } UINT8 addr_index = 0; UINT8 num = cache_env->num_addr; + if (num > MAX_DEVICE_IN_CACHE) { + num = MAX_DEVICE_IN_CACHE; + } cache_addr_info_t *addr_info = &cache_env->cache_addr[0]; for (addr_index = 0; addr_index < num; addr_index++, addr_info++) { if (!memcmp(addr_info->addr, bda, sizeof(BD_ADDR))) { + cache_env_unlock(); return addr_index; } } + cache_env_unlock(); return INVALID_ADDR_NUM; } UINT8 bta_gattc_co_find_hash_in_cache(hash_key_t hash_key) { + cache_env_lock(); + if (cache_env == NULL) { + cache_env_unlock(); + return INVALID_ADDR_NUM; + } UINT8 index = 0; UINT8 num = cache_env->num_addr; + if (num > MAX_DEVICE_IN_CACHE) { + num = MAX_DEVICE_IN_CACHE; + } cache_addr_info_t *addr_info = &cache_env->cache_addr[0]; - for (index = 0; index < num; index++) { + for (index = 0; index < num; index++, addr_info++) { if (!memcmp(addr_info->hash_key, hash_key, sizeof(hash_key_t))) { + cache_env_unlock(); return index; } } + cache_env_unlock(); return INVALID_ADDR_NUM; } UINT8 bta_gattc_co_get_addr_num(void) { + cache_env_lock(); if (cache_env == NULL) { + cache_env_unlock(); return 0; } - return cache_env->num_addr; + if (cache_env->num_addr > MAX_DEVICE_IN_CACHE) { + cache_env_unlock(); + return MAX_DEVICE_IN_CACHE; + } + UINT8 n = cache_env->num_addr; + cache_env_unlock(); + return n; } void bta_gattc_co_get_addr_list(BD_ADDR *addr_list) { + cache_env_lock(); + if (cache_env == NULL || addr_list == NULL) { + cache_env_unlock(); + return; + } UINT8 num = cache_env->num_addr; + if (num > MAX_DEVICE_IN_CACHE) { + num = MAX_DEVICE_IN_CACHE; + } for (UINT8 i = 0; i < num; i++) { memcpy(addr_list[i], cache_env->cache_addr[i].addr, sizeof(BD_ADDR)); } + cache_env_unlock(); } void bta_gattc_co_cache_addr_save(BD_ADDR bd_addr, hash_key_t hash_key) { + cache_env_lock(); + if (cache_env == NULL) { + cache_env_unlock(); + return; + } esp_err_t err_code; UINT8 index = 0; UINT8 new_index = cache_env->num_addr; UINT8 *p_buf = osi_malloc(MAX_ADDR_LIST_CACHE_BUF); if (p_buf == NULL) { APPL_TRACE_ERROR("%s malloc failed!", __func__); + cache_env_unlock(); return; } + if (cache_env->num_addr > MAX_DEVICE_IN_CACHE) { + APPL_TRACE_WARNING("%s: num_addr %u exceeds max, clamping", __func__, cache_env->num_addr); + cache_env->num_addr = MAX_DEVICE_IN_CACHE; + } + // check the address list has the same address or not // for the same address, it's hash key may be change due to service change if ((index = bta_gattc_co_find_addr_in_cache(bd_addr)) != INVALID_ADDR_NUM) { APPL_TRACE_DEBUG("%s the bd_addr already in the cache list, index = %x", __func__, index); + /* If the hash key changed (service change on the same peer), the + * per-device NVS namespace is keyed by the OLD hash. Just overwriting + * the in-RAM hash would leave cache_fp/is_open pointing at the old + * namespace, so a subsequent cacheOpen() would short-circuit on + * is_open==TRUE and the new attribute blob would be written into the + * stale namespace. After reboot the addr table maps bd_addr to the + * NEW hash and the load path would miss (and the old namespace is + * orphaned in NVS). Erase the old namespace and clear is_open so + * the next cacheOpen() reopens with the new hash-derived filename. */ + if (memcmp(cache_env->cache_addr[index].hash_key, hash_key, sizeof(hash_key_t)) != 0) { + APPL_TRACE_WARNING("%s: hash chg "MACSTR" %02x%02x%02x%02x->%02x%02x%02x%02x, erase NVS", + __func__, MAC2STR(bd_addr), + cache_env->cache_addr[index].hash_key[0], cache_env->cache_addr[index].hash_key[1], + cache_env->cache_addr[index].hash_key[2], cache_env->cache_addr[index].hash_key[3], + hash_key[0], hash_key[1], hash_key[2], hash_key[3]); + if (cache_env->cache_addr[index].is_open) { + (void)nvs_erase_all(cache_env->cache_addr[index].cache_fp); + nvs_close(cache_env->cache_addr[index].cache_fp); + cache_env->cache_addr[index].is_open = FALSE; + } else { + /* Not currently held open in this session; transiently open + * the old namespace just to erase its blob. Best-effort: + * ignore failures so a missing/corrupt old namespace does + * not block writing the new one. */ + char fname_old[NVS_KEY_NAME_MAX_SIZE] = {0}; + nvs_handle_t old_fp; + getFilename(fname_old, cache_env->cache_addr[index].hash_key); + if (nvs_open(fname_old, NVS_READWRITE, &old_fp) == ESP_OK) { + (void)nvs_erase_all(old_fp); + nvs_close(old_fp); + } + } + } //if the bd_addr already in the address list, update the hash key in it. memcpy(cache_env->cache_addr[index].addr, bd_addr, sizeof(BD_ADDR)); memcpy(cache_env->cache_addr[index].hash_key, hash_key, sizeof(hash_key_t)); } else { - if (cache_env->num_addr >= MAX_DEVICE_IN_CACHE) { + while (cache_env->num_addr >= MAX_DEVICE_IN_CACHE) { APPL_TRACE_WARNING("%s cache list full and remove the oldest addr info", __func__); + UINT8 before = cache_env->num_addr; cacheReset(cache_env->cache_addr[0].addr, FALSE); + if (cache_env->num_addr >= before) { + APPL_TRACE_ERROR("%s: cache eviction failed", __func__); + osi_free(p_buf); + cache_env_unlock(); + return; + } } new_index = cache_env->num_addr; - assert(new_index < MAX_DEVICE_IN_CACHE); + if (new_index >= MAX_DEVICE_IN_CACHE) { + APPL_TRACE_ERROR("%s: invalid new_index %u", __func__, new_index); + osi_free(p_buf); + cache_env_unlock(); + return; + } memcpy(cache_env->cache_addr[new_index].addr, bd_addr, sizeof(BD_ADDR)); memcpy(cache_env->cache_addr[new_index].hash_key, hash_key, sizeof(hash_key_t)); cache_env->num_addr++; @@ -603,22 +828,45 @@ void bta_gattc_co_cache_addr_save(BD_ADDR bd_addr, hash_key_t hash_key) //free the buffer after used. osi_free(p_buf); - return; + cache_env_unlock(); } BOOLEAN bta_gattc_co_cache_new_assoc_list(BD_ADDR src_addr, UINT8 index) { + cache_env_lock(); + if (cache_env == NULL) { + cache_env_unlock(); + return FALSE; + } + UNUSED(src_addr); + if (index >= MAX_DEVICE_IN_CACHE) { + cache_env_unlock(); + return FALSE; + } cache_addr_info_t *addr_info = &cache_env->cache_addr[index]; + /* Idempotent: if a list already exists at this slot (e.g. caller invoked + * us twice on the same index), free it first so we don't leak the prior + * list_t. The current sole caller is bta_gattc_co_cache_addr_init() which + * runs after a fresh memset, so this path is normally a no-op; the guard + * just keeps the function safe to reuse. */ + if (addr_info->assoc_addr != NULL) { + list_free(addr_info->assoc_addr); + addr_info->assoc_addr = NULL; + } addr_info->assoc_addr = list_new(osi_free_func); - return (addr_info->assoc_addr != NULL ? TRUE : FALSE); + BOOLEAN ok = (addr_info->assoc_addr != NULL ? TRUE : FALSE); + cache_env_unlock(); + return ok; } BOOLEAN bta_gattc_co_cache_append_assoc_addr(BD_ADDR src_addr, BD_ADDR assoc_addr) { + cache_env_lock(); UINT8 addr_index = 0; cache_addr_info_t *addr_info; UINT8 *p_assoc_buf = osi_malloc(sizeof(BD_ADDR)); if(!p_assoc_buf) { + cache_env_unlock(); return FALSE; } memcpy(p_assoc_buf, assoc_addr, sizeof(BD_ADDR)); @@ -630,6 +878,7 @@ BOOLEAN bta_gattc_co_cache_append_assoc_addr(BD_ADDR src_addr, BD_ADDR assoc_add if (addr_info->assoc_addr == NULL) { APPL_TRACE_ERROR("assoc_addr list creation failed"); osi_free(p_assoc_buf); + cache_env_unlock(); return FALSE; } @@ -638,6 +887,7 @@ BOOLEAN bta_gattc_co_cache_append_assoc_addr(BD_ADDR src_addr, BD_ADDR assoc_add if (!memcmp(list_node(sn), assoc_addr, sizeof(BD_ADDR))) { APPL_TRACE_WARNING("Association already exists"); osi_free(p_assoc_buf); + cache_env_unlock(); return TRUE; } } @@ -645,19 +895,23 @@ BOOLEAN bta_gattc_co_cache_append_assoc_addr(BD_ADDR src_addr, BD_ADDR assoc_add 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); + cache_env_unlock(); return FALSE; } + cache_env_unlock(); return TRUE; } else { osi_free(p_assoc_buf); } + cache_env_unlock(); return FALSE; } BOOLEAN bta_gattc_co_cache_remove_assoc_addr(BD_ADDR src_addr, BD_ADDR assoc_addr) { + cache_env_lock(); UINT8 addr_index = 0; cache_addr_info_t *addr_info; if ((addr_index = bta_gattc_co_find_addr_in_cache(src_addr)) != INVALID_ADDR_NUM) { @@ -667,20 +921,34 @@ BOOLEAN bta_gattc_co_cache_remove_assoc_addr(BD_ADDR src_addr, BD_ADDR assoc_add sn != list_end(addr_info->assoc_addr); sn = list_next(sn)) { void *addr = list_node(sn); if (!memcmp(addr, assoc_addr, sizeof(BD_ADDR))) { - return list_remove(addr_info->assoc_addr, addr); + BOOLEAN removed = list_remove(addr_info->assoc_addr, addr); + cache_env_unlock(); + return removed; } } //return list_remove(addr_info->assoc_addr, assoc_addr); } else { + cache_env_unlock(); return FALSE; } } + cache_env_unlock(); return FALSE; } UINT8* bta_gattc_co_cache_find_src_addr(BD_ADDR assoc_addr, UINT8 *index) { + cache_env_lock(); + if (index == NULL) { + cache_env_unlock(); + return NULL; + } + if (cache_env == NULL) { + *index = INVALID_ADDR_NUM; + cache_env_unlock(); + return NULL; + } 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; @@ -695,30 +963,37 @@ UINT8* bta_gattc_co_cache_find_src_addr(BD_ADDR assoc_addr, UINT8 *index) addr_data = (UINT8 *)list_node(node); if (!memcmp(addr_data, assoc_addr, sizeof(BD_ADDR))) { *index = i; - return (UINT8 *)addr_info->addr; + UINT8 *ret = (UINT8 *)addr_info->addr; + cache_env_unlock(); + return ret; } } addr_info++; } *index = INVALID_ADDR_NUM; + cache_env_unlock(); return NULL; } BOOLEAN bta_gattc_co_cache_clear_assoc_addr(BD_ADDR src_addr) { + cache_env_lock(); UINT8 addr_index = 0; cache_addr_info_t *addr_info; 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) { list_clear(addr_info->assoc_addr); + cache_env_unlock(); + return TRUE; } else { + cache_env_unlock(); return FALSE; } - return TRUE; } + cache_env_unlock(); return FALSE; } 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 bbf2b346bff..a29f9bd462e 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_main.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_main.c @@ -491,6 +491,12 @@ static char *gattc_evt_code(tBTA_GATTC_INT_EVT evt_code) return "BTA_GATTC_API_READ_BY_TYPE_EVT"; case BTA_GATTC_API_READ_MULTI_VAR_EVT: return "BTA_GATTC_API_READ_MULTI_VAR_EVT"; + case BTA_GATTC_ENC_CMPL_EVT: + return "BTA_GATTC_ENC_CMPL_EVT"; + case BTA_GATTC_API_CACHE_ASSOC_EVT: + return "BTA_GATTC_API_CACHE_ASSOC_EVT"; + case BTA_GATTC_API_CACHE_GET_ADDR_LIST_EVT: + return "BTA_GATTC_API_CACHE_GET_ADDR_LIST_EVT"; default: return "unknown GATTC event code"; } @@ -541,7 +547,8 @@ uint8_t bta_gattc_cl_rcb_active_count(void) for (uint8_t i = 0; i < BTA_GATTC_CL_MAX; i ++) { if (bta_gattc_cb.cl_rcb[i].in_use && - memcmp(bta_gattc_cb.cl_rcb[i].app_uuid.uu.uuid128, dm_gattc_uuid, 16)) { + (bta_gattc_cb.cl_rcb[i].app_uuid.len != LEN_UUID_128 || + memcmp(bta_gattc_cb.cl_rcb[i].app_uuid.uu.uuid128, dm_gattc_uuid, LEN_UUID_128))) { count++; } } 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 a0342514a3f..332bf33e6da 100644 --- a/components/bt/host/bluedroid/bta/gatt/bta_gattc_utils.c +++ b/components/bt/host/bluedroid/bta/gatt/bta_gattc_utils.c @@ -154,9 +154,12 @@ tBTA_GATTC_CLCB *bta_gattc_find_clcb_by_conn_id (UINT16 conn_id) tBTA_GATTC_CLCB *p_clcb = &bta_gattc_cb.clcb[0]; UINT8 i; + if (conn_id == 0 || conn_id == GATT_INVALID_CONN_ID) { + return NULL; + } + for (i = 0; i < BTA_GATTC_CLCB_MAX; i ++, p_clcb ++) { - if (p_clcb->in_use && - p_clcb->bta_conn_id == conn_id) { + if (p_clcb->in_use && p_clcb->bta_conn_id == conn_id) { return p_clcb; } } @@ -342,7 +345,7 @@ tBTA_GATTC_SERV *bta_gattc_find_srvr_cache(BD_ADDR bda) UINT8 i; for (i = 0; i < BTA_GATTC_KNOWN_SR_MAX; i ++, p_srcb ++) { - if (bdcmp(p_srcb->server_bda, bda) == 0) { + if (p_srcb->in_use && bdcmp(p_srcb->server_bda, bda) == 0) { return p_srcb; } } @@ -387,7 +390,7 @@ tBTA_GATTC_SERV *bta_gattc_srcb_alloc(BD_ADDR bda) if (!p_tcb->in_use) { found = TRUE; break; - } else if (!p_tcb->connected) { + } else if (!p_tcb->connected && p_tcb->num_clcb == 0) { p_recycle = p_tcb; } } @@ -417,6 +420,38 @@ tBTA_GATTC_SERV *bta_gattc_srcb_alloc(BD_ADDR bda) return p_tcb; } +/****************************************************************************** + * + * Fixed-length prefix size of a GATTC API message in the heap buffer passed + * to bta_gattc_enqueue. Must match osi_malloc sizes in bta_gattc_api.c. + * Variable payloads (write value, search UUID) are copied separately. + * + ******************************************************************************/ +static size_t bta_gattc_enqueue_api_fixed_size(UINT16 event) +{ + switch (event) { + case BTA_GATTC_API_READ_EVT: + case BTA_GATTC_API_READ_BY_TYPE_EVT: + return sizeof(tBTA_GATTC_API_READ); + case BTA_GATTC_API_WRITE_EVT: + return sizeof(tBTA_GATTC_API_WRITE); + case BTA_GATTC_API_EXEC_EVT: + return sizeof(tBTA_GATTC_API_EXEC); + case BTA_GATTC_API_CFG_MTU_EVT: + return sizeof(tBTA_GATTC_API_CFG_MTU); + case BTA_GATTC_API_SEARCH_EVT: + return sizeof(tBTA_GATTC_API_SEARCH); + case BTA_GATTC_API_CONFIRM_EVT: + return sizeof(tBTA_GATTC_API_CONFIRM); + case BTA_GATTC_API_READ_MULTI_EVT: + case BTA_GATTC_API_READ_MULTI_VAR_EVT: + return sizeof(tBTA_GATTC_API_READ_MULTI); + default: + APPL_TRACE_ERROR("%s: unexpected event 0x%x", __func__, event); + return 0; + } +} + static BOOLEAN bta_gattc_has_prepare_command_in_queue(tBTA_GATTC_CLCB *p_clcb) { assert(p_clcb != NULL); @@ -485,14 +520,29 @@ BOOLEAN bta_gattc_enqueue(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) if (p_data->hdr.event == BTA_GATTC_API_WRITE_EVT) { len = p_data->api_write.len; - if ((cmd_data = (tBTA_GATTC_DATA *)osi_malloc(sizeof(tBTA_GATTC_DATA) + len)) != NULL) { - memset(cmd_data, 0, sizeof(tBTA_GATTC_DATA) + len); - memcpy(cmd_data, p_data, sizeof(tBTA_GATTC_DATA)); - cmd_data->api_write.p_value = (UINT8 *)(cmd_data + 1); - memcpy(cmd_data->api_write.p_value, p_data->api_write.p_value, len); + if (len > 0) { + if (p_data->api_write.p_value == NULL) { + APPL_TRACE_ERROR("%s(), write len=%u but p_value is NULL", __func__, len); + return FALSE; + } + if ((cmd_data = (tBTA_GATTC_DATA *)osi_malloc(sizeof(tBTA_GATTC_DATA) + len)) != NULL) { + memset(cmd_data, 0, sizeof(tBTA_GATTC_DATA) + len); + memcpy(cmd_data, p_data, sizeof(tBTA_GATTC_API_WRITE)); + cmd_data->api_write.p_value = (UINT8 *)(cmd_data + 1); + memcpy(cmd_data->api_write.p_value, p_data->api_write.p_value, len); + } else { + APPL_TRACE_ERROR("%s(), line = %d, alloc fail, no memory.", __func__, __LINE__); + return FALSE; + } } else { - APPL_TRACE_ERROR("%s(), line = %d, alloc fail, no memory.", __func__, __LINE__); - return FALSE; + /* len == 0: no payload to copy; keep p_value NULL like BTA_GATTC_API_SEARCH_EVT without UUID */ + if ((cmd_data = (tBTA_GATTC_DATA *)osi_malloc(sizeof(tBTA_GATTC_DATA))) != NULL) { + memset(cmd_data, 0, sizeof(tBTA_GATTC_DATA)); + memcpy(cmd_data, p_data, sizeof(tBTA_GATTC_API_WRITE)); + } else { + APPL_TRACE_ERROR("%s(), line = %d, alloc fail, no memory.", __func__, __LINE__); + return FALSE; + } } } else if (p_data->hdr.event == BTA_GATTC_API_SEARCH_EVT) { /* @@ -547,7 +597,7 @@ BOOLEAN bta_gattc_enqueue(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) if ((cmd_data = (tBTA_GATTC_DATA *)osi_malloc(len)) != NULL) { memset(cmd_data, 0, len); /* Copy the structure */ - memcpy(cmd_data, p_data, sizeof(tBTA_GATTC_DATA)); + memcpy(cmd_data, p_data, sizeof(tBTA_GATTC_API_SEARCH)); /* Update pointer to point to the space after the structure */ cmd_data->api_search.p_srvc_uuid = (tBT_UUID *)(cmd_data + 1); /* Copy the UUID data */ @@ -561,16 +611,22 @@ BOOLEAN bta_gattc_enqueue(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data) /* p_srvc_uuid is NULL, no extra space needed (search all services) */ if ((cmd_data = (tBTA_GATTC_DATA *)osi_malloc(sizeof(tBTA_GATTC_DATA))) != NULL) { memset(cmd_data, 0, sizeof(tBTA_GATTC_DATA)); - memcpy(cmd_data, p_data, sizeof(tBTA_GATTC_DATA)); + memcpy(cmd_data, p_data, sizeof(tBTA_GATTC_API_SEARCH)); } else { APPL_TRACE_ERROR("%s(), line = %d, alloc fail, no memory.", __func__, __LINE__); return FALSE; } } } else { + size_t copy_sz = bta_gattc_enqueue_api_fixed_size(p_data->hdr.event); + if (copy_sz == 0) { + APPL_TRACE_ERROR("%s(), line = %d, unknown event for queue copy 0x%x.", __func__, __LINE__, + p_data->hdr.event); + return FALSE; + } if ((cmd_data = (tBTA_GATTC_DATA *)osi_malloc(sizeof(tBTA_GATTC_DATA))) != NULL) { memset(cmd_data, 0, sizeof(tBTA_GATTC_DATA)); - memcpy(cmd_data, p_data, sizeof(tBTA_GATTC_DATA)); + memcpy(cmd_data, p_data, copy_sz); } else { APPL_TRACE_ERROR("%s(), line = %d, alloc fail, no memory.", __func__, __LINE__); return FALSE; @@ -911,7 +967,10 @@ tBTA_GATTC_CONN *bta_gattc_conn_alloc(BD_ADDR remote_bda) #if BTA_GATT_DEBUG == TRUE APPL_TRACE_DEBUG("bta_gattc_conn_alloc: found conn_track[%d] available", i_conn); #endif - p_conn->in_use = TRUE; + p_conn->in_use = TRUE; + p_conn->svc_change_descr_handle = 0; + p_conn->write_remote_svc_change_ccc_in_progress = FALSE; + p_conn->write_remote_svc_change_ccc_done = FALSE; bdcpy(p_conn->remote_bda, remote_bda); return p_conn; } @@ -979,6 +1038,9 @@ BOOLEAN bta_gattc_conn_dealloc(BD_ADDR remote_bda) if (p_conn != NULL) { p_conn->in_use = FALSE; + p_conn->svc_change_descr_handle = 0; + p_conn->write_remote_svc_change_ccc_in_progress = FALSE; + p_conn->write_remote_svc_change_ccc_done = FALSE; memset(p_conn->remote_bda, 0, BD_ADDR_LEN); return TRUE; } 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 277d2b267cf..770d38456fa 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 @@ -211,6 +211,7 @@ typedef struct { typedef struct { BT_HDR hdr; BD_ADDR remote_bda; + BOOLEAN erase_flash; } tBTA_GATTC_API_CACHE_REFRESH; typedef struct { @@ -399,6 +400,8 @@ typedef struct { BOOLEAN in_use; BD_ADDR remote_bda; UINT16 svc_change_descr_handle; + /* Tracks the in-flight internal write to the Service Changed CCC descriptor */ + BOOLEAN write_remote_svc_change_ccc_in_progress; BOOLEAN write_remote_svc_change_ccc_done; } tBTA_GATTC_CONN; @@ -559,7 +562,7 @@ extern void bta_gattc_get_db_with_operation(UINT16 conn_id, extern void bta_gattc_get_gatt_db(UINT16 conn_id, UINT16 start_handle, UINT16 end_handle, btgatt_db_element_t **db, UINT16 *count); extern tBTA_GATT_STATUS bta_gattc_init_cache(tBTA_GATTC_SERV *p_srvc_cb); -extern void bta_gattc_rebuild_cache(tBTA_GATTC_SERV *p_srcv, UINT16 num_attr, tBTA_GATTC_NV_ATTR *attr); +extern tBTA_GATT_STATUS bta_gattc_rebuild_cache(tBTA_GATTC_SERV *p_srcv, UINT16 num_attr, tBTA_GATTC_NV_ATTR *attr); extern void bta_gattc_cache_save(tBTA_GATTC_SERV *p_srvc_cb, UINT16 conn_id); extern void bta_gattc_reset_discover_st(tBTA_GATTC_SERV *p_srcb, tBTA_GATT_STATUS status); diff --git a/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gattc.c b/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gattc.c index 4882584baa4..21f1c8cbf52 100644 --- a/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gattc.c +++ b/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gattc.c @@ -203,7 +203,7 @@ static void btc_gattc_cback(tBTA_GATTC_EVT event, tBTA_GATTC *p_data) static void btc_gattc_app_register(btc_ble_gattc_args_t *arg) { - tBT_UUID app_uuid; + tBT_UUID app_uuid = {0}; app_uuid.len = 2; app_uuid.uu.uuid16 = arg->app_reg.app_id; BTA_GATTC_AppRegister(&app_uuid, btc_gattc_cback); @@ -991,8 +991,6 @@ void btc_gattc_cb_handler(btc_msg_t *msg) case BTA_GATTC_CLOSE_EVT: { tBTA_GATTC_CLOSE *close = &arg->close; - // Free gattc clcb in BTC task to avoid race condition - bta_gattc_clcb_dealloc_by_conn_id(close->conn_id); gattc_if = close->client_if; param.close.status = close->status; param.close.conn_id = BTC_GATT_GET_CONN_ID(close->conn_id); diff --git a/components/bt/host/bluedroid/stack/gatt/att_protocol.c b/components/bt/host/bluedroid/stack/gatt/att_protocol.c index 686820071a8..704e1e969f0 100644 --- a/components/bt/host/bluedroid/stack/gatt/att_protocol.c +++ b/components/bt/host/bluedroid/stack/gatt/att_protocol.c @@ -390,6 +390,18 @@ tGATT_STATUS attp_send_msg_to_l2cap(tGATT_TCB *p_tcb, BT_HDR *p_toL2CAP) if (p_tcb->att_lcid == L2CAP_ATT_CID) { + /* L2CA_SendFixedChnlData() silently drops (osi_free) the buffer when the + * ATT fixed channel is already in cong_sent state, yet still returns + * L2CAP_DW_CONGESTED. Without distinguishing this from the post-enqueue + * congestion case, the upper layer would treat a dropped PDU as "sent" + * and wait for a response that never arrives. Detect the drop path + * up-front, release the buffer here and surface it as GATT_BUSY so that + * callers go through their failure path instead. */ + if (L2CA_CheckIsCongest(L2CAP_ATT_CID, p_tcb->peer_bda)) { + GATT_TRACE_WARNING("ATT fixed channel already congested, drop PDU"); + osi_free(p_toL2CAP); + return GATT_BUSY; + } l2cap_ret = L2CA_SendFixedChnlData (L2CAP_ATT_CID, p_tcb->peer_bda, p_toL2CAP); } else { #if (CLASSIC_BT_INCLUDED == TRUE) @@ -403,6 +415,8 @@ tGATT_STATUS attp_send_msg_to_l2cap(tGATT_TCB *p_tcb, BT_HDR *p_toL2CAP) GATT_TRACE_DEBUG("ATT failed to pass msg to L2CAP"); return GATT_INTERNAL_ERROR; } else if (l2cap_ret == L2CAP_DW_CONGESTED) { + /* Buffer was enqueued by L2CAP before congestion was reported; + * L2CAP retains ownership of it. */ GATT_TRACE_DEBUG("ATT congested, message accepted"); return GATT_CONGESTED; } diff --git a/components/bt/host/bluedroid/stack/gatt/gatt_api.c b/components/bt/host/bluedroid/stack/gatt/gatt_api.c index ddab5b986f8..3ade9f7f388 100644 --- a/components/bt/host/bluedroid/stack/gatt/gatt_api.c +++ b/components/bt/host/bluedroid/stack/gatt/gatt_api.c @@ -1750,9 +1750,22 @@ tGATT_STATUS GATTS_HandleMultiValueNotification (UINT16 conn_id, tGATT_HLV *tupl return GATT_ILLEGAL_PARAMETER; } + { + UINT32 new_len = (UINT32)notif.len + 4U + (UINT32)p_hlv->length; + if (new_len > (UINT32)GATT_MAX_ATTR_LEN) { + GATT_TRACE_ERROR("%s: len %u>MAX_ATTR_LEN", __func__, (unsigned)new_len); + return GATT_ILLEGAL_PARAMETER; + } + } + UINT16_TO_STREAM(p, p_hlv->handle); //handle UINT16_TO_STREAM(p, p_hlv->length); //length - memcpy (p, p_hlv->value, p_hlv->length); //value + if (p_hlv->length > 0) { + if (p_hlv->value == NULL) { + return GATT_ILLEGAL_PARAMETER; + } + memcpy (p, p_hlv->value, p_hlv->length); //value + } GATT_TRACE_DEBUG("%s handle %x, length %u", __func__, p_hlv->handle, p_hlv->length); p += p_hlv->length; notif.len += 4 + p_hlv->length; diff --git a/components/bt/host/bluedroid/stack/gatt/gatt_cl.c b/components/bt/host/bluedroid/stack/gatt/gatt_cl.c index 5d7c7e2bb39..28a50c4ac35 100644 --- a/components/bt/host/bluedroid/stack/gatt/gatt_cl.c +++ b/components/bt/host/bluedroid/stack/gatt/gatt_cl.c @@ -1124,10 +1124,14 @@ BOOLEAN gatt_cl_send_next_cmd_inq(tGATT_TCB *p_tcb) if (att_ret == GATT_SUCCESS || att_ret == GATT_CONGESTED) { sent = TRUE; p_cmd->to_send = FALSE; - if(p_cmd->p_cmd) { - osi_free(p_cmd->p_cmd); - p_cmd->p_cmd = NULL; - } + /* On GATT_SUCCESS / GATT_CONGESTED, L2CAP has taken ownership of + * p_cmd->p_cmd (it was either accepted normally, or enqueued just + * before the channel turned congested). The "already congested" + * drop path inside L2CA_SendFixedChnlData() is filtered out earlier + * by attp_send_msg_to_l2cap() and returned as GATT_BUSY, which + * falls into the error branch below. So we must not free the + * buffer here. */ + p_cmd->p_cmd = NULL; /* dequeue the request if is write command or sign write */ if (p_cmd->op_code != GATT_CMD_WRITE && p_cmd->op_code != GATT_SIGN_CMD_WRITE) { @@ -1145,13 +1149,22 @@ BOOLEAN gatt_cl_send_next_cmd_inq(tGATT_TCB *p_tcb) gatt_end_operation(p_clcb, att_ret, NULL); } } else { - GATT_TRACE_ERROR("gatt_cl_send_next_cmd_inq: L2CAP sent error"); + GATT_TRACE_ERROR("gatt_cl_send_next_cmd_inq: L2CAP sent error, status=%d", att_ret); /* 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->to_send = FALSE; + /* Dequeue the failing command so pending_cl_req is advanced and the + * associated p_clcb can be retrieved. */ + p_clcb = gatt_cmd_dequeue(p_tcb, &rsp_code); p_cmd = &p_tcb->cl_cmd_q[p_tcb->pending_cl_req]; + /* Notify the upper layer about the failure. Without this the + * response timer is never armed (non-write ops) and the write + * completion callback is never fired, leaving the application + * stuck waiting for a callback that will never come. The p_clcb + * would also leak. */ + if (p_clcb != NULL) { + gatt_end_operation(p_clcb, att_ret, NULL); + } } }