fix(ble/bluedroid): fix memory safety and state issues in BTA GATT layer

- Fix use-after-free and double-free in bta_gattc_update_include_service
- Fix heap buffer overflow in GATT database operations
- Fix GATTC cache load attr length check and NVS handle leak
- Fix parameter validation in bta_gattc_uuid_compare
- Ensure all CLCBs are cleaned up on deregister
- Remove unused bta_gattc_open_error
- Unify GATT db count/fill by declaration handle range
- Fix return status in gatts_set_attribute_value



(cherry picked from commit d4f3517da4)

Co-authored-by: zhiweijian <zhiweijian@espressif.com>

components/bt/host/bluedroid/bta/gatt/bta_gatt_common.c

@@ -16,6 +16,10 @@
 
 void BTA_GATT_SetLocalMTU(uint16_t mtu)
 {
+    if (mtu < GATT_DEF_BLE_MTU_SIZE || mtu > GATT_MAX_MTU_SIZE) {
+        APPL_TRACE_ERROR("%s: invalid mtu=%u", __func__, mtu);
+        return;
+    }
     gatt_set_local_mtu(mtu);
 }
 

components/bt/host/bluedroid/bta/gatt/bta_gattc_act.c

@@ -38,10 +38,6 @@
 #include "bta_hh_int.h"
 #include "btm_int.h"
 
-#if (defined BTA_HH_LE_INCLUDED && BTA_HH_LE_INCLUDED == TRUE)
-#include "bta_hh_int.h"
-#endif
-
 // #include "btif/include/btif_debug_conn.h"
 
 #include <string.h>
