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

Commit c40840ee authored by Jack He's avatar Jack He
Browse files

BtifConfig: Prevent unprotected access to btif config cache 

* btif_config_sections() used to allow unprotected access to internals
  of btif config cache that is normally protected by a mutex
* Changing that function to btif_config_get_paired_devices() as all
  users of that API only require paired devices
* Instead of returning a reference to the underlying list, the new API
  returns a copy of all paired devices, thus preventing any race
  conditon from happening when iterating through this vector

Bug: 67595284
Test: manual pairing test
Tag: #stability
Change-Id: If20db2c94cebb538f7c8d5c7c4b4c463dcfc1988
Merged-In: If20db2c94cebb538f7c8d5c7c4b4c463dcfc1988
parent 1a75b090

system/btif/include/btif_config.h

+1 −1
Original line number Diff line number Diff line
@@ -64,7 +64,7 @@ bool btif_config_remove(const std::string& section, const std::string& key); 
size_t btif_config_get_bin_length(const std::string& section,

                                  const std::string& key);



const std::list<section_t>& btif_config_sections();

std::vector<RawAddress> btif_config_get_paired_devices();



void btif_config_save(void);

void btif_config_flush(void);

system/btif/include/btif_config_cache.h

+1 −1
Original line number Diff line number Diff line
@@ -31,7 +31,7 @@ class BtifConfigCache { 


  void Clear();

  void Init(std::unique_ptr<config_t> source);

  const std::list<section_t>& GetPersistentSections();

  std::vector<std::string> GetPersistentSectionNames();

  config_t PersistentSectionCopy();

  bool HasSection(const std::string& section_name);

  bool HasUnpairedSection(const std::string& section_name);

system/btif/include/btif_storage.h

+3 −3
Original line number Diff line number Diff line
@@ -219,7 +219,7 @@ bt_status_t btif_storage_load_bonded_hid_info(void); 
 *                  BT_STATUS_FAIL otherwise

 *

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

bt_status_t btif_storage_remove_hid_info(RawAddress* remote_bd_addr);

bt_status_t btif_storage_remove_hid_info(const RawAddress& remote_bd_addr);



/** Loads information about bonded hearing aid devices */

void btif_storage_load_bonded_hearing_aids();
@@ -255,7 +255,7 @@ bt_status_t btif_storage_add_ble_bonding_key(RawAddress* remote_bd_addr, 
                                             const uint8_t* key,

                                             uint8_t key_type,

                                             uint8_t key_length);

bt_status_t btif_storage_get_ble_bonding_key(RawAddress* remote_bd_addr,

bt_status_t btif_storage_get_ble_bonding_key(const RawAddress& remote_bd_addr,

                                             uint8_t key_type,

                                             uint8_t* key_value,

                                             int key_length);
@@ -294,7 +294,7 @@ bt_status_t btif_storage_load_hidd(void); 
 *

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



bt_status_t btif_storage_set_hidd(RawAddress* remote_bd_addr);

bt_status_t btif_storage_set_hidd(const RawAddress& remote_bd_addr);



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

 *

system/btif/src/btif_config.cc

+23 −13
Original line number Diff line number Diff line
@@ -218,18 +218,14 @@ static void init_metric_id_allocator() { 
  // version of android without a metric id.

  std::vector<RawAddress> addresses_without_id;



  for (auto& section : btif_config_sections()) {

    auto& section_name = section.name;

    RawAddress mac_address;

    if (!RawAddress::FromString(section_name, mac_address)) {

      continue;

    }

  for (const auto& mac_address : btif_config_get_paired_devices()) {

    auto addr_str = mac_address.ToString();

    // if the section name is a mac address

    bool is_valid_id_found = false;

    if (btif_config_exist(section_name, BT_CONFIG_METRICS_ID_KEY)) {

    if (btif_config_exist(addr_str, BT_CONFIG_METRICS_ID_KEY)) {

      // there is one metric id under this mac_address

      int id = 0;

      btif_config_get_int(section_name, BT_CONFIG_METRICS_ID_KEY, &id);

      btif_config_get_int(addr_str, BT_CONFIG_METRICS_ID_KEY, &id);

      if (MetricIdAllocator::IsValidId(id)) {

        paired_device_map[mac_address] = id;

        is_valid_id_found = true;
@@ -581,8 +577,23 @@ bool btif_config_set_bin(const std::string& section, const std::string& key, 
  return true;

}



const std::list<section_t>& btif_config_sections() {

  return btif_config_cache.GetPersistentSections();

std::vector<RawAddress> btif_config_get_paired_devices() {

  std::vector<std::string> names;

  {

    std::unique_lock<std::recursive_mutex> lock(config_lock);

    names = btif_config_cache.GetPersistentSectionNames();

  }

  std::vector<RawAddress> result;

  result.reserve(names.size());

  for (const auto& name : names) {

    RawAddress addr = {};

    if (!RawAddress::FromString(name, addr)) {

      LOG(WARNING) << __func__ << ": " << name << " is not a valid address";

      continue;

    }

    result.emplace_back(addr);

  }

  return result;

}



bool btif_config_remove(const std::string& section, const std::string& key) {
@@ -672,9 +683,8 @@ void btif_debug_config_dump(int fd) { 
  if (!file_source) {

    file_source.emplace("Original");

  }



  dprintf(fd, "  Devices loaded: %zu\n",

          btif_config_cache.GetPersistentSections().size());

  auto devices = btif_config_cache.GetPersistentSectionNames();

  dprintf(fd, "  Devices loaded: %zu\n", devices.size());

  dprintf(fd, "  File created/tagged: %s\n", btif_config_time_created);

  dprintf(fd, "  File source: %s\n", file_source->c_str());

}

system/btif/src/btif_config_cache.cc

+9 −4
Original line number Diff line number Diff line
@@ -148,16 +148,21 @@ bool BtifConfigCache::RemoveKey(const std::string& section_name, 
  }

}



std::vector<std::string> BtifConfigCache::GetPersistentSectionNames() {

  std::vector<std::string> result;

  result.reserve(paired_devices_list_.sections.size());

  for (const auto& section : paired_devices_list_.sections) {

    result.emplace_back(section.name);

  }

  return result;

}



/* clone persistent sections (Local Adapter sections, remote paired devices

 * section,..) */

config_t BtifConfigCache::PersistentSectionCopy() {

  return paired_devices_list_;

}



const std::list<section_t>& BtifConfigCache::GetPersistentSections() {

  return paired_devices_list_.sections;

}



void BtifConfigCache::SetString(std::string section_name, std::string key,

                                std::string value) {

  if (trim_new_line(section_name) || trim_new_line(key) ||