Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6c56039b authored by Jakub Pawlowski's avatar Jakub Pawlowski
Browse files

GATT cache memory corruption fix 

When doing GATT discovery, we interleave adding elements to vector,
and storing pointers to them. This can cause memory corruption, because
vector can reallocate the memory when adding element, leaving the pointers
invalid. To fix that, never store pointers to mutable vector elements.

Bug: 70041392
Test: manual, connected to HID device
Change-Id: I6bb3fec61afa742f446b66e3291c5f326738ee17
parent 19d0aae5

system/bta/gatt/bta_gattc_act.cc

+4 −2
Original line number Diff line number Diff line
@@ -1125,8 +1125,10 @@ bool bta_gattc_process_srvc_chg_ind(uint16_t conn_id, tBTA_GATTC_RCB* p_clrcb, 


  const tBTA_GATTC_CHARACTERISTIC* p_char =

      bta_gattc_get_characteristic_srcb(p_srcb, p_notify->handle);

  if (!p_char || p_char->service->uuid != gattp_uuid ||

      p_char->uuid != srvc_chg_uuid) {

  if (!p_char) return false;

  const tBTA_GATTC_SERVICE* p_svc =

      bta_gattc_get_service_for_handle_srcb(p_srcb, p_char->value_handle);

  if (!p_svc || p_svc->uuid != gattp_uuid || p_char->uuid != srvc_chg_uuid) {

    return false;

  }

system/bta/gatt/bta_gattc_api.cc

+14 −0
Original line number Diff line number Diff line
@@ -309,6 +309,20 @@ const tBTA_GATTC_DESCRIPTOR* BTA_GATTC_GetDescriptor(uint16_t conn_id, 
  return bta_gattc_get_descriptor(conn_id, handle);

}



/* Return characteristic that owns descriptor with handle equal to |handle|, or

 * NULL */

const tBTA_GATTC_CHARACTERISTIC* BTA_GATTC_GetOwningCharacteristic(

    uint16_t conn_id, uint16_t handle) {

  return bta_gattc_get_owning_characteristic(conn_id, handle);

}



/* Return service that owns descriptor or characteristic with handle equal to

 * |handle|, or NULL */

const tBTA_GATTC_SERVICE* BTA_GATTC_GetOwningService(uint16_t conn_id,

                                                     uint16_t handle) {

  return bta_gattc_get_service_for_handle(conn_id, handle);

}



/*******************************************************************************

 *

 * Function         BTA_GATTC_GetGattDb

system/bta/gatt/bta_gattc_cache.cc

+28 −6
Original line number Diff line number Diff line
@@ -203,8 +203,7 @@ static void add_characteristic_to_gatt_db( 
      tBTA_GATTC_CHARACTERISTIC{.declaration_handle = attr_handle,

                                .value_handle = value_handle,

                                .properties = property,

                                .uuid = uuid,

                                .service = service});

                                .uuid = uuid});

  return;

}
@@ -237,9 +236,8 @@ static void add_descriptor_to_gatt_db(std::vector<tBTA_GATTC_SERVICE>& gatt_db, 
    char_node = &(*it);

  }



  char_node->descriptors.emplace_back(tBTA_GATTC_DESCRIPTOR{

      .handle = handle, .uuid = uuid, .characteristic = char_node,

  });

  char_node->descriptors.emplace_back(

      tBTA_GATTC_DESCRIPTOR{.handle = handle, .uuid = uuid});

}



/* Add an attribute into database cache buffer */
@@ -639,7 +637,7 @@ std::vector<tBTA_GATTC_SERVICE>* bta_gattc_get_services(uint16_t conn_id) { 
  return bta_gattc_get_services_srcb(p_srcb);

}



static tBTA_GATTC_SERVICE* bta_gattc_get_service_for_handle_srcb(

tBTA_GATTC_SERVICE* bta_gattc_get_service_for_handle_srcb(

    tBTA_GATTC_SERV* p_srcb, uint16_t handle) {

  std::vector<tBTA_GATTC_SERVICE>* services =

      bta_gattc_get_services_srcb(p_srcb);
@@ -707,6 +705,30 @@ const tBTA_GATTC_DESCRIPTOR* bta_gattc_get_descriptor(uint16_t conn_id, 
  return bta_gattc_get_descriptor_srcb(p_srcb, handle);

}



tBTA_GATTC_CHARACTERISTIC* bta_gattc_get_owning_characteristic_srcb(

    tBTA_GATTC_SERV* p_srcb, uint16_t handle) {

  tBTA_GATTC_SERVICE* service =

      bta_gattc_get_service_for_handle_srcb(p_srcb, handle);



  if (!service) return NULL;



  for (tBTA_GATTC_CHARACTERISTIC& charac : service->characteristics) {

    for (const tBTA_GATTC_DESCRIPTOR& desc : charac.descriptors) {

      if (handle == desc.handle) return &charac;

    }

  }



  return NULL;

}



const tBTA_GATTC_CHARACTERISTIC* bta_gattc_get_owning_characteristic(

    uint16_t conn_id, uint16_t handle) {

  tBTA_GATTC_CLCB* p_clcb = bta_gattc_find_clcb_by_conn_id(conn_id);

  if (!p_clcb) return NULL;



  return bta_gattc_get_owning_characteristic_srcb(p_clcb->p_srcb, handle);

}



/*******************************************************************************

 *

 * Function         bta_gattc_fill_gatt_db_el

system/bta/gatt/bta_gattc_int.h

+4 −0
Original line number Diff line number Diff line
@@ -433,10 +433,14 @@ extern const tBTA_GATTC_SERVICE* bta_gattc_get_service_for_handle( 
    uint16_t conn_id, uint16_t handle);

tBTA_GATTC_CHARACTERISTIC* bta_gattc_get_characteristic_srcb(

    tBTA_GATTC_SERV* p_srcb, uint16_t handle);

extern tBTA_GATTC_SERVICE* bta_gattc_get_service_for_handle_srcb(

    tBTA_GATTC_SERV* p_srcb, uint16_t handle);

extern tBTA_GATTC_CHARACTERISTIC* bta_gattc_get_characteristic(uint16_t conn_id,

                                                               uint16_t handle);

extern const tBTA_GATTC_DESCRIPTOR* bta_gattc_get_descriptor(uint16_t conn_id,

                                                             uint16_t handle);

extern const tBTA_GATTC_CHARACTERISTIC* bta_gattc_get_owning_characteristic(

    uint16_t conn_id, uint16_t handle);

extern void bta_gattc_get_gatt_db(uint16_t conn_id, uint16_t start_handle,

                                  uint16_t end_handle, btgatt_db_element_t** db,

                                  int* count);

system/bta/hh/bta_hh_le.cc

+17 −10
Original line number Diff line number Diff line
@@ -687,12 +687,11 @@ static void write_rpt_ctl_cfg_cb(uint16_t conn_id, tGATT_STATUS status, 
  uint8_t srvc_inst_id, hid_inst_id;



  tBTA_HH_DEV_CB* p_dev_cb = (tBTA_HH_DEV_CB*)data;

  const tBTA_GATTC_DESCRIPTOR* p_desc =

      BTA_GATTC_GetDescriptor(conn_id, handle);



  uint16_t char_uuid = p_desc->characteristic->uuid.As16Bit();

  const tBTA_GATTC_CHARACTERISTIC* characteristic =

      BTA_GATTC_GetOwningCharacteristic(conn_id, handle);

  uint16_t char_uuid = characteristic->uuid.As16Bit();



  srvc_inst_id = p_desc->characteristic->service->handle;

  srvc_inst_id = BTA_GATTC_GetOwningService(conn_id, handle)->handle;

  hid_inst_id = srvc_inst_id;

  switch (char_uuid) {

    case GATT_UUID_BATTERY_LEVEL: /* battery level clt cfg registered */
@@ -1314,10 +1313,15 @@ static void read_report_ref_desc_cb(uint16_t conn_id, tGATT_STATUS status, 
    return;

  }



  const tBTA_GATTC_CHARACTERISTIC* characteristic =

      BTA_GATTC_GetOwningCharacteristic(conn_id, handle);

  const tBTA_GATTC_SERVICE* service =

      BTA_GATTC_GetOwningService(conn_id, characteristic->value_handle);



  tBTA_HH_LE_RPT* p_rpt;

  p_rpt = bta_hh_le_find_report_entry(

      p_dev_cb, p_desc->characteristic->service->handle, GATT_UUID_HID_REPORT,

      p_desc->characteristic->value_handle);

  p_rpt = bta_hh_le_find_report_entry(p_dev_cb, service->handle,

                                      GATT_UUID_HID_REPORT,

                                      characteristic->value_handle);

  if (p_rpt) bta_hh_le_save_report_ref(p_dev_cb, p_rpt, status, value, len);

}
@@ -1728,8 +1732,11 @@ static void read_report_cb(uint16_t conn_id, tGATT_STATUS status, 
  hs_data.handle = p_dev_cb->hid_handle;



  if (status == GATT_SUCCESS) {

    p_rpt = bta_hh_le_find_report_entry(p_dev_cb, p_char->service->handle,

                                        char_uuid, p_char->value_handle);

    const tBTA_GATTC_SERVICE* p_svc =

        BTA_GATTC_GetOwningService(conn_id, p_char->value_handle);



    p_rpt = bta_hh_le_find_report_entry(p_dev_cb, p_svc->handle, char_uuid,

                                        p_char->value_handle);



    if (p_rpt != NULL && len) {

      p_buf = (BT_HDR*)osi_malloc(sizeof(BT_HDR) + len + 1);