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

Commit 1cd1e7d5 authored by Emil Lenngren's avatar Emil Lenngren Committed by Andre Eisenbach
Browse files

Fix BLE white list issues 

Since Bluetooth 4.2 and errata ESR08 there may not be more than one
connection between two LE device addresses. Also the stack assumes
there is at maximum one connection to the same address. This patch
makes sure there are no connected devices in the white list when a
connection attempt is started. Since some (even 4.2) controllers don't
handle this correctly, currently this method is used regardless of
controller in this patch.

When the maximum L2CAP connections were reached and a new connection
was established to a device using auto connect, the stack hung and
would no longer create new connections until Bluetooth was restarted,
since the state change to BLE_CONN_IDLE was forgotten. This patch
resets the state correctly, and also never initiates a connection
unless there is space to avoid connect-disconnect loop.

There were also bugs in the background_connections hash map; memory was
not freed when an element was erased and an incorrect hash function
which used the pointer to a bd addr instead of the bd addr itself which
basically meant that elements were never removed. This patch removes
the dynamic memory allocation and uses a correct hash function.

There was a bug that might lead to that the white list was filled
beyond its maximum, due to the counter was updated on the HCI command
complete event, which might run too late. Now the space is instead
calculated based on what commands have been sent to the controller.

The address type of the address added to the white list must also be
tracked, otherwise it might be updated due to a BLE scan, and later the
wrong address is removed from the white list. This patch fixes this.
(Preferably 49-bit bd addrs should be used as identifier through the
whole stack but we're not there yet.)

There was a queue of size 10 with pending white list operations. That
queue got full if there was initially 10 devices in the white list,
then the 10 devices were removed and immediately after 10 other devices
were added. This patch removes the queue altogether by instead syncing
against the background_connections hash map.

Bug: https://code.google.com/p/android/issues/detail?id=219910
Test: stress-testing with a bunch of BLE devices and inspecting HCI log
Change-Id: I78de654ffbea5f4962a189caf984f7f2934e8fbe
parent 37a5d291

system/stack/btm/btm_ble_bgconn.cc

+120 −83
Original line number Diff line number Diff line
@@ -50,39 +50,63 @@ static void btm_resume_wl_activity(tBTM_BLE_WL_STATE wl_state); 
// TODO: Move all of this to controller/le/background_list or similar?

typedef struct background_connection_t {

  bt_bdaddr_t address;

  uint8_t addr_type;



  bool in_controller_wl;

  uint8_t addr_type_in_wl;



  bool pending_removal;

} background_connection_t;



struct KeyEqual {

  bool operator()(const bt_bdaddr_t* x, const bt_bdaddr_t* y) const {

    return bdaddr_equals(x, y);

struct BgConnHash {

  bool operator()(const bt_bdaddr_t& x) const {

    const uint8_t* a = x.address;

    return a[0] ^ (a[1] << 8) ^ (a[2] << 16) ^ (a[3] << 24) ^ a[4] ^

           (a[5] << 8);

  }

};



static std::unordered_map<bt_bdaddr_t*, background_connection_t*,

                          std::hash<bt_bdaddr_t*>, KeyEqual>

struct BgConnKeyEqual {

  bool operator()(const bt_bdaddr_t& x, const bt_bdaddr_t& y) const {

    return bdaddr_equals(&x, &y);

  }

};



static std::unordered_map<bt_bdaddr_t, background_connection_t, BgConnHash,

                          BgConnKeyEqual>

    background_connections;



static void background_connection_add(bt_bdaddr_t* address) {

static void background_connection_add(uint8_t addr_type, bt_bdaddr_t* address) {

  CHECK(address);



  auto map_iter = background_connections.find(address);

  auto map_iter = background_connections.find(*address);

  if (map_iter == background_connections.end()) {

    background_connection_t* connection =

        (background_connection_t*)osi_calloc(sizeof(background_connection_t));

    connection->address = *address;

    background_connections[&(connection->address)] = connection;

    background_connections[*address] =

        background_connection_t{*address, addr_type, false, 0, false};

  } else {

    background_connection_t* connection = &map_iter->second;

    connection->addr_type = addr_type;

    connection->pending_removal = false;

  }

}



static void background_connection_remove(bt_bdaddr_t* address) {

  background_connections.erase(address);

  auto map_iter = background_connections.find(*address);

  if (map_iter != background_connections.end()) {

    if (map_iter->second.in_controller_wl) {

      map_iter->second.pending_removal = true;

    } else {

      background_connections.erase(map_iter);

    }

  }

}



static void background_connections_clear() { background_connections.clear(); }



static bool background_connections_pending() {

  for (const auto& map_el : background_connections) {

    background_connection_t* connection = map_el.second;

  for (auto& map_el : background_connections) {

    background_connection_t* connection = &map_el.second;

    if (connection->pending_removal) continue;

    const bool connected =

        BTM_IsAclConnectionUp(connection->address.address, BT_TRANSPORT_LE);

    if (!connected) {
@@ -92,6 +116,14 @@ static bool background_connections_pending() { 
  return false;

}



static int background_connections_count() {

  int count = 0;

  for (auto& map_el : background_connections) {

    if (!map_el.second.pending_removal) ++count;

  }

  return count;

}



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

 *

 * Function         btm_update_scanner_filter_policy
@@ -117,6 +149,34 @@ void btm_update_scanner_filter_policy(tBTM_BLE_SFP scan_policy) { 
      p_inq->scan_type, (uint16_t)scan_interval, (uint16_t)scan_window,

      btm_cb.ble_ctr_cb.addr_mgnt_cb.own_addr_type, scan_policy);

}



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

 *

 * Function         btm_ble_bgconn_cancel_if_disconnected

 *

 * Description      If a device has been disconnected, it must be re-added to

 *                  the white list. If needed, this function cancels a pending

 *                  initiate command in order to trigger restart of the initiate

 *                  command which in turn updates the white list.

 *

 * Parameters       bd_addr: updated device

 *

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

void btm_ble_bgconn_cancel_if_disconnected(BD_ADDR bd_addr) {

  if (btm_cb.ble_ctr_cb.conn_state != BLE_BG_CONN) return;



  bt_bdaddr_t addr = *(bt_bdaddr_t*)bd_addr;



  auto map_it = background_connections.find(addr);

  if (map_it != background_connections.end()) {

    background_connection_t* connection = &map_it->second;

    if (!connection->in_controller_wl && !connection->pending_removal &&

        !BTM_IsAclConnectionUp(bd_addr, BT_TRANSPORT_LE)) {

      btm_ble_start_auto_conn(false);

    }

  }

}



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

 *

 * Function         btm_add_dev_to_controller
@@ -132,30 +192,29 @@ bool btm_add_dev_to_controller(bool to_add, BD_ADDR bd_addr) { 
    if (to_add) {

      if (p_dev_rec->ble.ble_addr_type == BLE_ADDR_PUBLIC ||

          !BTM_BLE_IS_RESOLVE_BDA(bd_addr)) {

        btsnd_hcic_ble_add_white_list(p_dev_rec->ble.ble_addr_type, bd_addr);

        background_connection_add(p_dev_rec->ble.ble_addr_type,

                                  (bt_bdaddr_t*)bd_addr);

        started = true;

        p_dev_rec->ble.in_controller_list |= BTM_WHITE_LIST_BIT;

      } else if (memcmp(p_dev_rec->ble.static_addr, bd_addr, BD_ADDR_LEN) !=

                     0 &&

                 memcmp(p_dev_rec->ble.static_addr, dummy_bda, BD_ADDR_LEN) !=

                     0) {

        btsnd_hcic_ble_add_white_list(p_dev_rec->ble.static_addr_type,

                                      p_dev_rec->ble.static_addr);

        background_connection_add(p_dev_rec->ble.static_addr_type,

                                  (bt_bdaddr_t*)p_dev_rec->ble.static_addr);

        started = true;

        p_dev_rec->ble.in_controller_list |= BTM_WHITE_LIST_BIT;

      }

    } else {

      if (p_dev_rec->ble.ble_addr_type == BLE_ADDR_PUBLIC ||

          !BTM_BLE_IS_RESOLVE_BDA(bd_addr)) {

        btsnd_hcic_ble_remove_from_white_list(p_dev_rec->ble.ble_addr_type,

                                              bd_addr);

        background_connection_remove((bt_bdaddr_t*)bd_addr);

        started = true;

      }



      if (memcmp(p_dev_rec->ble.static_addr, dummy_bda, BD_ADDR_LEN) != 0 &&

          memcmp(p_dev_rec->ble.static_addr, bd_addr, BD_ADDR_LEN) != 0) {

        btsnd_hcic_ble_remove_from_white_list(p_dev_rec->ble.static_addr_type,

                                              p_dev_rec->ble.static_addr);

        background_connection_remove((bt_bdaddr_t*)p_dev_rec->ble.static_addr);

        started = true;

      }
@@ -166,9 +225,11 @@ bool btm_add_dev_to_controller(bool to_add, BD_ADDR bd_addr) { 
     */

    uint8_t addr_type =

        BTM_IS_PUBLIC_BDA(bd_addr) ? BLE_ADDR_PUBLIC : BLE_ADDR_RANDOM;

    btsnd_hcic_ble_remove_from_white_list(addr_type, bd_addr);

    started = true;

    if (to_add) btsnd_hcic_ble_add_white_list(addr_type, bd_addr);

    if (to_add)

      background_connection_add(addr_type, (bt_bdaddr_t*)bd_addr);

    else

      background_connection_remove((bt_bdaddr_t*)bd_addr);

  }



  return started;
@@ -181,45 +242,37 @@ bool btm_add_dev_to_controller(bool to_add, BD_ADDR bd_addr) { 
 *                                                                  removing)

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

bool btm_execute_wl_dev_operation(void) {

  tBTM_BLE_WL_OP* p_dev_op = btm_cb.ble_ctr_cb.wl_op_q;

  uint8_t i = 0;

  bool rt = true;



  for (i = 0; i < BTM_BLE_MAX_BG_CONN_DEV_NUM && rt; i++, p_dev_op++) {

    if (p_dev_op->in_use) {

      rt = btm_add_dev_to_controller(p_dev_op->to_add, p_dev_op->bd_addr);

      memset(p_dev_op, 0, sizeof(tBTM_BLE_WL_OP));

  // handle removals first to avoid filling up controller's white list

  for (auto map_it = background_connections.begin();

       map_it != background_connections.end();) {

    background_connection_t* connection = &map_it->second;

    if (connection->pending_removal) {

      btsnd_hcic_ble_remove_from_white_list(connection->addr_type_in_wl,

                                            connection->address.address);

      map_it = background_connections.erase(map_it);

    } else

      break;

      ++map_it;

  }

  return rt;

}

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

 *

 * Function         btm_enq_wl_dev_operation

 *

 * Description      enqueue the pending whitelist device operation (loading or

 *                                                                  removing).

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

void btm_enq_wl_dev_operation(bool to_add, BD_ADDR bd_addr) {

  tBTM_BLE_WL_OP* p_dev_op = btm_cb.ble_ctr_cb.wl_op_q;

  uint8_t i = 0;



  for (i = 0; i < BTM_BLE_MAX_BG_CONN_DEV_NUM; i++, p_dev_op++) {

    if (p_dev_op->in_use && !memcmp(p_dev_op->bd_addr, bd_addr, BD_ADDR_LEN)) {

      p_dev_op->to_add = to_add;

      return;

    } else if (!p_dev_op->in_use)

      break;

  for (auto& map_el : background_connections) {

    background_connection_t* connection = &map_el.second;

    const bool connected =

        BTM_IsAclConnectionUp(connection->address.address, BT_TRANSPORT_LE);

    if (!connection->in_controller_wl && !connected) {

      btsnd_hcic_ble_add_white_list(connection->addr_type,

                                    connection->address.address);

      connection->in_controller_wl = true;

      connection->addr_type_in_wl = connection->addr_type;

    } else if (connection->in_controller_wl && connected) {

      /* Bluetooth Core 4.2 as well as ESR08 disallows more than one

         connection between two LE addresses. Not all controllers handle this

         correctly, therefore we must make sure connected devices are not in

         the white list when bg connection attempt is active. */

      btsnd_hcic_ble_remove_from_white_list(connection->addr_type_in_wl,

                                            connection->address.address);

      connection->in_controller_wl = false;

    }

  if (i != BTM_BLE_MAX_BG_CONN_DEV_NUM) {

    p_dev_op->in_use = true;

    p_dev_op->to_add = to_add;

    memcpy(p_dev_op->bd_addr, bd_addr, BD_ADDR_LEN);

  } else {

    BTM_TRACE_ERROR("max pending WL operation reached, discard");

  }

  return;

  return true;

}



/*******************************************************************************
@@ -233,18 +286,15 @@ void btm_enq_wl_dev_operation(bool to_add, BD_ADDR bd_addr) { 
bool btm_update_dev_to_white_list(bool to_add, BD_ADDR bd_addr) {

  tBTM_BLE_CB* p_cb = &btm_cb.ble_ctr_cb;



  if (to_add && p_cb->white_list_avail_size == 0) {

  if (to_add &&

      background_connections_count() ==

          controller_get_interface()->get_ble_white_list_size()) {

    BTM_TRACE_ERROR("%s Whitelist full, unable to add device", __func__);

    return false;

  }



  if (to_add)

    background_connection_add((bt_bdaddr_t*)bd_addr);

  else

    background_connection_remove((bt_bdaddr_t*)bd_addr);



  btm_suspend_wl_activity(p_cb->wl_state);

  btm_enq_wl_dev_operation(to_add, bd_addr);

  btm_add_dev_to_controller(to_add, bd_addr);

  btm_resume_wl_activity(p_cb->wl_state);

  return true;

}
@@ -271,15 +321,10 @@ void btm_ble_clear_white_list(void) { 
 ******************************************************************************/

void btm_ble_clear_white_list_complete(uint8_t* p_data,

                                       UNUSED_ATTR uint16_t evt_len) {

  tBTM_BLE_CB* p_cb = &btm_cb.ble_ctr_cb;

  uint8_t status;



  BTM_TRACE_EVENT("btm_ble_clear_white_list_complete");

  STREAM_TO_UINT8(status, p_data);



  if (status == HCI_SUCCESS)

    p_cb->white_list_avail_size =

        controller_get_interface()->get_ble_white_list_size();

  BTM_TRACE_EVENT("%s status=%d", __func__, status);

}



/*******************************************************************************
@@ -291,7 +336,6 @@ void btm_ble_clear_white_list_complete(uint8_t* p_data, 
 ******************************************************************************/

void btm_ble_white_list_init(uint8_t white_list_size) {

  BTM_TRACE_DEBUG("%s white_list_size = %d", __func__, white_list_size);

  btm_cb.ble_ctr_cb.white_list_avail_size = white_list_size;

}



/*******************************************************************************
@@ -303,7 +347,6 @@ void btm_ble_white_list_init(uint8_t white_list_size) { 
 ******************************************************************************/

void btm_ble_add_2_white_list_complete(uint8_t status) {

  BTM_TRACE_EVENT("%s status=%d", __func__, status);

  if (status == HCI_SUCCESS) --btm_cb.ble_ctr_cb.white_list_avail_size;

}



/*******************************************************************************
@@ -316,7 +359,6 @@ void btm_ble_add_2_white_list_complete(uint8_t status) { 
void btm_ble_remove_from_white_list_complete(uint8_t* p,

                                             UNUSED_ATTR uint16_t evt_len) {

  BTM_TRACE_EVENT("%s status=%d", __func__, *p);

  if (*p == HCI_SUCCESS) ++btm_cb.ble_ctr_cb.white_list_avail_size;

}



void btm_send_hci_create_connection(
@@ -380,9 +422,11 @@ bool btm_ble_start_auto_conn(bool start) { 
  if (controller_get_interface()->supports_ble_2m_phy()) phy |= PHY_LE_2M;

  if (controller_get_interface()->supports_ble_coded_phy()) phy |= PHY_LE_CODED;



  BTM_TRACE_EVENT("%s start=%d", __func__, start);



  if (start) {

    if (p_cb->conn_state == BLE_CONN_IDLE && background_connections_pending() &&

        btm_ble_topology_check(BTM_BLE_STATE_INIT)) {

        btm_ble_topology_check(BTM_BLE_STATE_INIT) && l2cu_can_allocate_lcb()) {

      p_cb->wl_state |= BTM_BLE_WL_INIT;



      btm_execute_wl_dev_operation();
@@ -470,9 +514,6 @@ static void btm_suspend_wl_activity(tBTM_BLE_WL_STATE wl_state) { 
  if (wl_state & BTM_BLE_WL_INIT) {

    btm_ble_start_auto_conn(false);

  }

  if (wl_state & BTM_BLE_WL_ADV) {

    btm_ble_stop_adv();

  }

}

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

 *
@@ -485,10 +526,6 @@ static void btm_suspend_wl_activity(tBTM_BLE_WL_STATE wl_state) { 
 ******************************************************************************/

static void btm_resume_wl_activity(tBTM_BLE_WL_STATE wl_state) {

  btm_ble_resume_bg_conn();



  if (wl_state & BTM_BLE_WL_ADV) {

    btm_ble_start_adv();

  }

}

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

 *

system/stack/btm/btm_ble_gap.cc

+4 −5
Original line number Diff line number Diff line
@@ -2417,10 +2417,6 @@ tBTM_STATUS btm_ble_start_adv(void) { 
    /* enable resolving list is desired */

    btm_ble_enable_resolving_list_for_platform(BTM_BLE_RL_ADV);

#endif

  if (p_cb->afp != AP_SCAN_CONN_ALL) {

    btm_execute_wl_dev_operation();

    btm_cb.ble_ctr_cb.wl_state |= BTM_BLE_WL_ADV;

  }



  btsnd_hcic_ble_set_adv_enable(BTM_BLE_ADV_ENABLE);

  p_cb->adv_mode = BTM_BLE_ADV_ENABLE;
@@ -2445,7 +2441,6 @@ tBTM_STATUS btm_ble_stop_adv(void) { 


    p_cb->fast_adv_on = false;

    p_cb->adv_mode = BTM_BLE_ADV_DISABLE;

    btm_cb.ble_ctr_cb.wl_state &= ~BTM_BLE_WL_ADV;



    /* clear all adv states */

    btm_ble_clear_topology_mask(BTM_BLE_STATE_ALL_ADV_MASK);
@@ -2675,6 +2670,10 @@ void btm_ble_update_mode_operation(uint8_t link_role, BD_ADDR bd_addr, 
                               btm_cb.ble_ctr_cb.inq_var.connectable_mode);

  }



  /* in case of disconnected, we must cancel bgconn and restart

     in order to add back device to white list in order to reconnect */

  btm_ble_bgconn_cancel_if_disconnected(bd_addr);



  /* when no connection is attempted, and controller is not rejecting last

     request

     due to resource limitation, start next direct connection or background

system/stack/btm/btm_ble_int.h

+1 −0
Original line number Diff line number Diff line
@@ -138,6 +138,7 @@ extern void btm_ble_update_mode_operation(uint8_t link_role, BD_ADDR bda, 
                                          uint8_t status);

extern bool btm_execute_wl_dev_operation(void);

extern void btm_ble_update_link_topology_mask(uint8_t role, bool increase);

extern void btm_ble_bgconn_cancel_if_disconnected(BD_ADDR bd_addr);



/* direct connection utility */

extern bool btm_send_pending_direct_conn(void);

system/stack/btm/btm_ble_int_types.h

+0 −6
Original line number Diff line number Diff line
@@ -174,8 +174,6 @@ typedef struct { 
  alarm_t* refresh_raddr_timer;

} tBTM_LE_RANDOM_CB;



#define BTM_BLE_MAX_BG_CONN_DEV_NUM 10



typedef struct {

  uint16_t min_conn_int;

  uint16_t max_conn_int;
@@ -194,7 +192,6 @@ typedef struct { 
/* white list using state as a bit mask */

#define BTM_BLE_WL_IDLE 0

#define BTM_BLE_WL_INIT 1

#define BTM_BLE_WL_ADV 4

typedef uint8_t tBTM_BLE_WL_STATE;



/* resolving list using state as a bit mask */
@@ -300,7 +297,6 @@ typedef struct { 
  uint32_t scan_win;



  /* white list information */

  uint8_t white_list_avail_size;

  tBTM_BLE_WL_STATE wl_state;



  fixed_queue_t* conn_pending_q;
@@ -321,8 +317,6 @@ typedef struct { 
  tBTM_BLE_RL_STATE rl_state; /* Resolving list state */

#endif



  tBTM_BLE_WL_OP wl_op_q[BTM_BLE_MAX_BG_CONN_DEV_NUM];



  /* current BLE link state */

  tBTM_BLE_STATE_MASK cur_states; /* bit mask of tBTM_BLE_STATE */

  uint8_t link_count[2];          /* total link count master and slave*/

system/stack/l2cap/l2c_ble.cc

+3 −0
Original line number Diff line number Diff line
@@ -292,6 +292,7 @@ void l2cble_scanner_conn_comp(uint16_t handle, BD_ADDR bda, tBLE_ADDR_TYPE type, 
    if (!p_lcb) {

      btm_sec_disconnect(handle, HCI_ERR_NO_CONNECTION);

      L2CAP_TRACE_ERROR("l2cble_scanner_conn_comp - failed to allocate LCB");

      btm_ble_set_conn_st(BLE_CONN_IDLE);

      return;

    } else {

      if (!l2cu_initialize_fixed_ccb(
@@ -300,12 +301,14 @@ void l2cble_scanner_conn_comp(uint16_t handle, BD_ADDR bda, tBLE_ADDR_TYPE type, 
                   .fixed_chnl_opts)) {

        btm_sec_disconnect(handle, HCI_ERR_NO_CONNECTION);

        L2CAP_TRACE_WARNING("l2cble_scanner_conn_comp - LCB but no CCB");

        btm_ble_set_conn_st(BLE_CONN_IDLE);

        return;

      }

    }

  } else if (p_lcb->link_state != LST_CONNECTING) {

    L2CAP_TRACE_ERROR("L2CAP got BLE scanner conn_comp in bad state: %d",

                      p_lcb->link_state);

    btm_ble_set_conn_st(BLE_CONN_IDLE);

    return;

  }

  alarm_cancel(p_lcb->l2c_lcb_timer);