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

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

Memory safe events in bta_dm_disc 

This patch explores possibility of replacing malloced memory with
unique_ptr to variant for messages passed in BTA layer.

Such container would be memory safe, and free of any potential leaks.
In such containers, one could pass types like std::vector, and std::list

Bug: 330675788
Flag: exempt, infrastructure refactor that can't be easily isolated
Test: mma -j32
Change-Id: Ia7ea581e8a0d17ea4605d50f303803380b6d6625
parent 1fffd987

system/bta/dm/bta_dm_disc.cc

+193 −204

File changed.

Preview size limit exceeded, changes collapsed.

system/bta/dm/bta_dm_disc_int.h

+4 −27
Original line number Diff line number Diff line
@@ -65,25 +65,21 @@ inline std::string bta_dm_event_text(const tBTA_DM_EVT& event) { 


/* data type for BTA_DM_API_SEARCH_EVT */

typedef struct {

  BT_HDR_RIGID hdr;

  tBTA_SERVICE_MASK services;

  tBTA_DM_SEARCH_CBACK* p_cback;

} tBTA_DM_API_SEARCH;



/* data type for BTA_DM_API_DISCOVER_EVT */

typedef struct {

  BT_HDR_RIGID hdr;

  RawAddress bd_addr;

  service_discovery_callbacks cbacks;

  tBT_TRANSPORT transport;

} tBTA_DM_API_DISCOVER;



typedef struct {

  BT_HDR_RIGID hdr;

} tBTA_DM_API_DISCOVERY_CANCEL;



typedef struct {

  BT_HDR_RIGID hdr;

  RawAddress bd_addr;

  BD_NAME bd_name; /* Name of peer device. */

  tHCI_STATUS hci_status;
@@ -91,45 +87,26 @@ typedef struct { 


/* data type for tBTA_DM_DISC_RESULT */

typedef struct {

  BT_HDR_RIGID hdr;

  tBTA_DM_SEARCH result;

} tBTA_DM_DISC_RESULT;



/* data type for BTA_DM_INQUIRY_CMPL_EVT */

typedef struct {

  BT_HDR_RIGID hdr;

  uint8_t num;

} tBTA_DM_INQUIRY_CMPL;



/* data type for BTA_DM_SDP_RESULT_EVT */

typedef struct {

  BT_HDR_RIGID hdr;

  tSDP_RESULT sdp_result;

} tBTA_DM_SDP_RESULT;



typedef struct {

  BT_HDR_RIGID hdr;

  bool enable;

} tBTA_DM_API_BLE_FEATURE;



/* union of all data types */

typedef union {

  /* GKI event buffer header */

  BT_HDR_RIGID hdr;



  tBTA_DM_API_SEARCH search;



  tBTA_DM_API_DISCOVER discover;



  tBTA_DM_REMOTE_NAME remote_name_msg;



  tBTA_DM_DISC_RESULT disc_result;



  tBTA_DM_INQUIRY_CMPL inq_cmpl;



  tBTA_DM_SDP_RESULT sdp_event;



} tBTA_DM_MSG;

using tBTA_DM_MSG =

    std::variant<tBTA_DM_API_SEARCH, tBTA_DM_API_DISCOVER, tBTA_DM_REMOTE_NAME,

                 tBTA_DM_DISC_RESULT, tBTA_DM_INQUIRY_CMPL, tBTA_DM_SDP_RESULT>;



/* DM search state */

typedef enum {
@@ -167,7 +144,7 @@ typedef struct { 
  BD_NAME peer_name;

  alarm_t* search_timer;

  uint8_t service_index;

  tBTA_DM_MSG* p_pending_search;

  std::unique_ptr<tBTA_DM_MSG> p_pending_search;

  fixed_queue_t* pending_discovery_queue;

  bool wait_disc;

  bool sdp_results;

system/bta/test/bta_disc_test.cc

+14 −31
Original line number Diff line number Diff line
@@ -39,9 +39,7 @@ namespace testing { 
void bta_dm_disc_init_search_cb(tBTA_DM_SEARCH_CB& bta_dm_search_cb);

bool bta_dm_read_remote_device_name(const RawAddress& bd_addr,

                                    tBT_TRANSPORT transport);

const tBTA_DM_SEARCH_CB& bta_dm_disc_search_cb();

tBTA_DM_SEARCH_CB bta_dm_disc_get_search_cb();

void bta_dm_disc_search_cb(const tBTA_DM_SEARCH_CB& search_cb);

tBTA_DM_SEARCH_CB& bta_dm_disc_search_cb();

void bta_dm_discover_next_device();

void bta_dm_execute_queued_request();

void bta_dm_find_services(const RawAddress& bd_addr);
@@ -53,9 +51,8 @@ void bta_dm_observe_results_cb(tBTM_INQ_RESULTS* p_inq, const uint8_t* p_eir, 
void bta_dm_opportunistic_observe_results_cb(tBTM_INQ_RESULTS* p_inq,

                                             const uint8_t* p_eir,

                                             uint16_t eir_len);

void bta_dm_queue_search(tBTA_DM_MSG* p_data);

void bta_dm_sdp_result(tBTA_DM_MSG* p_data);

void bta_dm_search_result(tBTA_DM_MSG* p_data);

void bta_dm_queue_search(tBTA_DM_API_SEARCH& search);

void bta_dm_search_result(tBTA_DM_DISC_RESULT& p_data);

void bta_dm_search_timer_cback(void* data);

void bta_dm_service_search_remname_cback(const RawAddress& bd_addr,

                                         DEV_CLASS dc, BD_NAME bd_name);
@@ -156,10 +153,8 @@ TEST_F(BtaInitializedTest, bta_dm_opportunistic_observe_results_cb) { 
}



TEST_F(BtaInitializedTest, bta_dm_queue_search) {

  tBTA_DM_MSG msg = {

      .search = {},

  };

  bluetooth::legacy::testing::bta_dm_queue_search(&msg);

  tBTA_DM_API_SEARCH search{};

  bluetooth::legacy::testing::bta_dm_queue_search(search);



  // Release the queued search

  bta_dm_disc_stop();
@@ -171,10 +166,8 @@ TEST_F(BtaInitializedTest, bta_dm_read_remote_device_name) { 
}



TEST_F(BtaInitializedTest, bta_dm_search_result) {

  tBTA_DM_MSG msg = {

      .disc_result = {},

  };

  bluetooth::legacy::testing::bta_dm_search_result(&msg);

  tBTA_DM_DISC_RESULT disc_result = {};

  bluetooth::legacy::testing::bta_dm_search_result(disc_result);

}



TEST_F(BtaInitializedTest, bta_dm_search_timer_cback) {
@@ -185,10 +178,9 @@ TEST_F(BtaInitializedTest, bta_dm_search_timer_cback) { 
TEST_F(BtaInitializedTest, bta_dm_service_search_remname_cback__expected_name) {

  DEV_CLASS dc;

  BD_NAME bd_name;

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.peer_bdaddr = kRawAddress,

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);

  bluetooth::legacy::testing::bta_dm_service_search_remname_cback(kRawAddress,

                                                                  dc, bd_name);

}
@@ -197,10 +189,9 @@ TEST_F(BtaInitializedTest, 
       bta_dm_service_search_remname_cback__unexpected_name) {

  DEV_CLASS dc;

  BD_NAME bd_name;

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.peer_bdaddr = RawAddress::kAny;

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);

  bluetooth::legacy::testing::bta_dm_service_search_remname_cback(kRawAddress,

                                                                  dc, bd_name);

}
@@ -258,20 +249,12 @@ TEST_F(BtaInitializedTest, init_bta_dm_search_cb__conn_id) { 
  constexpr uint16_t kConnId = 123;



  // Set the global search block target field to some non-reset value

  tBTA_DM_SEARCH_CB search_cb = {};

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.conn_id = kConnId;

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);

  // Get the global search block and ensure it is still intact

  search_cb = bluetooth::legacy::testing::bta_dm_disc_search_cb();

  ASSERT_EQ(kConnId, search_cb.conn_id);



  // Initialize *this* global search block

  bluetooth::legacy::testing::bta_dm_disc_init_search_cb(search_cb);



  // Verify *this* global search block field reset value is correct

  // Verify global search block field reset value is correct

  ASSERT_EQ(search_cb.conn_id, GATT_INVALID_CONN_ID);



  // Get the global search block and ensure it is still intact

  search_cb = bluetooth::legacy::testing::bta_dm_disc_search_cb();

  ASSERT_EQ(kConnId, search_cb.conn_id);

}

system/bta/test/bta_dm_test.cc

+26 −39
Original line number Diff line number Diff line
@@ -59,13 +59,10 @@ constexpr char kRemoteName[] = "TheRemoteName"; 


namespace bluetooth::legacy::testing {



const tBTA_DM_SEARCH_CB& bta_dm_disc_search_cb();

tBTA_DM_SEARCH_CB bta_dm_disc_get_search_cb();

tBTA_DM_SEARCH_CB& bta_dm_disc_search_cb();

void bta_dm_deinit_cb();

void bta_dm_disc_search_cb(const tBTA_DM_SEARCH_CB& search_cb);

void bta_dm_init_cb();

void bta_dm_remote_name_cmpl(const tBTA_DM_MSG* p_data);

void bta_dm_sdp_result(tBTA_DM_MSG* p_data);

void bta_dm_remote_name_cmpl(const tBTA_DM_REMOTE_NAME& remote_name_msg);



}  // namespace bluetooth::legacy::testing
@@ -360,10 +357,10 @@ TEST_F(BtaDmTest, bta_dm_state_text) { 
}



TEST_F(BtaDmTest, bta_dm_remname_cback__typical) {

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  search_cb.peer_bdaddr = kRawAddress, search_cb.name_discover_done = false,

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.peer_bdaddr = kRawAddress;

  search_cb.name_discover_done = false;



  tBTM_REMOTE_DEV_NAME name = {

      .status = BTM_SUCCESS,
@@ -388,13 +385,12 @@ TEST_F(BtaDmTest, bta_dm_remname_cback__typical) { 
}



TEST_F(BtaDmTest, bta_dm_remname_cback__wrong_address) {

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.p_device_search_cback = nullptr;

  search_cb.service_search_cbacks = {};

  search_cb.peer_bdaddr = kRawAddress;

  search_cb.name_discover_done = false;

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);



  tBTM_REMOTE_DEV_NAME name = {

      .status = BTM_SUCCESS,
@@ -417,8 +413,8 @@ TEST_F(BtaDmTest, bta_dm_remname_cback__wrong_address) { 
}



TEST_F(BtaDmTest, bta_dm_remname_cback__HCI_ERR_CONNECTION_EXISTS) {

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.peer_bdaddr = kRawAddress;

  search_cb.name_discover_done = false;
@@ -445,10 +441,9 @@ TEST_F(BtaDmTest, bta_dm_remname_cback__HCI_ERR_CONNECTION_EXISTS) { 
}



TEST_F(BtaDmTest, bta_dm_determine_discovery_transport__BT_TRANSPORT_BR_EDR) {

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.transport = BT_TRANSPORT_BR_EDR;

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);



  ASSERT_EQ(BT_TRANSPORT_BR_EDR,

            bluetooth::legacy::testing::bta_dm_determine_discovery_transport(
@@ -456,10 +451,9 @@ TEST_F(BtaDmTest, bta_dm_determine_discovery_transport__BT_TRANSPORT_BR_EDR) { 
}



TEST_F(BtaDmTest, bta_dm_determine_discovery_transport__BT_TRANSPORT_LE) {

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.transport = BT_TRANSPORT_LE;

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);



  ASSERT_EQ(BT_TRANSPORT_LE,

            bluetooth::legacy::testing::bta_dm_determine_discovery_transport(
@@ -468,10 +462,9 @@ TEST_F(BtaDmTest, bta_dm_determine_discovery_transport__BT_TRANSPORT_LE) { 


TEST_F(BtaDmTest,

       bta_dm_determine_discovery_transport__BT_TRANSPORT_AUTO__BR_EDR) {

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.transport = BT_TRANSPORT_AUTO;

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);



  mock_btm_client_interface.peer.BTM_ReadDevInfo =

      [](const RawAddress& remote_bda, tBT_DEVICE_TYPE* p_dev_type,
@@ -487,10 +480,9 @@ TEST_F(BtaDmTest, 


TEST_F(BtaDmTest,

       bta_dm_determine_discovery_transport__BT_TRANSPORT_AUTO__BLE__PUBLIC) {

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.transport = BT_TRANSPORT_AUTO;

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);



  mock_btm_client_interface.peer.BTM_ReadDevInfo =

      [](const RawAddress& remote_bda, tBT_DEVICE_TYPE* p_dev_type,
@@ -506,10 +498,9 @@ TEST_F(BtaDmTest, 


TEST_F(BtaDmTest,

       bta_dm_determine_discovery_transport__BT_TRANSPORT_AUTO__DUMO) {

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.transport = BT_TRANSPORT_AUTO;

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);



  mock_btm_client_interface.peer.BTM_ReadDevInfo =

      [](const RawAddress& remote_bda, tBT_DEVICE_TYPE* p_dev_type,
@@ -545,17 +536,13 @@ TEST_F(BtaDmTest, bta_dm_search_evt_text) { 
}



TEST_F(BtaDmTest, bta_dm_remote_name_cmpl) {

  tBTA_DM_MSG msg = {

      .remote_name_msg =

          {

  tBTA_DM_REMOTE_NAME remote_name_msg{

      // tBTA_DM_REMOTE_NAME

              .hdr = {},

      .bd_addr = kRawAddress,

      .bd_name = {0},

      .hci_status = HCI_SUCCESS,

          },

  };

  bluetooth::legacy::testing::bta_dm_remote_name_cmpl(&msg);

  bluetooth::legacy::testing::bta_dm_remote_name_cmpl(remote_name_msg);

  ASSERT_EQ(1, get_func_call_count("BTM_InqDbRead"));

}

system/bta/test/bta_sdp_test.cc

+7 −16
Original line number Diff line number Diff line
@@ -37,10 +37,8 @@ namespace bluetooth { 
namespace legacy {

namespace testing {



const tBTA_DM_SEARCH_CB& bta_dm_disc_search_cb();

tBTA_DM_SEARCH_CB bta_dm_disc_get_search_cb();

void bta_dm_disc_search_cb(const tBTA_DM_SEARCH_CB& search_cb);

void bta_dm_sdp_result(tBTA_DM_MSG* p_data);

tBTA_DM_SEARCH_CB& bta_dm_disc_search_cb();

void bta_dm_sdp_result(tBTA_DM_SDP_RESULT& sdp_event);



}  // namespace testing

}  // namespace legacy
@@ -71,21 +69,14 @@ class BtaSdpRegisteredTest : public BtaSdpTest { 
TEST_F(BtaSdpTest, nop) {}



TEST_F(BtaSdpRegisteredTest, bta_dm_sdp_result_SDP_SUCCESS) {

  tBTA_DM_SEARCH_CB search_cb =

      bluetooth::legacy::testing::bta_dm_disc_get_search_cb();

  tBTA_DM_SEARCH_CB& search_cb =

      bluetooth::legacy::testing::bta_dm_disc_search_cb();

  search_cb.service_index = BTA_MAX_SERVICE_ID;

  bluetooth::legacy::testing::bta_dm_disc_search_cb(search_cb);



  tBTA_DM_MSG msg = {

      .sdp_event =

          {

              .hdr = {},

              .sdp_result = SDP_SUCCESS,

          },

  };

  mock_btm_client_interface.security.BTM_SecReadDevName =

      [](const RawAddress& bd_addr) -> const char* { return kName; };

  mock_btm_client_interface.security.BTM_SecDeleteRmtNameNotifyCallback =

      [](tBTM_RMT_NAME_CALLBACK*) -> bool { return true; };

  bluetooth::legacy::testing::bta_dm_sdp_result(&msg);

  tBTA_DM_SDP_RESULT result{.sdp_result = SDP_SUCCESS};

  bluetooth::legacy::testing::bta_dm_sdp_result(result);

}