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

Commit ab002fc8 authored by Łukasz Rymanowski's avatar Łukasz Rymanowski
Browse files

gatt: Fix crash in gatt 

This patch, makes sure to invalide or erase outstanding p_clbc's from
the p_tcb queues (both cl_cmd_q and pending_enc_clcb) if the p_clbc is
removed.
Othwerwise, we could endup proceding gatt response with the invalid
p_clbc which could lead to crash.

Bug: 258984004
Bug: 259351506
Tag: #feature
Test: manual testing

Merged-In: I3bc96253b852e86d439f1189fbc971e8ca47c5b2
Change-Id: I3bc96253b852e86d439f1189fbc971e8ca47c5b2
(cherry picked from commit f6e6a9a8)
parent f7cfc29e

system/stack/gatt/gatt_api.cc

+4 −2
Original line number Diff line number Diff line
@@ -894,7 +894,8 @@ tGATT_STATUS GATTC_Read(uint16_t conn_id, tGATT_READ_TYPE type, 
  }



  /* start security check */

  if (gatt_security_check_start(p_clcb)) p_tcb->pending_enc_clcb.push(p_clcb);

  if (gatt_security_check_start(p_clcb))

    p_tcb->pending_enc_clcb.push_back(p_clcb);

  return GATT_SUCCESS;

}
@@ -943,7 +944,8 @@ tGATT_STATUS GATTC_Write(uint16_t conn_id, tGATT_WRITE_TYPE type, 
    p->offset = 0;

  }



  if (gatt_security_check_start(p_clcb)) p_tcb->pending_enc_clcb.push(p_clcb);

  if (gatt_security_check_start(p_clcb))

    p_tcb->pending_enc_clcb.push_back(p_clcb);

  return GATT_SUCCESS;

}

system/stack/gatt/gatt_auth.cc

