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

Commit 98d45b6a authored by Jakub Pawlowski's avatar Jakub Pawlowski
Browse files

Split sec_state into link-specific variables 

Currently when both LE and Classic links are established at same time,
and both are encrypted at same time, we have a problem. Same variable is
used to mark state as "encrypting" for both links, and it can cause race
conditions.

To fix that, keep security state as separate variable for LE and Classic
link.

This patch also removes security states that were added to address
similar issues around disconnection.

Bug: 343639888
Test: establish LE and Classic connection to dual mode device, verify
      encryption and profiles are connected on both transports
Test: m -j32;
Flag: exempt, can come up with good way to flag this without
      significantly obfruscating the code
Change-Id: I0de9075278010f36a332156a8d984efba08fac00
parent 68247bbc

system/stack/btm/btm_ble_sec.cc

+7 −7
Original line number Diff line number Diff line
@@ -1042,7 +1042,7 @@ void btm_ble_link_sec_check(const RawAddress& bd_addr, tBTM_LE_AUTH_REQ auth_req 
  }



  if (p_dev_rec->sec_rec.is_security_state_encrypting() ||

      p_dev_rec->sec_rec.sec_state == tSECURITY_STATE::AUTHENTICATING) {

      p_dev_rec->sec_rec.le_link == tSECURITY_STATE::AUTHENTICATING) {

    /* race condition: discard the security request while central is encrypting

     * the link */

    *p_sec_req_act = BTM_BLE_SEC_REQ_ACT_DISCARD;
@@ -1145,7 +1145,7 @@ tBTM_STATUS btm_ble_set_encryption(const RawAddress& bd_addr, tBTM_BLE_SEC_ACT s 


      if (SMP_Pair(bd_addr) == SMP_STARTED) {

        cmd = BTM_CMD_STARTED;

        p_rec->sec_rec.sec_state = tSECURITY_STATE::AUTHENTICATING;

        p_rec->sec_rec.le_link = tSECURITY_STATE::AUTHENTICATING;

      }

      break;
@@ -1224,8 +1224,8 @@ tBTM_STATUS btm_ble_start_encrypt(const RawAddress& bda, bool use_stk, Octet16* 
    return BTM_ERR_KEY_MISSING;

  }



  if (p_rec->sec_rec.sec_state == tSECURITY_STATE::IDLE) {

    p_rec->sec_rec.sec_state = tSECURITY_STATE::LE_ENCRYPTING;

  if (p_rec->sec_rec.le_link == tSECURITY_STATE::IDLE) {

    p_rec->sec_rec.le_link = tSECURITY_STATE::ENCRYPTING;

  }



  return BTM_CMD_STARTED;
@@ -1294,7 +1294,7 @@ void btm_ble_link_encrypted(const RawAddress& bd_addr, uint8_t encr_enable) { 
    p_dev_rec->sec_rec.enc_key_size = p_dev_rec->sec_rec.ble_keys.key_size;

  }



  p_dev_rec->sec_rec.sec_state = tSECURITY_STATE::IDLE;

  p_dev_rec->sec_rec.le_link = tSECURITY_STATE::IDLE;

  if (p_dev_rec->sec_rec.p_callback && enc_cback) {

    if (encr_enable) {

      btm_sec_dev_rec_cback_event(p_dev_rec, BTM_SUCCESS, true);
@@ -1556,7 +1556,7 @@ tBTM_STATUS btm_proc_smp_cback(tSMP_EVT event, const RawAddress& bd_addr, 
        }

        btm_sec_cb.pairing_bda = bd_addr;

        if (event != SMP_CONSENT_REQ_EVT) {

          p_dev_rec->sec_rec.sec_state = tSECURITY_STATE::AUTHENTICATING;

          p_dev_rec->sec_rec.le_link = tSECURITY_STATE::AUTHENTICATING;

        }

        btm_sec_cb.pairing_flags |= BTM_PAIR_FLAGS_LE_ACTIVE;

        FALLTHROUGH_INTENDED; /* FALLTHROUGH */
@@ -1608,7 +1608,7 @@ tBTM_STATUS btm_proc_smp_cback(tSMP_EVT event, const RawAddress& bd_addr, 
          }



          if (res == BTM_SUCCESS) {

            p_dev_rec->sec_rec.sec_state = tSECURITY_STATE::IDLE;

            p_dev_rec->sec_rec.le_link = tSECURITY_STATE::IDLE;



            if (p_dev_rec->sec_rec.bond_type != BOND_TYPE_TEMPORARY) {

              // Add all bonded device into resolving list if IRK is available.

system/stack/btm/btm_dev.cc

+2 −1
Original line number Diff line number Diff line
@@ -211,7 +211,8 @@ void BTM_SecClearSecurityFlags(const RawAddress& bd_addr) { 
  }



  p_dev_rec->sec_rec.sec_flags = 0;

  p_dev_rec->sec_rec.sec_state = tSECURITY_STATE::IDLE;

  p_dev_rec->sec_rec.le_link = tSECURITY_STATE::IDLE;

  p_dev_rec->sec_rec.classic_link = tSECURITY_STATE::IDLE;

  p_dev_rec->sm4 = BTM_SM4_UNKNOWN;

}

system/stack/btm/btm_devctl.cc

+2 −1
Original line number Diff line number Diff line
@@ -163,7 +163,8 @@ void BTM_db_reset(void) { 


static bool set_sec_state_idle(void* data, void* /* context */) {

  tBTM_SEC_DEV_REC* p_dev_rec = static_cast<tBTM_SEC_DEV_REC*>(data);

  p_dev_rec->sec_rec.sec_state = tSECURITY_STATE::IDLE;

  p_dev_rec->sec_rec.le_link = tSECURITY_STATE::IDLE;

  p_dev_rec->sec_rec.classic_link = tSECURITY_STATE::IDLE;

  return true;

}

system/stack/btm/btm_sec.cc

+91 −130

File changed.

Preview size limit exceeded, changes collapsed.

system/stack/btm/security_device_record.h

+6 −31

File changed.

Preview size limit exceeded, changes collapsed.