@@ -290,12 +286,21 @@ void bta_gattc_deregister(tBTA_GATTC_CB *p_cb, tBTA_GATTC_RCB  *p_clreg)
         /* remove bg connection associated with this rcb */
         for (i = 0; i < BTA_GATTC_KNOWN_SR_MAX; i ++) {
             if (p_cb->bg_track[i].in_use) {
-                if (p_cb->bg_track[i].cif_mask & (1 << (p_clreg->client_if - 1))) {
+                if (p_cb->bg_track[i].cif_mask & BTA_GATTC_CIF_MASK_BIT(p_clreg->client_if)) {
-                    bta_gattc_mark_bg_conn(p_clreg->client_if, p_cb->bg_track[i].remote_bda, FALSE, FALSE);
+                    /* bta_gattc_mark_bg_conn is called to remove the application's bit from the mask.
-                    GATT_CancelConnect(p_clreg->client_if, p_cb->bg_track[i].remote_bda, FALSE);
+                    If this was the last application interested in the device,
+                    bta_gattc_mark_bg_conn zeroes out the entire track entry using memset
+                    */
+                    BD_ADDR remote_bda;
+                    bdcpy(remote_bda, p_cb->bg_track[i].remote_bda);
+                    bta_gattc_mark_bg_conn(p_clreg->client_if, remote_bda, FALSE, FALSE);
+                    GATT_CancelConnect(p_clreg->client_if, remote_bda, FALSE);
                 }
-                if (p_cb->bg_track[i].cif_adv_mask & (1 << (p_clreg->client_if - 1))) {
+                if (p_cb->bg_track[i].cif_adv_mask & BTA_GATTC_CIF_MASK_BIT(p_clreg->client_if)) {
-                    bta_gattc_mark_bg_conn(p_clreg->client_if, p_cb->bg_track[i].remote_bda, FALSE, TRUE);
+                    BD_ADDR remote_bda_adv;
+                    bdcpy(remote_bda_adv, p_cb->bg_track[i].remote_bda);
+                    bta_gattc_mark_bg_conn(p_clreg->client_if, remote_bda_adv, FALSE, TRUE);
+                    GATT_CancelConnect(p_clreg->client_if, remote_bda_adv, FALSE);
                 }
             }
         }
@@ -461,28 +466,6 @@ void bta_gattc_cancel_open_error(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_dat
     }
 }
 
-/*******************************************************************************
-**
-** Function         bta_gattc_open_error
-**
-** Description
-**
-** Returns          void
-**
-*******************************************************************************/
-void bta_gattc_open_error(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data)
-{
-    UNUSED(p_data);
-
-    APPL_TRACE_ERROR("Connection already opened. wrong state");
-
-    bta_gattc_send_open_cback(p_clcb->p_rcb,
-                              BTA_GATT_OK,
-                              p_clcb->bda,
-                              p_clcb->bta_conn_id,
-                              p_clcb->transport,
-                              0);
-}
 /*******************************************************************************
 **
 ** Function         bta_gattc_open_fail
@@ -734,6 +717,18 @@ void bta_gattc_cancel_open(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data)
 void bta_gattc_conn(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data)
 {
     tBTA_GATTC_IF   gatt_if;
+    if (p_clcb->p_srcb == NULL) {
+        APPL_TRACE_ERROR("%s, p_clcb->p_srcb is NULL", __func__);
+        if (p_clcb->p_rcb) {
+            bta_gattc_send_open_cback(p_clcb->p_rcb,
+                                        BTA_GATT_ERROR,
+                                        p_clcb->bda,
+                                        p_clcb->bta_conn_id,
+                                        p_clcb->transport,
+                                        GATT_DEF_BLE_MTU_SIZE);
+        }
+        return;
+    }
     APPL_TRACE_DEBUG("bta_gattc_conn server cache state=%d", p_clcb->p_srcb->state);
 
     if (p_data != NULL) {
@@ -882,6 +877,10 @@ void bta_gattc_close_fail(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data)
 *******************************************************************************/
 void bta_gattc_close(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data)
 {
+    if (!p_clcb || !p_clcb->p_rcb) {
+        APPL_TRACE_ERROR("%s, p_clcb or p_clcb->p_rcb is NULL", __func__);
+        return;
+    }
     tBTA_GATTC_CBACK    *p_cback = p_clcb->p_rcb->p_cback;
     tBTA_GATTC_RCB      *p_clreg = p_clcb->p_rcb;
     tBTA_GATTC           cb_data;
@@ -1117,6 +1116,11 @@ void bta_gattc_disc_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data)
 
     APPL_TRACE_DEBUG("bta_gattc_disc_cmpl conn_id=%d, status = %d", p_clcb->bta_conn_id, p_clcb->status);
 
+    if (p_clcb->p_srcb == NULL) {
+        APPL_TRACE_ERROR("%s, p_clcb->p_srcb is NULL", __func__);
+        return;
+    }
+
     p_clcb->p_srcb->state = BTA_GATTC_SERV_IDLE;
     p_clcb->disc_active = FALSE;
 
@@ -1235,16 +1239,13 @@ void bta_gattc_read_multi(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data)
     if (bta_gattc_enqueue(p_clcb, p_data)) {
         memset(&read_param, 0, sizeof(tGATT_READ_PARAM));
 
-        if (status == BTA_GATT_OK) {
+        read_param.read_multiple.num_handles = p_data->api_read_multi.num_attr;
-            read_param.read_multiple.num_handles = p_data->api_read_multi.num_attr;
+        read_param.read_multiple.auth_req = p_data->api_read_multi.auth_req;
-            read_param.read_multiple.auth_req = p_data->api_read_multi.auth_req;
+        memcpy(&read_param.read_multiple.handles, p_data->api_read_multi.handles,
-            memcpy(&read_param.read_multiple.handles, p_data->api_read_multi.handles,
+                sizeof(UINT16) * p_data->api_read_multi.num_attr);
-                    sizeof(UINT16) * p_data->api_read_multi.num_attr);
 
-            status = GATTC_Read(p_clcb->bta_conn_id, GATT_READ_MULTIPLE, &read_param);
+        status = GATTC_Read(p_clcb->bta_conn_id, GATT_READ_MULTIPLE, &read_param);
-        }
 
-        /* read fail */
         if (status != BTA_GATT_OK) {
             bta_gattc_cmpl_sendmsg(p_clcb->bta_conn_id, GATTC_OPTYPE_READ, status, NULL);
         }
@@ -1266,16 +1267,13 @@ void bta_gattc_read_multi_var(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data)
     if (bta_gattc_enqueue(p_clcb, p_data)) {
         memset(&read_param, 0, sizeof(tGATT_READ_PARAM));
 
-        if (status == BTA_GATT_OK) {
+        read_param.read_multiple.num_handles = p_data->api_read_multi.num_attr;
-            read_param.read_multiple.num_handles = p_data->api_read_multi.num_attr;
+        read_param.read_multiple.auth_req = p_data->api_read_multi.auth_req;
-            read_param.read_multiple.auth_req = p_data->api_read_multi.auth_req;
+        memcpy(&read_param.read_multiple.handles, p_data->api_read_multi.handles,
-            memcpy(&read_param.read_multiple.handles, p_data->api_read_multi.handles,
+                sizeof(UINT16) * p_data->api_read_multi.num_attr);
-                    sizeof(UINT16) * p_data->api_read_multi.num_attr);
 
-            status = GATTC_Read(p_clcb->bta_conn_id, GATT_READ_MULTIPLE_VAR, &read_param);
+        status = GATTC_Read(p_clcb->bta_conn_id, GATT_READ_MULTIPLE_VAR, &read_param);
-        }
 
-        /* read fail */
         if (status != BTA_GATT_OK) {
             bta_gattc_cmpl_sendmsg(p_clcb->bta_conn_id, GATTC_OPTYPE_READ, status, NULL);
         }
@@ -1385,6 +1383,11 @@ void bta_gattc_read_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data)
     tBTA_GATTC          cb_data;
     tBTA_GATT_UNFMT     read_value;
 
+    if (p_clcb->p_q_cmd == NULL) {
+        APPL_TRACE_ERROR("%s, p_clcb->p_q_cmd is NULL", __func__);
+        return;
+    }
+
     memset(&cb_data, 0, sizeof(tBTA_GATTC));
     memset(&read_value, 0, sizeof(tBTA_GATT_UNFMT));
 
@@ -1427,6 +1430,18 @@ void bta_gattc_write_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data)
 {
     tBTA_GATTC      cb_data = {0};
     UINT8          event;
+    if (p_data->p_cmpl == NULL) {
+        APPL_TRACE_ERROR("%s, p_data->p_cmpl is NULL", __func__);
+        UINT16 handle = p_clcb->p_q_cmd->api_write.handle;
+        tBTA_GATTC_EVT cmpl_evt = p_clcb->p_q_cmd->api_write.cmpl_evt;
+        bta_gattc_free_command_data(p_clcb);
+        bta_gattc_pop_command_to_send(p_clcb);
+        cb_data.write.status = BTA_GATT_ERROR;
+        cb_data.write.handle = handle;
+        cb_data.write.conn_id = p_clcb->bta_conn_id;
+        ( *p_clcb->p_rcb->p_cback)(cmpl_evt, (tBTA_GATTC *)&cb_data);
+        return;
+    }
     tBTA_GATTC_CONN *p_conn = bta_gattc_conn_find(p_clcb->bda);
 
     memset(&cb_data, 0, sizeof(tBTA_GATTC));
@@ -1435,6 +1450,17 @@ void bta_gattc_write_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_OP_CMPL *p_data)
     cb_data.write.handle = p_data->p_cmpl->att_value.handle;
     if (p_clcb->p_q_cmd->api_write.hdr.event == BTA_GATTC_API_WRITE_EVT &&
         p_clcb->p_q_cmd->api_write.write_type == BTA_GATTC_WRITE_PREPARE) {
+        // Check if the parameters are valid
+        // should not happen, but just in case
+        if (p_clcb->p_q_cmd->api_write.p_value == NULL) {
+            APPL_TRACE_ERROR("%s, p_clcb->p_q_cmd->api_write.p_value is NULL", __func__);
+            bta_gattc_free_command_data(p_clcb);
+            bta_gattc_pop_command_to_send(p_clcb);
+            cb_data.write.status = BTA_GATT_ERROR;
+            cb_data.write.conn_id = p_clcb->bta_conn_id;
+            ( *p_clcb->p_rcb->p_cback)(BTA_GATTC_PREP_WRITE_EVT, (tBTA_GATTC *)&cb_data);
+            return;
+        }
         // Should check the value received from the peer device is correct or not.
         if (memcmp(p_clcb->p_q_cmd->api_write.p_value, p_data->p_cmpl->att_value.value,
                    p_data->p_cmpl->att_value.len) != 0) {
@@ -1530,6 +1556,13 @@ void  bta_gattc_op_cmpl(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data)
 
     APPL_TRACE_DEBUG("bta_gattc_op_cmpl op = %d", op);
 
+    // Check if the operation is valid
+    // should not happen, but just in case
+    if (op > GATTC_OPTYPE_INDICATION) {
+        APPL_TRACE_ERROR("unexpected operation, ignored");
+        return;
+    }
+
     if (op == GATTC_OPTYPE_INDICATION || op == GATTC_OPTYPE_NOTIFICATION) {
         APPL_TRACE_ERROR("unexpected operation, ignored");
     } else if (op >= GATTC_OPTYPE_READ) {
@@ -1914,49 +1947,30 @@ void bta_gattc_process_api_cache_assoc(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_m
 {
     tBTA_GATTC gattc_cb = {0};
     gattc_cb.set_assoc.client_if = p_msg->api_assoc.client_if;
+    gattc_cb.set_assoc.status = BTA_GATT_OK;
     BOOLEAN state = FALSE;
     tBTA_GATTC_CLCB *p_assoc_clcb = bta_gattc_find_clcb_by_cif(p_msg->api_assoc.client_if,
                                                              p_msg->api_assoc.assoc_addr, BTA_TRANSPORT_LE);
     tBTA_GATTC_RCB *p_clrcb = bta_gattc_cl_get_regcb(p_msg->api_assoc.client_if);
-    if (p_assoc_clcb != NULL) {
-        if (p_assoc_clcb->state == BTA_GATTC_CONN_ST || p_assoc_clcb->state == BTA_GATTC_DISCOVER_ST) {
-            gattc_cb.set_assoc.status = BTA_GATT_BUSY;
-            if (p_clrcb != NULL) {
-                (*p_clrcb->p_cback)(BTA_GATTC_ASSOC_EVT, &gattc_cb);
-                return;
-            }
-        }
-    }
 
-    if (p_msg->api_assoc.is_assoc) {
+    if (p_assoc_clcb != NULL &&
-        if ((state = bta_gattc_co_cache_append_assoc_addr(p_msg->api_assoc.src_addr, p_msg->api_assoc.assoc_addr)) == TRUE) {
+            (p_assoc_clcb->state == BTA_GATTC_CONN_ST || p_assoc_clcb->state == BTA_GATTC_DISCOVER_ST)) {
-            gattc_cb.set_assoc.status = BTA_GATT_OK;
+        gattc_cb.set_assoc.status = BTA_GATT_BUSY;
-
+    } else if (p_msg->api_assoc.is_assoc) {
-        } else {
+        state = bta_gattc_co_cache_append_assoc_addr(p_msg->api_assoc.src_addr, p_msg->api_assoc.assoc_addr);
+        if (!state) {
             gattc_cb.set_assoc.status = BTA_GATT_ERROR;
-            if (p_clrcb != NULL) {
-                (*p_clrcb->p_cback)(BTA_GATTC_ASSOC_EVT, &gattc_cb);
-                return;
-            }
         }
     } else {
-        if (( state = bta_gattc_co_cache_remove_assoc_addr(p_msg->api_assoc.src_addr, p_msg->api_assoc.assoc_addr)) == TRUE) {
+        state = bta_gattc_co_cache_remove_assoc_addr(p_msg->api_assoc.src_addr, p_msg->api_assoc.assoc_addr);
-            gattc_cb.set_assoc.status = BTA_GATT_OK;
+        if (!state) {
-        } else {
             gattc_cb.set_assoc.status = BTA_GATT_ERROR;
-            if (p_clrcb != NULL) {
-                (*p_clrcb->p_cback)(BTA_GATTC_ASSOC_EVT, &gattc_cb);
-                return;
-            }
         }
     }
 
-    if (p_clrcb != NULL) {
+    if (p_clrcb != NULL && p_clrcb->p_cback != NULL) {
         (*p_clrcb->p_cback)(BTA_GATTC_ASSOC_EVT, &gattc_cb);
     }
-
-    return;
-
 }
 void bta_gattc_process_api_cache_get_addr_list(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_msg)
 {
@@ -1978,7 +1992,7 @@ void bta_gattc_process_api_cache_get_addr_list(tBTA_GATTC_CB *p_cb, tBTA_GATTC_D
         gattc_cb.get_addr_list.status = BTA_GATT_NOT_FOUND;
     }
 
-    if (p_clrcb != NULL) {
+    if (p_clrcb != NULL && p_clrcb->p_cback != NULL) {
         (* p_clrcb->p_cback)(BTA_GATTC_GET_ADDR_LIST_EVT, &gattc_cb);
     }
 
@@ -2384,7 +2398,7 @@ tBTA_GATTC_FIND_SERVICE_CB bta_gattc_register_service_change_notify(UINT16 conn_
     tBTA_GATTC_SERVICE *p_service = NULL;
     tBTA_GATTC_CHARACTERISTIC *p_char = NULL;
     tBTA_GATTC_DESCRIPTOR *p_desc = NULL;
-    tBTA_GATTC_FIND_SERVICE_CB    result;
+    tBTA_GATTC_FIND_SERVICE_CB    result = SERVICE_CHANGE_CACHE_NOT_FOUND;
     BOOLEAN             gatt_cache_found = FALSE;
     BOOLEAN             gatt_service_found = FALSE;
     BOOLEAN             gatt_service_change_found = FALSE;

components/bt/host/bluedroid/bta/gatt/bta_gattc_api.c

@@ -151,6 +151,7 @@ void BTA_GATTC_Enh_Open(tBTA_GATTC_IF client_if, BD_ADDR remote_bda, tBTA_ADDR_T
     tBTA_GATTC_API_OPEN  *p_buf;
 
     if ((p_buf = (tBTA_GATTC_API_OPEN *) osi_malloc(sizeof(tBTA_GATTC_API_OPEN))) != NULL) {
+        memset(p_buf, 0, sizeof(tBTA_GATTC_API_OPEN));
         p_buf->hdr.event = BTA_GATTC_API_OPEN_EVT;
 
         p_buf->client_if = client_if;
@@ -365,7 +366,7 @@ void BTA_GATTC_GetServiceWithUUID(UINT16 conn_id, tBT_UUID *svc_uuid,
 void BTA_GATTC_GetAllChar(UINT16 conn_id, UINT16 start_handle, UINT16 end_handle,
                           btgatt_db_element_t **db, UINT16 *count)
 {
-    bta_gattc_get_db_with_opration(conn_id,
+    bta_gattc_get_db_with_operation(conn_id,
                                    GATT_OP_GET_ALL_CHAR,
                                    0,
                                    NULL,
@@ -380,7 +381,7 @@ void BTA_GATTC_GetAllChar(UINT16 conn_id, UINT16 start_handle, UINT16 end_handle
 void BTA_GATTC_GetAllDescriptor(UINT16 conn_id, UINT16 char_handle,
                                 btgatt_db_element_t **db, UINT16 *count)
 {
-    bta_gattc_get_db_with_opration(conn_id,
+    bta_gattc_get_db_with_operation(conn_id,
                                    GATT_OP_GET_ALL_DESCRI,
                                    char_handle,
                                    NULL,
@@ -395,7 +396,7 @@ void BTA_GATTC_GetAllDescriptor(UINT16 conn_id, UINT16 char_handle,
 void BTA_GATTC_GetCharByUUID(UINT16 conn_id, UINT16 start_handle, UINT16 end_handle, tBT_UUID char_uuid,
                              btgatt_db_element_t **db, UINT16 *count)
 {
-    bta_gattc_get_db_with_opration(conn_id,
+    bta_gattc_get_db_with_operation(conn_id,
                                    GATT_OP_GET_CHAR_BY_UUID,
                                    0,
                                    NULL,
@@ -411,7 +412,7 @@ void BTA_GATTC_GetDescrByUUID(UINT16 conn_id, uint16_t start_handle, uint16_t en
                               tBT_UUID char_uuid, tBT_UUID descr_uuid,
                               btgatt_db_element_t **db, UINT16 *count)
 {
-    bta_gattc_get_db_with_opration(conn_id,
+    bta_gattc_get_db_with_operation(conn_id,
                                    GATT_OP_GET_DESCRI_BY_UUID,
                                    0,
                                    NULL,
@@ -426,7 +427,7 @@ void BTA_GATTC_GetDescrByUUID(UINT16 conn_id, uint16_t start_handle, uint16_t en
 void BTA_GATTC_GetDescrByCharHandle(UINT16 conn_id, UINT16 char_handle, tBT_UUID descr_uuid,
                                     btgatt_db_element_t **db, UINT16 *count)
 {
-    bta_gattc_get_db_with_opration(conn_id,
+    bta_gattc_get_db_with_operation(conn_id,
                                    GATT_OP_GET_DESCRI_BY_HANDLE,
                                    char_handle,
                                    NULL,
@@ -441,7 +442,7 @@ void BTA_GATTC_GetDescrByCharHandle(UINT16 conn_id, UINT16 char_handle, tBT_UUID
 void BTA_GATTC_GetIncludeService(UINT16 conn_id, UINT16 start_handle, UINT16 end_handle,
                                  tBT_UUID *incl_uuid, btgatt_db_element_t **db, UINT16 *count)
 {
-    bta_gattc_get_db_with_opration(conn_id,
+    bta_gattc_get_db_with_operation(conn_id,
                                    GATT_OP_GET_INCLUDE_SVC,
                                    0,
                                    incl_uuid,
@@ -528,9 +529,8 @@ void BTA_GATTC_ReadCharacteristic(UINT16 conn_id, UINT16 handle, tBTA_GATT_AUTH_
 void BTA_GATTC_ReadCharDescr (UINT16 conn_id, UINT16 handle, tBTA_GATT_AUTH_REQ auth_req)
 {
     tBTA_GATTC_API_READ  *p_buf;
-    UINT16  len = (UINT16)(sizeof(tBTA_GATT_ID) + sizeof(tBTA_GATTC_API_READ));
 
-    if ((p_buf = (tBTA_GATTC_API_READ *) osi_malloc(len)) != NULL) {
+    if ((p_buf = (tBTA_GATTC_API_READ *) osi_malloc(sizeof(tBTA_GATTC_API_READ))) != NULL) {
         memset(p_buf, 0, sizeof(tBTA_GATTC_API_READ));
 
         p_buf->hdr.event = BTA_GATTC_API_READ_EVT;
@@ -560,6 +560,7 @@ void BTA_GATTC_ReadCharDescr (UINT16 conn_id, UINT16 handle, tBTA_GATT_AUTH_REQ
 void BTA_GATTC_ReadMultiple(UINT16 conn_id, tBTA_GATTC_MULTI *p_read_multi,
                             tBTA_GATT_AUTH_REQ auth_req)
 {
+    // p_read_multi should not be NULL
     tBTA_GATTC_API_READ_MULTI  *p_buf;
     //tBTA_GATTC_API_READ_MULTI          *p_value;
     UINT16      len = (UINT16)(sizeof(tBTA_GATTC_API_READ_MULTI));
@@ -921,7 +922,7 @@ tBTA_GATT_STATUS BTA_GATTC_RegisterForNotifications (tBTA_GATTC_IF client_if,
 
     if (!handle)
     {
-        APPL_TRACE_ERROR("deregistration failed, handle is 0");
+        APPL_TRACE_ERROR("registration failed, handle is 0");
         return status;
     }
 
@@ -1117,10 +1118,15 @@ typedef struct {
     } uuid;
 } __attribute__((packed)) tAPP_UUID;
 
-uint8_t BTA_GATTC_Discover(uint8_t gatt_if, uint16_t conn_id, void *uuid, uint8_t disc_type, uint16_t s_handle, uint16_t e_handle)
+int BTA_GATTC_Discover(uint8_t gatt_if, uint16_t conn_id, void *uuid, uint8_t disc_type, uint16_t s_handle, uint16_t e_handle)
 {
     tGATT_STATUS status;
     tGATT_DISC_PARAM param;
+
+    if (uuid == NULL && disc_type == GATT_DISC_SRVC_BY_UUID) {
+        APPL_TRACE_ERROR("%s invalid parameters", __func__);
+        return -1;
+    }
     tAPP_UUID *app_uuid = (tAPP_UUID *)uuid;
 
     conn_id = (UINT16)((((UINT8)conn_id) << 8) | gatt_if);
@@ -1144,6 +1150,7 @@ uint8_t BTA_GATTC_Discover(uint8_t gatt_if, uint16_t conn_id, void *uuid, uint8_
             memcpy(param.service.uu.uuid128, app_uuid->uuid.uuid128, LEN_UUID_128);
         } else {
             APPL_TRACE_ERROR("%s invalid uuid len %u", __func__, app_uuid->len);
+            return -1;
         }
     }
 

components/bt/host/bluedroid/bta/gatt/bta_gattc_cache.c

@@ -40,6 +40,7 @@
 #include "stack/l2c_api.h"
 #include "btm_int.h"
 #include "errno.h"
+#include "gatt_int.h"
 
 // #include "osi/include/log.h"
 
@@ -287,6 +288,12 @@ static tBTA_GATT_STATUS bta_gattc_add_srvc_to_cache(tBTA_GATTC_SERV *p_srvc_cb,
 
     if(!p_srvc_cb->p_srvc_cache) {
         APPL_TRACE_WARNING("%s(), no resource.", __func__);
+        if (p_new_srvc->characteristics) {
+            list_free(p_new_srvc->characteristics);
+        }
+        if (p_new_srvc->included_svc) {
+            list_free(p_new_srvc->included_svc);
+        }
         osi_free(p_new_srvc);
         return BTA_GATT_NO_RESOURCES;
     }
@@ -336,7 +343,14 @@ static tBTA_GATT_STATUS bta_gattc_add_char_to_cache(tBTA_GATTC_SERV *p_srvc_cb,
     memcpy(&characteristic->uuid, p_uuid, sizeof(tBT_UUID));
 
     characteristic->service = service;
-    list_append(service->characteristics, characteristic);
+    if (!list_append(service->characteristics, characteristic)) {
+        APPL_TRACE_WARNING("%s(), no resource.", __func__);
+        if (characteristic->descriptors) {
+            list_free(characteristic->descriptors);
+        }
+        osi_free(characteristic);
+        return BTA_GATT_NO_RESOURCES;
+    }
 
     return BTA_GATT_OK;
 }
@@ -556,7 +570,8 @@ void bta_gattc_update_include_service(const list_t *services) {
     for (list_node_t *sn = list_begin(services); sn != list_end(services); sn = list_next(sn)) {
         tBTA_GATTC_SERVICE *service = list_node(sn);
         if(!service || !service->included_svc || list_is_empty(service->included_svc)) break;
-        for (list_node_t *sn = list_begin(service->included_svc); sn != list_end(service->included_svc); sn = list_next(sn)) {
+        for (list_node_t *sn = list_begin(service->included_svc); sn != list_end(service->included_svc);) {
+            list_node_t *sn_next = list_next(sn);
             tBTA_GATTC_INCLUDED_SVC *include_service = list_node(sn);
             if(include_service && !include_service->included_service) {
                 //update
@@ -564,9 +579,10 @@ void bta_gattc_update_include_service(const list_t *services) {
                 if(!include_service->included_service) {
                     //not match, free it
                     list_remove(service->included_svc, include_service);
-                    osi_free(include_service);
+                    // osi_free(include_service);
                 }
             }
+            sn = sn_next;
         }
 
     }
@@ -594,7 +610,7 @@ static void bta_gattc_explore_srvc(UINT16 conn_id, tBTA_GATTC_SERV *p_srvc_cb)
         APPL_TRACE_ERROR("unknown connection ID");
         return;
     }
-    /* start expore a service if there is service not been explored */
+    /* start explore a service if there is service not been explored */
     if (p_srvc_cb->cur_srvc_idx < p_srvc_cb->total_srvc) {
         /* add the first service into cache */
         if (bta_gattc_add_srvc_to_cache (p_srvc_cb,
@@ -817,7 +833,7 @@ static tBTA_GATT_STATUS bta_gattc_add_char_to_list(tBTA_GATTC_SERV *p_srvc_cb,
         p_rec->e_handle = (p_srvc_cb->p_srvc_list + p_srvc_cb->cur_srvc_idx)->e_handle;
         memcpy(&p_rec->uuid, &uuid, sizeof(tBT_UUID));
 
-        /* update the endind handle of pervious characteristic if available */
+        /* update the endind handle of previous characteristic if available */
         if (p_srvc_cb->total_char > 1) {
             p_rec -= 1;
             p_rec->e_handle = decl_handle - 1;
@@ -1319,7 +1335,7 @@ void bta_gattc_fill_gatt_db_el(btgatt_db_element_t *p_attr,
     bta_to_btif_uuid(&p_attr->uuid, &uuid);
 }
 
-void bta_gattc_get_db_with_opration(UINT16 conn_id,
+void bta_gattc_get_db_with_operation(UINT16 conn_id,
                                                       bt_gatt_get_db_op_t op,
                                                       UINT16 char_handle,
                                                       tBT_UUID *incl_uuid,
@@ -1502,10 +1518,9 @@ static size_t bta_gattc_get_db_size_with_type(list_t *services,
     }
 
     size_t db_size = 0;
-    UINT16 svc_length = list_length(services) - 1;
 
     for (list_node_t *sn = list_begin(services);
-         sn != list_end(services); sn = list_next(sn), svc_length--) {
+         sn != list_end(services); sn = list_next(sn)) {
         tBTA_GATTC_SERVICE *p_cur_srvc = list_node(sn);
 
         if (p_cur_srvc->e_handle < start_handle) {
@@ -1519,12 +1534,8 @@ static size_t bta_gattc_get_db_size_with_type(list_t *services,
         if (type == BTGATT_DB_PRIMARY_SERVICE || type == BTGATT_DB_SECONDARY_SERVICE) {
             if ((type == BTGATT_DB_PRIMARY_SERVICE && p_cur_srvc->is_primary) ||
                 (type == BTGATT_DB_SECONDARY_SERVICE && !p_cur_srvc->is_primary)) {
-                // if the current service is the last service in the db, need to ensure the current service start handle is not less than the start_handle.
+                /* Count only when declaration handle s_handle is within [start_handle, end_handle] (GATT spec). */
-                if (!svc_length) {
+                if (p_cur_srvc->s_handle >= start_handle) {
-                    if (p_cur_srvc->s_handle >= start_handle) {
-                        db_size++;
-                    }
-                } else {
                     db_size++;
                 }
             }
@@ -1620,9 +1631,8 @@ static size_t bta_gattc_get_db_size(list_t *services,
     }
 
     size_t db_size = 0;
-    UINT16 svc_length = list_length(services) - 1;
     for (list_node_t *sn = list_begin(services);
-         sn != list_end(services); sn = list_next(sn), svc_length--) {
+         sn != list_end(services); sn = list_next(sn)) {
         tBTA_GATTC_SERVICE *p_cur_srvc = list_node(sn);
 
         if (p_cur_srvc->e_handle < start_handle) {
@@ -1633,41 +1643,38 @@ static size_t bta_gattc_get_db_size(list_t *services,
             break;
         }
 
-        if (!svc_length) {
+        /* Count service only when declaration handle s_handle is within [start_handle, end_handle] (GATT spec).
-            if (p_cur_srvc->s_handle >= start_handle) {
+         * Skip counting this service and all its contents when s_handle < start_handle (consistent with bta_gattc_get_gatt_db_impl). */
-                db_size++;
+        if (p_cur_srvc->s_handle < start_handle) {
-            }
-        } else {
-            db_size++;
-        }
-
-        if (!p_cur_srvc->characteristics || list_is_empty(p_cur_srvc->characteristics)) {
             continue;
         }
+        db_size++;
 
-        for (list_node_t *cn = list_begin(p_cur_srvc->characteristics);
+        if (p_cur_srvc->characteristics && !list_is_empty(p_cur_srvc->characteristics)) {
-             cn != list_end(p_cur_srvc->characteristics); cn = list_next(cn)) {
+            for (list_node_t *cn = list_begin(p_cur_srvc->characteristics);
-            tBTA_GATTC_CHARACTERISTIC *p_char = list_node(cn);
+                 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) {
+                if (p_char->handle < start_handle) {
-                continue;
+                    continue;
-            }
+                }
-            if (p_char->handle > end_handle) {
+                if (p_char->handle > end_handle) {
-                return db_size;
+                    return db_size;
-            }
+                }
-            db_size++;
+                db_size++;
 
-            if (p_char->descriptors) {
+                if (p_char->descriptors) {
-                for (list_node_t *dn = list_begin(p_char->descriptors);
+                    for (list_node_t *dn = list_begin(p_char->descriptors);
-                     dn != list_end(p_char->descriptors); dn = list_next(dn)) {
+                         dn != list_end(p_char->descriptors); dn = list_next(dn)) {
-                    tBTA_GATTC_DESCRIPTOR *p_desc = list_node(dn);
+                        tBTA_GATTC_DESCRIPTOR *p_desc = list_node(dn);
-                    if (p_desc->handle < start_handle) {
+                        if (p_desc->handle < start_handle) {
-                        continue;
+                            continue;
+                        }
+                        if (p_desc->handle > end_handle) {
+                            return db_size;
+                        }
+                        db_size++;
                     }
-                    if (p_desc->handle > end_handle) {
-                        return db_size;
-                    }
-                    db_size++;
                 }
             }
         }
@@ -1702,7 +1709,7 @@ void bta_gattc_get_db_size_handle(UINT16 conn_id, UINT16 start_handle, UINT16 en
     }
 
     tBTA_GATTC_SERV *p_srcb = p_clcb->p_srcb;
-    if (!p_srcb->p_srvc_cache || list_is_empty(p_srcb->p_srvc_cache)) {
+    if ((p_srcb == NULL) || !p_srcb->p_srvc_cache || list_is_empty(p_srcb->p_srvc_cache)) {
         *count = 0;
         return;
     }
@@ -1792,6 +1799,11 @@ static void bta_gattc_get_gatt_db_impl(tBTA_GATTC_SERV *p_srvc_cb,
             break;
         }
 
+        /* Output service only when declaration handle s_handle is within [start_handle, end_handle] (consistent with count logic). */
+        if (p_cur_srvc->s_handle < start_handle) {
+            continue;
+        }
+
         bta_gattc_fill_gatt_db_el(curr_db_attr,
                                   p_cur_srvc->is_primary ?
                                   BTGATT_DB_PRIMARY_SERVICE :
@@ -1879,8 +1891,8 @@ static void bta_gattc_get_gatt_db_impl(tBTA_GATTC_SERV *p_srvc_cb,
             bta_gattc_fill_gatt_db_el(curr_db_attr,
                                       BTGATT_DB_INCLUDED_SERVICE,
                                       p_isvc->handle,
-                                      0 /* s_handle */,
+                                      p_isvc->incl_srvc_s_handle,
-                                      0 /* e_handle */,
+                                      p_isvc->incl_srvc_e_handle,
                                       p_isvc->handle,
                                       p_isvc->uuid,
                                       0 /* property */);
@@ -1912,17 +1924,26 @@ void bta_gattc_get_gatt_db(UINT16 conn_id, UINT16 start_handle, UINT16 end_handl
 
     if (p_clcb == NULL) {
         APPL_TRACE_ERROR("Unknown conn ID: %d", conn_id);
+        *count = 0;
+        *db = NULL;
         return;
     }
 
     if (p_clcb->state != BTA_GATTC_CONN_ST) {
         APPL_TRACE_ERROR("server cache not available, CLCB state = %d",
                          p_clcb->state);
+        *count = 0;
+        *db = NULL;
         return;
     }
 
-    if (!p_clcb->p_srcb || p_clcb->p_srcb->p_srvc_list || /* no active discovery */
+    /* Reject if no server context, discovery still in progress (p_srvc_list in use), or cache not ready
+    p_srvc_list must set to NULL after osi_free;
+    */
+    if (!p_clcb->p_srcb || p_clcb->p_srcb->p_srvc_list ||
         !p_clcb->p_srcb->p_srvc_cache) {
+        *count = 0;
+        *db = NULL;
         APPL_TRACE_ERROR("No server cache available");
         return;
     }
@@ -2032,10 +2053,11 @@ void bta_gattc_cache_save(tBTA_GATTC_SERV *p_srvc_cb, UINT16 conn_id)
         return;
     }
 
-    int i = 0;
+    /* i: current write index, equals actual count after loops; db_size: allocated slots from cache count */
+    size_t i = 0;
     size_t db_size = bta_gattc_get_db_size(p_srvc_cb->p_srvc_cache, 0x0000, 0xFFFF);
     tBTA_GATTC_NV_ATTR *nv_attr = osi_malloc(db_size * sizeof(tBTA_GATTC_NV_ATTR));
-    // This step is very importent, if not clear the memory, the hasy key base on the attribute case will be not corret.
+    // This step is very important, if not clear the memory, the hasy key base on the attribute case will be not correct.
     if (nv_attr != NULL) {
         memset(nv_attr, 0, db_size * sizeof(tBTA_GATTC_NV_ATTR));
     }
@@ -2048,6 +2070,11 @@ void bta_gattc_cache_save(tBTA_GATTC_SERV *p_srvc_cb, UINT16 conn_id)
          sn != list_end(p_srvc_cb->p_srvc_cache); sn = list_next(sn)) {
         tBTA_GATTC_SERVICE *p_cur_srvc = list_node(sn);
 
+        /* Bounds check: avoid buffer overflow if cache is modified and count exceeds db_size */
+        if (i >= db_size) {
+            APPL_TRACE_ERROR("%s(), db_size exceeded (services), skip saving.", __func__);
+            goto save_error;
+        }
         bta_gattc_fill_nv_attr(&nv_attr[i++],
                                 BTA_GATTC_ATTR_TYPE_SRVC,
                                p_cur_srvc->s_handle,
@@ -2063,41 +2090,49 @@ void bta_gattc_cache_save(tBTA_GATTC_SERV *p_srvc_cb, UINT16 conn_id)
          sn != list_end(p_srvc_cb->p_srvc_cache); sn = list_next(sn)) {
         tBTA_GATTC_SERVICE *p_cur_srvc = list_node(sn);
 
-        if (!p_cur_srvc->characteristics || list_is_empty(p_cur_srvc->characteristics)) {
+        if (p_cur_srvc->characteristics && !list_is_empty(p_cur_srvc->characteristics)) {
-            continue;
+            for (list_node_t *cn = list_begin(p_cur_srvc->characteristics);
-        }
+                 cn != list_end(p_cur_srvc->characteristics); cn = list_next(cn)) {
-
+                tBTA_GATTC_CHARACTERISTIC *p_char = list_node(cn);
-        for (list_node_t *cn = list_begin(p_cur_srvc->characteristics);
-             cn != list_end(p_cur_srvc->characteristics); cn = list_next(cn)) {
-            tBTA_GATTC_CHARACTERISTIC *p_char = list_node(cn);
-
-            bta_gattc_fill_nv_attr(&nv_attr[i++],
-                                   BTA_GATTC_ATTR_TYPE_CHAR,
-                                   p_char->handle,
-                                   0,
-                                   p_char->uuid,
-                                   p_char->properties,
-                                   0 /* incl_srvc_s_handle */,
-                                   0 /* incl_srvc_e_handle */,
-                                   FALSE);
-
-            if (!p_char->descriptors || list_is_empty(p_char->descriptors)) {
-                continue;
-            }
-
-            for (list_node_t *dn = list_begin(p_char->descriptors);
-                 dn != list_end(p_char->descriptors); dn = list_next(dn)) {
-                tBTA_GATTC_DESCRIPTOR *p_desc = list_node(dn);
 
+                /* Bounds check: avoid buffer overflow if count exceeds db_size */
+                if (i >= db_size) {
+                    APPL_TRACE_ERROR("%s(), db_size exceeded (characteristics), skip saving.", __func__);
+                    goto save_error;
+                }
                 bta_gattc_fill_nv_attr(&nv_attr[i++],
-                                       BTA_GATTC_ATTR_TYPE_CHAR_DESCR,
+                                       BTA_GATTC_ATTR_TYPE_CHAR,
-                                       p_desc->handle,
+                                       p_char->handle,
                                        0,
-                                       p_desc->uuid,
+                                       p_char->uuid,
-                                       0 /* properties */,
+                                       p_char->properties,
                                        0 /* incl_srvc_s_handle */,
                                        0 /* incl_srvc_e_handle */,
                                        FALSE);
+
+                if (!p_char->descriptors || list_is_empty(p_char->descriptors)) {
+                    continue;
+                }
+
+                for (list_node_t *dn = list_begin(p_char->descriptors);
+                     dn != list_end(p_char->descriptors); dn = list_next(dn)) {
+                    tBTA_GATTC_DESCRIPTOR *p_desc = list_node(dn);
+
+                    /* Bounds check: avoid buffer overflow if count exceeds db_size */
+                    if (i >= db_size) {
+                        APPL_TRACE_ERROR("%s(), db_size exceeded (descriptors), skip saving.", __func__);
+                        goto save_error;
+                    }
+                    bta_gattc_fill_nv_attr(&nv_attr[i++],
+                                           BTA_GATTC_ATTR_TYPE_CHAR_DESCR,
+                                           p_desc->handle,
+                                           0,
+                                           p_desc->uuid,
+                                           0 /* properties */,
+                                           0 /* incl_srvc_s_handle */,
+                                           0 /* incl_srvc_e_handle */,
+                                           FALSE);
+                }
             }
         }
 
@@ -2108,21 +2143,32 @@ void bta_gattc_cache_save(tBTA_GATTC_SERV *p_srvc_cb, UINT16 conn_id)
         for (list_node_t *an = list_begin(p_cur_srvc->included_svc);
              an != list_end(p_cur_srvc->included_svc); an = list_next(an)) {
             tBTA_GATTC_INCLUDED_SVC *p_isvc = list_node(an);
+            UINT16 incl_s_handle = p_isvc->included_service ? p_isvc->included_service->s_handle : p_isvc->incl_srvc_s_handle;
+            UINT16 incl_e_handle = p_isvc->included_service ? p_isvc->included_service->e_handle : p_isvc->incl_srvc_e_handle;
 
+            /* Bounds check: avoid buffer overflow if count exceeds db_size */
+            if (i >= db_size) {
+                APPL_TRACE_ERROR("%s(), db_size exceeded (included_svc), skip saving.", __func__);
+                goto save_error;
+            }
             bta_gattc_fill_nv_attr(&nv_attr[i++],
                                    BTA_GATTC_ATTR_TYPE_INCL_SRVC,
                                    p_isvc->handle,
                                    0,
                                    p_isvc->uuid,
                                    0 /* properties */,
-                                   p_isvc->included_service->s_handle,
+                                   incl_s_handle,
-                                   p_isvc->included_service->e_handle,
+                                   incl_e_handle,
                                    FALSE);
         }
     }
 
-    /* TODO: Gattc cache write/read need to be added in IDF 3.1*/
+    bta_gattc_cache_write(p_srvc_cb->server_bda, (UINT16)db_size, nv_attr);
-    bta_gattc_cache_write(p_srvc_cb->server_bda, db_size, nv_attr);
+    osi_free(nv_attr);
+    return;
+
+save_error:
+    APPL_TRACE_ERROR("%s(), cache inconsistency detected, not saving partial cache.", __func__);
     osi_free(nv_attr);
 }
 
@@ -2149,11 +2195,14 @@ bool bta_gattc_cache_load(tBTA_GATTC_CLCB *p_clcb)
         return false;
     }
 
-    size_t num_attr = bta_gattc_get_cache_attr_length(index) / sizeof(tBTA_GATTC_NV_ATTR);
+    size_t length = bta_gattc_get_cache_attr_length(index);
 
-    if (!num_attr) {
+    if (length == 0 || (length % sizeof(tBTA_GATTC_NV_ATTR)) != 0) {
+        APPL_TRACE_ERROR("%s, Invalid cache length %d", __func__, length);
         return false;
     }
+
+    size_t num_attr = length / sizeof(tBTA_GATTC_NV_ATTR);
     //don't forget to set the total attribute number.
     p_clcb->p_srcb->total_attr = num_attr;
     APPL_TRACE_DEBUG("%s(), index = %x, num_attr = %d", __func__, index, num_attr);
@@ -2163,6 +2212,7 @@ bool bta_gattc_cache_load(tBTA_GATTC_CLCB *p_clcb)
     }
     if ((status = bta_gattc_co_cache_load(attr, index)) != BTA_GATT_OK) {
         APPL_TRACE_DEBUG("%s(), gattc cache load fail, status = %x", __func__, status);
+        osi_free(attr);
         return false;
     }
     p_clcb->searched_service_source = BTA_GATTC_SERVICE_INFO_FROM_NVS_FLASH;

components/bt/host/bluedroid/bta/gatt/bta_gattc_co.c

@@ -172,6 +172,14 @@ static void cacheReset(BD_ADDR bda, BOOLEAN update)
             return;
         }
 
+        // Free the assoc_addr list_t structure before array shift to prevent memory leak
+        // Note: bta_gattc_co_cache_clear_assoc_addr only calls list_clear which clears nodes
+        // but doesn't free the list_t structure itself
+        if (cache_env->cache_addr[index].assoc_addr != NULL) {
+            list_free(cache_env->cache_addr[index].assoc_addr);
+            cache_env->cache_addr[index].assoc_addr = NULL;
+        }
+
         UINT8 num = cache_env->num_addr;
         //delete the server_bda in the addr_info list.
         for(UINT8 i = index; i < (num - 1); i++) {
@@ -435,6 +443,8 @@ void bta_gattc_co_cache_addr_init(void)
     } else {
         APPL_TRACE_ERROR("%s, Line = %d, nvs flash open fail, err_code = %x", __func__, __LINE__, err_code);
         osi_free(p_buf);
+        osi_free(cache_env);
+        cache_env = NULL;
         return;
     }
 
@@ -444,11 +454,18 @@ void bta_gattc_co_cache_addr_init(void)
 
 void bta_gattc_co_cache_addr_deinit(void)
 {
-    APPL_TRACE_DEBUG("%s is_open=%d", __func__, cache_env->is_open);
+    if(cache_env == NULL) {
-
-    if(!cache_env->is_open) {
         return;
     }
+
+    APPL_TRACE_DEBUG("%s is_open=%d", __func__, cache_env->is_open);
+
+    if (!cache_env->is_open) {
+        osi_free(cache_env);
+        cache_env = NULL;
+        return;
+    }
+
     nvs_close(cache_env->addr_fp);
     cache_env->is_open = false;
 
@@ -608,9 +625,30 @@ BOOLEAN bta_gattc_co_cache_append_assoc_addr(BD_ADDR src_addr, BD_ADDR assoc_add
     if ((addr_index = bta_gattc_co_find_addr_in_cache(src_addr)) != INVALID_ADDR_NUM) {
         addr_info = &cache_env->cache_addr[addr_index];
         if (addr_info->assoc_addr == NULL) {
-            addr_info->assoc_addr =list_new(NULL);
+            addr_info->assoc_addr =list_new(osi_free_func);
         }
-        return list_append(addr_info->assoc_addr, p_assoc_buf);
+        if (addr_info->assoc_addr == NULL) {
+            APPL_TRACE_ERROR("assoc_addr list creation failed");
+            osi_free(p_assoc_buf);
+            return FALSE;
+        }
+
+        for (list_node_t *sn = list_begin(addr_info->assoc_addr);
+             sn != list_end(addr_info->assoc_addr); sn = list_next(sn)) {
+            if (!memcmp(list_node(sn), assoc_addr, sizeof(BD_ADDR))) {
+                APPL_TRACE_WARNING("Association already exists");
+                osi_free(p_assoc_buf);
+                return TRUE;
+            }
+        }
+
+        if (!list_append(addr_info->assoc_addr, p_assoc_buf)) {
+            APPL_TRACE_ERROR("Failed to append to assoc_addr list");
+            osi_free(p_assoc_buf);
+            return FALSE;
+
+        }
+        return TRUE;
     } else {
         osi_free(p_assoc_buf);
     }
@@ -643,16 +681,15 @@ BOOLEAN bta_gattc_co_cache_remove_assoc_addr(BD_ADDR src_addr, BD_ADDR assoc_add
 
 UINT8* bta_gattc_co_cache_find_src_addr(BD_ADDR assoc_addr, UINT8 *index)
 {
-    UINT8 num = cache_env->num_addr;
+    UINT8 num = (cache_env->num_addr > MAX_DEVICE_IN_CACHE) ? MAX_DEVICE_IN_CACHE : cache_env->num_addr;
     cache_addr_info_t *addr_info = &cache_env->cache_addr[0];
     UINT8 *addr_data;
-    //Check the assoc_addr list is NULL or not
-    if (addr_info->assoc_addr == NULL) {
-        *index = INVALID_ADDR_NUM;
-        return NULL;
-    }
 
     for (int i = 0; i < num; i++) {
+        if (addr_info->assoc_addr == NULL) {
+            addr_info++;
+            continue;
+        }
        for (const list_node_t *node = list_begin(addr_info->assoc_addr); node != list_end(addr_info->assoc_addr);
             node = list_next(node)) {
             addr_data = (UINT8 *)list_node(node);
@@ -662,11 +699,6 @@ UINT8* bta_gattc_co_cache_find_src_addr(BD_ADDR assoc_addr, UINT8 *index)
            }
        }
        addr_info++;
-
-       if (addr_info->assoc_addr == NULL) {
-           *index = INVALID_ADDR_NUM;
-           return NULL;
-       }
     }
 
     *index = INVALID_ADDR_NUM;

components/bt/host/bluedroid/bta/gatt/bta_gattc_main.c

@@ -41,7 +41,6 @@
 enum {
     BTA_GATTC_OPEN,
     BTA_GATTC_OPEN_FAIL,
-    BTA_GATTC_OPEN_ERROR,
     BTA_GATTC_CANCEL_OPEN,
     BTA_GATTC_CANCEL_OPEN_OK,
     BTA_GATTC_CANCEL_OPEN_ERROR,
@@ -77,7 +76,6 @@ typedef void (*tBTA_GATTC_ACTION)(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_da
 const tBTA_GATTC_ACTION bta_gattc_action[] = {
     bta_gattc_open,
     bta_gattc_open_fail,
-    bta_gattc_open_error,
     bta_gattc_cancel_open,
     bta_gattc_cancel_open_ok,
     bta_gattc_cancel_open_error,
@@ -289,6 +287,11 @@ BOOLEAN bta_gattc_sm_execute(tBTA_GATTC_CLCB *p_clcb, UINT16 event, tBTA_GATTC_D
                      gattc_evt_code(in_event));
 #endif
 
+    // should not happen, but just in case
+    if (!p_clcb) {
+        APPL_TRACE_ERROR("p_clcb or p_data is NULL, event is %d", event);
+        return rt;
+    }
 
     /* look up the state table for the current state */
     state_table = bta_gattc_st_tbl[p_clcb->state];

components/bt/host/bluedroid/bta/gatt/bta_gattc_utils.c

@@ -35,37 +35,16 @@
 #include "bta_gattc_int.h"
 #include "stack/l2c_api.h"
 #include "osi/allocator.h"
+#include "gatt_int.h"
 
 /*****************************************************************************
 **  Constants
 *****************************************************************************/
 
-
-static const UINT8  base_uuid[LEN_UUID_128] = {0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
-                                               0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
-                                              };
-
 static const BD_ADDR dummy_bda = {0, 0, 0, 0, 0, 0};
 
 #define GATTC_COMMAND_QUEUE_SIZE_MAX    30
 
-/*******************************************************************************
-**
-** Function         bta_gatt_convert_uuid16_to_uuid128
-**
-** Description      Convert a 16 bits UUID to be an standard 128 bits one.
-**
-** Returns          TRUE if two uuid match; FALSE otherwise.
-**
-*******************************************************************************/
-void bta_gatt_convert_uuid16_to_uuid128(UINT8 uuid_128[LEN_UUID_128], UINT16 uuid_16)
-{
-    UINT8   *p = &uuid_128[LEN_UUID_128 - 4];
-
-    memcpy (uuid_128, base_uuid, LEN_UUID_128);
-
-    UINT16_TO_STREAM(p, uuid_16);
-}
 /*******************************************************************************
 **
 ** Function         bta_gattc_uuid_compare
@@ -77,11 +56,8 @@ void bta_gatt_convert_uuid16_to_uuid128(UINT8 uuid_128[LEN_UUID_128], UINT16 uui
 *******************************************************************************/
 BOOLEAN bta_gattc_uuid_compare (const tBT_UUID *p_src, const tBT_UUID *p_tar, BOOLEAN is_precise)
 {
-    UINT8  su[LEN_UUID_128], tu[LEN_UUID_128];
-    const UINT8  *ps, *pt;
-
     /* any of the UUID is unspecified */
-    if (p_src == 0 || p_tar == 0) {
+    if (p_src == 0 || p_tar == 0 || p_src->len == 0 || p_tar->len == 0) {
         if (is_precise) {
             return FALSE;
         } else {
@@ -89,29 +65,7 @@ BOOLEAN bta_gattc_uuid_compare (const tBT_UUID *p_src, const tBT_UUID *p_tar, BO
         }
     }
 
-    /* If both are 16-bit, we can do a simple compare */
+    return gatt_uuid_compare(*p_src, *p_tar);
-    if (p_src->len == 2 && p_tar->len == 2) {
-        return p_src->uu.uuid16 == p_tar->uu.uuid16;
-    }
-
-    /* One or both of the UUIDs is 128-bit */
-    if (p_src->len == LEN_UUID_16) {
-        /* convert a 16 bits UUID to 128 bits value */
-        bta_gatt_convert_uuid16_to_uuid128(su, p_src->uu.uuid16);
-        ps = su;
-    } else {
-        ps = p_src->uu.uuid128;
-    }
-
-    if (p_tar->len == LEN_UUID_16) {
-        /* convert a 16 bits UUID to 128 bits value */
-        bta_gatt_convert_uuid16_to_uuid128(tu, p_tar->uu.uuid16);
-        pt = tu;
-    } else {
-        pt = p_tar->uu.uuid128;
-    }
-
-    return (memcmp(ps, pt, LEN_UUID_128) == 0);
 }
 
 /*******************************************************************************
@@ -242,6 +196,10 @@ tBTA_GATTC_CLCB *bta_gattc_clcb_alloc(tBTA_GATTC_IF client_if, BD_ADDR remote_bd
                 p_clcb->p_srcb->num_clcb ++;
                 p_clcb->p_rcb->num_clcb ++;
             } else {
+                if (p_clcb->p_cmd_list) {
+                     list_free(p_clcb->p_cmd_list);
+                     p_clcb->p_cmd_list = NULL;
+                }
                 /* release this clcb if clcb or srcb allocation failed */
                 p_clcb->in_use = FALSE;
                 p_clcb = NULL;
@@ -305,6 +263,9 @@ void bta_gattc_clcb_dealloc(tBTA_GATTC_CLCB *p_clcb)
                 list_free(p_srcb->p_srvc_cache);
                 p_srcb->p_srvc_cache = NULL;
             }
+#if (GATTC_CACHE_NVS == TRUE)
+            bta_gattc_co_cache_close(p_srcb->server_bda, 0);
+#endif
         }
 
         if ( p_clcb->p_q_cmd != NULL && !list_contains(p_clcb->p_cmd_list, p_clcb->p_q_cmd)){
@@ -740,12 +701,12 @@ BOOLEAN bta_gattc_mark_bg_conn (tBTA_GATTC_IF client_if,  BD_ADDR_PTR remote_bda
             if (add)
                 /* mask on the cif bit */
             {
-                *p_cif_mask |= (1 << (client_if - 1));
+                *p_cif_mask |= BTA_GATTC_CIF_MASK_BIT(client_if);
             } else {
-                if (client_if != 0) {
+                if (client_if == 0) {
-                    *p_cif_mask &= (~(1 << (client_if - 1)));
-                } else {
                     *p_cif_mask = 0;
+                } else {
+                    *p_cif_mask &= (~BTA_GATTC_CIF_MASK_BIT(client_if));
                 }
             }
             /* no BG connection for this device, make it available */
@@ -777,7 +738,7 @@ BOOLEAN bta_gattc_mark_bg_conn (tBTA_GATTC_IF client_if,  BD_ADDR_PTR remote_bda
 
                 p_cif_mask = is_listen ? &p_bg_tck->cif_adv_mask : &p_bg_tck->cif_mask;
 
-                *p_cif_mask = (1 << (client_if - 1));
+                *p_cif_mask = BTA_GATTC_CIF_MASK_BIT(client_if);
                 return TRUE;
             }
         }
@@ -804,12 +765,12 @@ BOOLEAN bta_gattc_check_bg_conn (tBTA_GATTC_IF client_if,  BD_ADDR remote_bda, U
         if (p_bg_tck->in_use &&
                 (bdcmp(p_bg_tck->remote_bda, remote_bda) == 0 ||
                  bdcmp(p_bg_tck->remote_bda, dummy_bda) == 0)) {
-            if (((p_bg_tck->cif_mask & (1 << (client_if - 1))) != 0) &&
+            if (((p_bg_tck->cif_mask & BTA_GATTC_CIF_MASK_BIT(client_if)) != 0) &&
                     role == HCI_ROLE_MASTER) {
                 is_bg_conn = TRUE;
             }
 
-            if (((p_bg_tck->cif_adv_mask & (1 << (client_if - 1))) != 0) &&
+            if (((p_bg_tck->cif_adv_mask & BTA_GATTC_CIF_MASK_BIT(client_if)) != 0) &&
                     role == HCI_ROLE_SLAVE) {
                 is_bg_conn = TRUE;
             }

components/bt/host/bluedroid/bta/gatt/bta_gatts_act.c

@@ -36,6 +36,7 @@
 #include <string.h>
 #include "osi/allocator.h"
 #include "l2c_int.h"
+#include "gatt_int.h"
 
 static void bta_gatts_nv_save_cback(BOOLEAN is_saved, tGATTS_HNDL_RANGE *p_hndl_range);
 static BOOLEAN bta_gatts_nv_srv_chg_cback(tGATTS_SRV_CHG_CMD cmd, tGATTS_SRV_CHG_REQ *p_req,
@@ -176,13 +177,15 @@ void bta_gatts_register(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg)
     tBTA_GATT_STATUS         status = BTA_GATT_OK;
     UINT8                    i, first_unuse = 0xff;
 
+    memset(&cb_data, 0, sizeof(tBTA_GATTS));
+
     if (p_cb->enabled == FALSE) {
         bta_gatts_enable(p_cb);
     }
 
     for (i = 0; i < BTA_GATTS_MAX_APP_NUM; i ++) {
         if (p_cb->rcb[i].in_use) {
-            if (bta_gatts_uuid_compare(p_cb->rcb[i].app_uuid, p_msg->api_reg.app_uuid)) {
+            if (gatt_uuid_compare(p_cb->rcb[i].app_uuid, p_msg->api_reg.app_uuid)) {
                 APPL_TRACE_ERROR("application already registered.\n");
                 status = BTA_GATT_DUP_REG;
                 break;
@@ -199,9 +202,7 @@ void bta_gatts_register(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg)
         }
 
         cb_data.reg_oper.server_if = BTA_GATTS_INVALID_IF;
-// btla-specific ++
         memcpy(&cb_data.reg_oper.uuid, &p_msg->api_reg.app_uuid, sizeof(tBT_UUID));
-// btla-specific --
         if (first_unuse != 0xff) {
             APPL_TRACE_VERBOSE("register application first_unuse rcb_idx = %d", first_unuse);
 
@@ -213,6 +214,7 @@ void bta_gatts_register(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg)
                     GATT_Register(&p_msg->api_reg.app_uuid, &bta_gatts_cback);
             if ( !p_cb->rcb[first_unuse].gatt_if) {
                 status = BTA_GATT_NO_RESOURCES;
+                memset( &p_cb->rcb[first_unuse], 0 , sizeof(tBTA_GATTS_RCB));
             } else {
                 if ((p_buf =
                             (tBTA_GATTS_INT_START_IF *) osi_malloc(sizeof(tBTA_GATTS_INT_START_IF))) != NULL) {
@@ -222,6 +224,7 @@ void bta_gatts_register(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg)
                     bta_sys_sendmsg(p_buf);
                 } else {
                     status = BTA_GATT_NO_RESOURCES;
+                    GATT_Deregister(p_cb->rcb[first_unuse].gatt_if);
                     memset( &p_cb->rcb[first_unuse], 0 , sizeof(tBTA_GATTS_RCB));
                 }
             }
@@ -309,7 +312,7 @@ void bta_gatts_deregister(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg)
 void bta_gatts_create_srvc(tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg)
 {
     UINT8               rcb_idx;
-    tBTA_GATTS          cb_data;
+    tBTA_GATTS          cb_data = {0};
     UINT8               srvc_idx;
     UINT16              service_id = 0;
 

components/bt/host/bluedroid/bta/gatt/bta_gatts_api.c

@@ -302,6 +302,8 @@ void BTA_GATTS_AddCharDescriptor (UINT16 service_id,
                     memcpy(p_buf->attr_val.attr_val, attr_val->attr_val, value_len);
                 }
                 else{
+                    p_buf->attr_val.attr_len = 0;
+                    p_buf->attr_val.attr_max_len = 0;
                     APPL_TRACE_ERROR("Allocate fail for %s\n", __func__);
 
                 }

components/bt/host/bluedroid/bta/gatt/bta_gatts_main.c

@@ -109,6 +109,14 @@ BOOLEAN bta_gatts_hdl_event(BT_HDR *p_msg)
     case BTA_GATTS_API_SET_ATTR_VAL_EVT:{
         UINT16 attr_id = ((tBTA_GATTS_DATA *) p_msg)->api_set_val.hdr.layer_specific;
         p_srvc_cb = bta_gatts_find_srvc_cb_by_attr_id(p_cb, attr_id);
+        if (p_srvc_cb == NULL) {
+            if (((tBTA_GATTS_DATA *) p_msg)->api_set_val.value != NULL) {
+                osi_free(((tBTA_GATTS_DATA *) p_msg)->api_set_val.value);
+            }
+            APPL_TRACE_ERROR("Service not found for attribute ID: %d", attr_id);
+            break;
+        }
+
         bta_gatts_set_attr_value(p_srvc_cb, (tBTA_GATTS_DATA *) p_msg);
         break;
     }

components/bt/host/bluedroid/bta/gatt/bta_gatts_utils.c

@@ -31,27 +31,7 @@
 #include "bta/bta_sys.h"
 #include "bta_gatts_int.h"
 
-static const UINT8  base_uuid[LEN_UUID_128] = {0xFB, 0x34, 0x9B, 0x5F, 0x80, 0x00, 0x00, 0x80,
-                                               0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
-                                              };
 
-/*******************************************************************************
-**
-** Function         bta_gatt_convert_uuid16_to_uuid128
-**
-** Description      Convert a 16 bits UUID to be an standard 128 bits one.
-**
-** Returns          TRUE if two uuid match; FALSE otherwise.
-**
-*******************************************************************************/
-static void bta_gatt_convert_uuid16_to_uuid128(UINT8 uuid_128[LEN_UUID_128], UINT16 uuid_16)
-{
-    UINT8   *p = &uuid_128[LEN_UUID_128 - 4];
-
-    memcpy (uuid_128, base_uuid, LEN_UUID_128);
-
-    UINT16_TO_STREAM(p, uuid_16);
-}
 /*******************************************************************************
 **
 ** Function         bta_gatts_alloc_srvc_cb
@@ -167,6 +147,7 @@ tBTA_GATTS_SRVC_CB *bta_gatts_find_srvc_cb_by_attr_id(tBTA_GATTS_CB *p_cb, UINT1
              attr_id >= p_cb->srvc_cb[i].service_id)    ||
             /* last service incb */
             (i == (BTA_GATTS_MAX_SRVC_NUM - 1) &&
+             p_cb->srvc_cb[i].in_use &&
              attr_id >= p_cb->srvc_cb[i].service_id)
         ) {
             return &p_cb->srvc_cb[i];
@@ -174,51 +155,5 @@ tBTA_GATTS_SRVC_CB *bta_gatts_find_srvc_cb_by_attr_id(tBTA_GATTS_CB *p_cb, UINT1
     }
     return NULL;
 }
-/*******************************************************************************
-**
-** Function         bta_gatts_uuid_compare
-**
-** Description      Compare two UUID to see if they are the same.
-**
-** Returns          TRUE if two uuid match; FALSE otherwise.
-**
-*******************************************************************************/
-BOOLEAN bta_gatts_uuid_compare(tBT_UUID tar, tBT_UUID src)
-{
-    UINT8  su[LEN_UUID_128], tu[LEN_UUID_128];
-    UINT8  *ps, *pt;
-
-    /* any of the UUID is unspecified */
-    if (src.len == 0 || tar.len == 0) {
-        return TRUE;
-    }
-
-    /* If both are 16-bit, we can do a simple compare */
-    if (src.len == 2 && tar.len == 2) {
-        return src.uu.uuid16 == tar.uu.uuid16;
-    }
-
-    /* One or both of the UUIDs is 128-bit */
-    if (src.len == LEN_UUID_16) {
-        /* convert a 16 bits UUID to 128 bits value */
-        bta_gatt_convert_uuid16_to_uuid128(su, src.uu.uuid16);
-        ps = su;
-    } else {
-        ps = src.uu.uuid128;
-    }
-
-    if (tar.len == LEN_UUID_16) {
-        /* convert a 16 bits UUID to 128 bits value */
-        bta_gatt_convert_uuid16_to_uuid128(tu, tar.uu.uuid16);
-        pt = tu;
-    } else {
-        pt = tar.uu.uuid128;
-    }
-
-    return (memcmp(ps, pt, LEN_UUID_128) == 0);
-}
-
-
-
 
 #endif  /* GATTS_INCLUDED */

components/bt/host/bluedroid/bta/gatt/include/bta_gattc_int.h

@@ -383,6 +383,10 @@ typedef UINT16 tBTA_GATTC_CIF_MASK;
 typedef UINT32 tBTA_GATTC_CIF_MASK;
 #endif
 
+/* Safe bit for client_if in cif_mask; avoids UB when client_if is 0 or > GATT_MAX_APPS */
+#define BTA_GATTC_CIF_MASK_BIT(cif) \
+    ((tBTA_GATTC_CIF_MASK)(((cif) >= 1 && (cif) <= (unsigned)GATT_MAX_APPS) ? (1U << ((cif) - 1)) : 0U))
+
 typedef struct {
     BOOLEAN                 in_use;
     BD_ADDR                 remote_bda;
@@ -456,7 +460,6 @@ extern void bta_gattc_process_enc_cmpl(tBTA_GATTC_CB *p_cb, tBTA_GATTC_DATA *p_m
 /* function within state machine */
 extern void bta_gattc_open(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data);
 extern void bta_gattc_open_fail(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data);
-extern void bta_gattc_open_error(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data);
 
 extern void bta_gattc_cancel_open(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data);
 extern void bta_gattc_cancel_open_ok(tBTA_GATTC_CLCB *p_clcb, tBTA_GATTC_DATA *p_data);
@@ -542,7 +545,7 @@ extern void bta_gattc_get_service_with_uuid(UINT16 conn_id, tBT_UUID *svc_uuid,
                                             btgatt_db_element_t **svc_db,
                                             UINT16 *count);
 
-extern void bta_gattc_get_db_with_opration(UINT16 conn_id,
+extern void bta_gattc_get_db_with_operation(UINT16 conn_id,
                                                       bt_gatt_get_db_op_t op,
                                                       UINT16 char_handle,
                                                       tBT_UUID *incl_uuid,

components/bt/host/bluedroid/bta/gatt/include/bta_gatts_int.h

@@ -243,7 +243,6 @@ extern void bta_gatts_close (tBTA_GATTS_CB *p_cb, tBTA_GATTS_DATA *p_msg);
 extern void bta_gatts_send_service_change_indication (tBTA_GATTS_DATA *p_msg);
 extern void bta_gatts_show_local_database (void);
 
-extern BOOLEAN bta_gatts_uuid_compare(tBT_UUID tar, tBT_UUID src);
 extern tBTA_GATTS_RCB *bta_gatts_find_app_rcb_by_app_if(tBTA_GATTS_IF server_if);
 extern UINT8 bta_gatts_find_app_rcb_idx_by_app_if(tBTA_GATTS_CB *p_cb, tBTA_GATTS_IF server_if);
 extern UINT8 bta_gatts_alloc_srvc_cb(tBTA_GATTS_CB *p_cb, UINT8 rcb_idx);