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

Commit e22426a5 authored by Jakub Pawlowski's avatar Jakub Pawlowski
Browse files

Fix race condition around Le Start Encryption handling. 

When sending Le Start Encryption, it's possible that at the same exact
moment the connection was disconnected, and there is an incoming
"Disconnection Complete Event". The controller would send error 0x02
"unknown connection identifier" in such case. This can happen
sporadically when reconnecting device multiple times.

Currently, we don't handle this error code. This lead to the timeout
when waiting for "Encryption Complete", and unbonding of the device.

From now on, if 0x02 error code is returned, we'll cancel current SMP
operation, which is the "LE Start Encryption". This will stop the SMP
timer, and make sure the bond to remote device is not lost.

Test: Manual test reconnecting with HID device multiple time in a row,
causing the link drop right after connection, by removing battery, or
closing the shield box.
Bug: 113652889
Change-Id: I2ff9c13dbc8e7b71505908996e26b89fa1ea6a42
parent 80c707e1

system/bta/dm/bta_dm_act.cc

+14 −2
Original line number Diff line number Diff line
@@ -3845,8 +3845,20 @@ static uint8_t bta_dm_ble_smp_cback(tBTM_LE_EVT event, const RawAddress& bda, 
      if (p_data->complt.reason != 0) {

        sec_event.auth_cmpl.fail_reason =

            BTA_DM_AUTH_CONVERT_SMP_CODE(((uint8_t)p_data->complt.reason));



        if (btm_sec_is_a_bonded_dev(bda) &&

            p_data->complt.reason == SMP_CONN_TOUT) {

          // Bonded device failed to encrypt - to test this remove battery from

          // HID device right after connection, but before encryption is

          // established

          LOG(INFO) << __func__

                    << ": bonded device disconnected when encrypting - no "

                       "reason to unbond";

        } else {

          /* delete this device entry from Sec Dev DB */

          bta_dm_remove_sec_dev_entry(bda);

        }



      } else {

        sec_event.auth_cmpl.success = true;

        if (!p_data->complt.smp_over_br)

system/btif/src/btif_dm.cc

+14 −1
Original line number Diff line number Diff line
@@ -2857,10 +2857,23 @@ static void btif_dm_ble_auth_cmpl_evt(tBTA_DM_AUTH_CMPL* p_auth_cmpl) { 
      case BTA_DM_AUTH_SMP_PAIR_AUTH_FAIL:

      case BTA_DM_AUTH_SMP_CONFIRM_VALUE_FAIL:

      case BTA_DM_AUTH_SMP_UNKNOWN_ERR:

      case BTA_DM_AUTH_SMP_CONN_TOUT:

        btif_dm_remove_ble_bonding_keys();

        status = BT_STATUS_AUTH_FAILURE;

        break;



      case BTA_DM_AUTH_SMP_CONN_TOUT: {

        if (btm_sec_is_a_bonded_dev(bd_addr)) {

          LOG(INFO) << __func__ << " Bonded device addr=" << bd_addr

                    << " timed out - will not remove the keys";

          // Don't send state change to upper layers - otherwise Java think we

          // unbonded, and will disconnect HID profile.

          return;

        }



        btif_dm_remove_ble_bonding_keys();

        status = BT_STATUS_AUTH_FAILURE;

        break;

      }

      case BTA_DM_AUTH_SMP_PAIR_NOT_SUPPORT:

        status = BT_STATUS_AUTH_REJECTED;

        break;

system/stack/btu/btu_hcif.cc

+15 −0
Original line number Diff line number Diff line
@@ -53,6 +53,7 @@ using tracked_objects::Location; 


extern void btm_process_cancel_complete(uint8_t status, uint8_t mode);

extern void btm_ble_test_command_complete(uint8_t* p);

extern void smp_cancel_start_encryption_attempt();



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

/*            L O C A L    F U N C T I O N     P R O T O T Y P E S            */
@@ -725,6 +726,11 @@ static void btu_hcif_encryption_change_evt(uint8_t* p) { 
  STREAM_TO_UINT16(handle, p);

  STREAM_TO_UINT8(encr_enable, p);



  if (status == HCI_ERR_CONNECTION_TOUT) {

    smp_cancel_start_encryption_attempt();

    return;

  }



  btm_acl_encrypt_change(handle, status, encr_enable);

  btm_sec_encrypt_change(handle, status, encr_enable);

}
@@ -1130,6 +1136,15 @@ static void btu_hcif_hdl_command_status(uint16_t opcode, uint8_t status, 
            btm_sec_auth_complete(BTM_INVALID_HCI_HANDLE, status);

            break;



          case HCI_BLE_START_ENC:

            // Race condition: disconnection happened right before we send

            // "LE Encrypt", controller responds with no connection, we should

            // cancel the encryption attempt, rather than unpair the device.

            if (status == HCI_ERR_NO_CONNECTION) {

              smp_cancel_start_encryption_attempt();

            }

            break;



          case HCI_SET_CONN_ENCRYPTION:

            /* Device refused to start encryption.  That should be treated as

             * encryption failure. */

system/stack/smp/smp_act.cc

+5 −0
Original line number Diff line number Diff line
@@ -1869,6 +1869,11 @@ void smp_link_encrypted(const RawAddress& bda, uint8_t encr_enable) { 
  }

}



void smp_cancel_start_encryption_attempt() {

  SMP_TRACE_ERROR("%s: Encryption request cancelled", __func__);

  smp_sm_event(&smp_cb, SMP_DISCARD_SEC_REQ_EVT, NULL);

}



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

 *

 * Function         smp_proc_ltk_request