+8 −7
Original line number Diff line number Diff line
@@ -174,7 +174,7 @@ void gatt_enc_cmpl_cback(const RawAddress* bd_addr, tBT_TRANSPORT transport, 
  }



  tGATT_CLCB* p_clcb = p_tcb->pending_enc_clcb.front();

  p_tcb->pending_enc_clcb.pop();

  p_tcb->pending_enc_clcb.pop_front();



  bool status = false;

  if (result == BTM_SUCCESS) {
@@ -188,11 +188,11 @@ void gatt_enc_cmpl_cback(const RawAddress* bd_addr, tBT_TRANSPORT transport, 
  gatt_sec_check_complete(status, p_clcb, p_tcb->sec_act);



  /* start all other pending operation in queue */

  std::queue<tGATT_CLCB*> new_pending_clcbs;

  std::deque<tGATT_CLCB*> new_pending_clcbs;

  while (!p_tcb->pending_enc_clcb.empty()) {

    tGATT_CLCB* p_clcb = p_tcb->pending_enc_clcb.front();

    p_tcb->pending_enc_clcb.pop();

    if (gatt_security_check_start(p_clcb)) new_pending_clcbs.push(p_clcb);

    p_tcb->pending_enc_clcb.pop_front();

    if (gatt_security_check_start(p_clcb)) new_pending_clcbs.push_back(p_clcb);

  }

  p_tcb->pending_enc_clcb = new_pending_clcbs;

}
@@ -225,11 +225,12 @@ void gatt_notify_enc_cmpl(const RawAddress& bd_addr) { 
  if (gatt_get_sec_act(p_tcb) == GATT_SEC_ENC_PENDING) {

    gatt_set_sec_act(p_tcb, GATT_SEC_NONE);



    std::queue<tGATT_CLCB*> new_pending_clcbs;

    std::deque<tGATT_CLCB*> new_pending_clcbs;

    while (!p_tcb->pending_enc_clcb.empty()) {

      tGATT_CLCB* p_clcb = p_tcb->pending_enc_clcb.front();

      p_tcb->pending_enc_clcb.pop();

      if (gatt_security_check_start(p_clcb)) new_pending_clcbs.push(p_clcb);

      p_tcb->pending_enc_clcb.pop_front();

      if (gatt_security_check_start(p_clcb))

        new_pending_clcbs.push_back(p_clcb);

    }

    p_tcb->pending_enc_clcb = new_pending_clcbs;

  }

system/stack/gatt/gatt_int.h

+1 −1
Original line number Diff line number Diff line
@@ -304,7 +304,7 @@ typedef struct { 
} tGATT_SRV_LIST_ELEM;



typedef struct {

  std::queue<tGATT_CLCB*> pending_enc_clcb; /* pending encryption channel q */

  std::deque<tGATT_CLCB*> pending_enc_clcb; /* pending encryption channel q */

  tGATT_SEC_ACTION sec_act;

  RawAddress peer_bda;

  tBT_TRANSPORT transport;

system/stack/gatt/gatt_main.cc

+1 −1
Original line number Diff line number Diff line
@@ -159,7 +159,7 @@ void gatt_free(void) { 
  fixed_queue_free(gatt_cb.srv_chg_clt_q, NULL);

  gatt_cb.srv_chg_clt_q = NULL;

  for (i = 0; i < GATT_MAX_PHY_CHANNEL; i++) {

    gatt_cb.tcb[i].pending_enc_clcb = std::queue<tGATT_CLCB*>();

    gatt_cb.tcb[i].pending_enc_clcb = std::deque<tGATT_CLCB*>();



    fixed_queue_free(gatt_cb.tcb[i].pending_ind_q, NULL);

    gatt_cb.tcb[i].pending_ind_q = NULL;

system/stack/gatt/gatt_utils.cc

+17 −0
Original line number Diff line number Diff line
@@ -1138,6 +1138,7 @@ uint16_t gatt_tcb_get_payload_size_rx(tGATT_TCB& tcb, uint16_t cid) { 
void gatt_clcb_dealloc(tGATT_CLCB* p_clcb) {

  if (p_clcb) {

    alarm_free(p_clcb->gatt_rsp_timer_ent);

    gatt_clcb_invalidate(p_clcb->p_tcb, p_clcb);

    for (auto clcb_it = gatt_cb.clcb_queue.begin();

         clcb_it != gatt_cb.clcb_queue.end(); clcb_it++) {

      if (&(*clcb_it) == p_clcb) {
@@ -1161,6 +1162,17 @@ void gatt_clcb_invalidate(tGATT_TCB* p_tcb, const tGATT_CLCB* p_clcb) { 
  std::deque<tGATT_CMD_Q>* cl_cmd_q_p;

  uint16_t cid = p_clcb->cid;



  if (!p_tcb->pending_enc_clcb.empty()) {

    auto iter = std::find_if(p_tcb->pending_enc_clcb.begin(),

                             p_tcb->pending_enc_clcb.end(),

                             [p_clcb](auto& el) { return el == p_clcb; });

    if (iter != p_tcb->pending_enc_clcb.end()) {

      p_tcb->pending_enc_clcb.erase(iter);

      LOG_WARN("Removing clcb (%p) for conn id=0x%04x from pending_enc_clcb",

               p_clcb, p_clcb->conn_id);

    }

  }



  if (cid == p_tcb->att_lcid) {

    cl_cmd_q_p = &p_tcb->cl_cmd_q;

  } else {
@@ -1186,10 +1198,15 @@ void gatt_clcb_invalidate(tGATT_TCB* p_tcb, const tGATT_CLCB* p_clcb) { 
  if (iter->to_send) {

    /* If command was not send, just remove the entire element */

    cl_cmd_q_p->erase(iter);

    LOG_WARN("Removing scheduled clcb (%p) for conn_id=0x%04x", p_clcb,

             p_clcb->conn_id);

  } else {

    /* If command has been sent, just invalidate p_clcb pointer for proper

     * response handling */

    iter->p_clcb = NULL;

    LOG_WARN(

        "Invalidating clcb (%p) for already sent request on conn_id=0x%04x",

        p_clcb, p_clcb->conn_id);

  }

}

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