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

Commit b4668b6a authored by Jakub Pawlowski's avatar Jakub Pawlowski Committed by Stanley Tng
Browse files

Fix race condition during LE device remove bond. 

BTA_GATTC_CancelOpen doesn't execute immediately, but rather schedule
operation for execution later. During bond removal, we must make sure
that device is removed from white list before it's removed from
resolving list. Calling GATT_CancelConnect directly fixes this issue.

Additionally, after checking the logic, unconditional remove should
remove all connection attempts, including direct connections, rather
than just background connections.

Bug: 122918938
Bug: 112827989
Test: re-bond hearing aids
Change-Id: I9d861b7d83ea4da9c2a278eea2e34f2b7aca2417
parent c49c9b1f

system/bta/dm/bta_dm_act.cc

+13 −0
Original line number Diff line number Diff line
@@ -48,6 +48,7 @@ 
#include "osi/include/osi.h"

#include "sdp_api.h"

#include "stack/gatt/connection_manager.h"

#include "stack/include/gatt_api.h"

#include "utl.h"



#if (GAP_INCLUDED == TRUE)
@@ -644,6 +645,10 @@ void bta_dm_remove_device(const RawAddress& bd_addr) { 
      auto& peer_device = bta_dm_cb.device_list.peer_device[i];

      if (peer_device.peer_bdaddr == bd_addr) {

        peer_device.conn_state = BTA_DM_UNPAIRING;



        /* Make sure device is not in white list before we disconnect */

        GATT_CancelConnect(0, bd_addr, false);



        btm_remove_acl(bd_addr, peer_device.transport);

        APPL_TRACE_DEBUG("%s: transport: %d", __func__, peer_device.transport);
@@ -680,6 +685,10 @@ void bta_dm_remove_device(const RawAddress& bd_addr) { 
      auto& peer_device = bta_dm_cb.device_list.peer_device[i];

      if (peer_device.peer_bdaddr == other_address) {

        peer_device.conn_state = BTA_DM_UNPAIRING;



        /* Make sure device is not in white list before we disconnect */

        GATT_CancelConnect(0, bd_addr, false);



        btm_remove_acl(other_address, peer_device.transport);

        break;

      }
@@ -761,6 +770,10 @@ void bta_dm_close_acl(const RawAddress& bd_addr, bool remove_dev, 
    } else {

      APPL_TRACE_ERROR("unknown device, remove ACL failed");

    }



    /* Make sure device is not in white list before we disconnect */

    GATT_CancelConnect(0, bd_addr, false);



    /* Disconnect the ACL link */

    btm_remove_acl(bd_addr, transport);

  }

system/stack/gatt/connection_manager.cc

+2 −2
Original line number Diff line number Diff line
@@ -115,9 +115,9 @@ bool background_connect_add(uint8_t app_id, const RawAddress& address) { 
  return true;

}



/** Removes all registrations for background connection for given device.

/** Removes all registrations for connection for given device.

 * Returns true if anything was removed, false otherwise */

bool background_connect_remove_unconditional(const RawAddress& address) {

bool remove_unconditional(const RawAddress& address) {

  auto it = bgconn_dev.find(address);

  if (it == bgconn_dev.end()) return false;

system/stack/gatt/connection_manager.h

+1 −1
Original line number Diff line number Diff line
@@ -40,7 +40,7 @@ using tAPP_ID = uint8_t; 
extern bool background_connect_add(tAPP_ID app_id, const RawAddress& address);

extern bool background_connect_remove(tAPP_ID app_id,

                                      const RawAddress& address);

extern bool background_connect_remove_unconditional(const RawAddress& address);

extern bool remove_unconditional(const RawAddress& address);



extern void reset(bool after_reset);

system/stack/gatt/gatt_api.cc

+5 −13
Original line number Diff line number Diff line
@@ -1178,28 +1178,20 @@ bool GATT_CancelConnect(tGATT_IF gatt_if, const RawAddress& bd_addr, 


  VLOG(1) << " unconditional";



  if (is_direct) {

  /* only LE connection can be cancelled */

  tGATT_TCB* p_tcb = gatt_find_tcb_by_addr(bd_addr, BT_TRANSPORT_LE);

    if (!p_tcb || p_tcb->app_hold_link.empty()) {

      LOG(ERROR) << __func__ << " no app found";

      return false;

    }



  if (p_tcb && !p_tcb->app_hold_link.empty()) {

    for (auto it = p_tcb->app_hold_link.begin();

         it != p_tcb->app_hold_link.end();) {

      auto next = std::next(it);

      // gatt_cancel_open modifies the app_hold_link.

      if (!gatt_cancel_open(*it, bd_addr)) return false;

      gatt_cancel_open(*it, bd_addr);



      it = next;

    }



    return true;

  }



  // is not direct

  if (!connection_manager::background_connect_remove_unconditional(bd_addr)) {

  if (!connection_manager::remove_unconditional(bd_addr)) {

    LOG(ERROR)

        << __func__

        << ": no app associated with the bg device for unconditional removal";