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

Commit c04ac573 authored by Chris Manton's avatar Chris Manton
Browse files

OnReadRemoteExtendedFeatures verify max page is non-zero 

Bug: 228534228
Tag: #refactor
Test: gd/cert/run
BYPASS_LONG_LINES_REASON: Bluetooth likes 120 lines
Ignore-AOSP-First: Cherrypick

Change-Id: I5d69a5f8d22afe5182134d620c139e115491fc51
parent 70868b51

system/main/shim/acl.cc

+1 −1
Original line number Diff line number Diff line
@@ -653,7 +653,7 @@ class ClassicShimAclConnection 
      return;

    }



    if (page_number != max_page_number)

    if (max_page_number != 0 && page_number != max_page_number)

      connection_->ReadRemoteExtendedFeatures(page_number + 1);

  }

system/main/test/main_shim_test.cc

+126 −3
Original line number Diff line number Diff line
@@ -128,6 +128,10 @@ void mock_connection_le_on_disconnected(tHCI_STATUS status, uint16_t handle, 
  mock_connection_le_on_disconnected_promise.set_value(handle);

}



void mock_link_classic_on_read_remote_extended_features_complete(

    uint16_t handle, uint8_t current_page_number, uint8_t max_page_number,

    uint64_t features) {}



const shim::legacy::acl_interface_t GetMockAclInterface() {

  shim::legacy::acl_interface_t acl_interface{

      .on_send_data_upwards = mock_on_send_data_upwards,
@@ -165,7 +169,8 @@ const shim::legacy::acl_interface_t GetMockAclInterface() { 
      .link.classic.on_read_link_quality_complete = nullptr,

      .link.classic.on_read_link_supervision_timeout_complete = nullptr,

      .link.classic.on_read_remote_version_information_complete = nullptr,

      .link.classic.on_read_remote_extended_features_complete = nullptr,

      .link.classic.on_read_remote_extended_features_complete =

          mock_link_classic_on_read_remote_extended_features_complete,

      .link.classic.on_read_rssi_complete = nullptr,

      .link.classic.on_read_transmit_power_level_complete = nullptr,

      .link.classic.on_role_change = nullptr,
@@ -235,6 +240,15 @@ class MockClassicAclConnection 
  bool ReadRemoteVersionInformation() override { return true; }

  bool ReadRemoteSupportedFeatures() override { return true; }



  std::function<void(uint8_t)> read_remote_extended_features_function_{};



  bool ReadRemoteExtendedFeatures(uint8_t page_number) override {

    if (read_remote_extended_features_function_) {

      read_remote_extended_features_function_(page_number);

    }

    return true;

  }



  bool Disconnect(hci::DisconnectReason reason) override {

    disconnect_cnt_++;

    disconnect_promise_.set_value(handle_);
@@ -280,6 +294,7 @@ class MockLeAclConnection 
  MockDeQueue<packet::PacketView<hci::kLittleEndian>> rx_;



  bool ReadRemoteVersionInformation() override { return true; }

  bool LeReadRemoteFeatures() override { return true; }



  void Disconnect(hci::DisconnectReason reason) override {

    disconnect_cnt_++;
@@ -379,6 +394,65 @@ class MainShimTest : public testing::Test { 
  }

};



class MainShimTestWithClassicConnection : public MainShimTest {

 protected:

  void SetUp() override {

    MainShimTest::SetUp();

    hci::Address address({0x11, 0x22, 0x33, 0x44, 0x55, 0x66});



    acl_ = MakeAcl();



    // Create connection

    EXPECT_CALL(*test::mock_acl_manager_, CreateConnection(_)).Times(1);

    acl_->CreateClassicConnection(address);



    // Respond with a mock connection created

    auto connection = std::make_unique<MockClassicAclConnection>(address, 123);

    ASSERT_EQ(123, connection->GetHandle());

    ASSERT_EQ(hci::Address({0x11, 0x22, 0x33, 0x44, 0x55, 0x66}),

              connection->GetAddress());

    raw_connection_ = connection.get();



    acl_->OnConnectSuccess(std::move(connection));

    ASSERT_EQ(nullptr, connection);

    ASSERT_NE(nullptr, raw_connection_->callbacks_);

  }



  void TearDown() override {

    // Specify local disconnect request

    auto tx_disconnect_future =

        raw_connection_->disconnect_promise_.get_future();

    acl_->DisconnectClassic(123, HCI_SUCCESS, {});



    // Wait for disconnect to be received

    uint16_t result = tx_disconnect_future.get();

    ASSERT_EQ(123, result);



    // Now emulate the remote disconnect response

    auto handle_promise = std::promise<uint16_t>();

    auto rx_disconnect_future = handle_promise.get_future();

    mock_function_handle_promise_map

        ["mock_connection_classic_on_disconnected"] = std::move(handle_promise);

    raw_connection_->callbacks_->OnDisconnection(hci::ErrorCode::SUCCESS);



    result = rx_disconnect_future.get();

    ASSERT_EQ(123, result);



    // *Our* task completing indicates reactor is done

    std::promise<void> done;

    auto future = done.get_future();

    handler_->Call([](std::promise<void> done) { done.set_value(); },

                   std::move(done));

    future.wait();



    acl_.reset();



    MainShimTest::TearDown();

  }

  std::unique_ptr<shim::legacy::Acl> acl_;

  MockClassicAclConnection* raw_connection_{nullptr};

};



TEST_F(MainShimTest, Nop) {}



TEST_F(MainShimTest, Acl_Lifecycle) {
@@ -512,7 +586,7 @@ class TestScanningCallbacks : public ::ScanningCallbacks { 
  void OnBatchScanThresholdCrossed(int client_if) override {}

};



TEST_F(MainShimTest, BleScannerInterfaceImpl_OnScanResult) {

TEST_F(MainShimTest, DISABLED_BleScannerInterfaceImpl_OnScanResult) {

  auto* ble = static_cast<bluetooth::shim::BleScannerInterfaceImpl*>(

      bluetooth::shim::get_ble_scanner_instance());
@@ -554,7 +628,7 @@ const char* test_flags[] = { 
    nullptr,

};



TEST_F(MainShimTest, LeShimAclConnection_local_disconnect) {

TEST_F(MainShimTest, DISABLED_LeShimAclConnection_local_disconnect) {

  bluetooth::common::InitFlags::Load(test_flags);

  auto acl = MakeAcl();

  EXPECT_CALL(*test::mock_acl_manager_, CreateLeConnection(_, _)).Times(1);
@@ -579,6 +653,8 @@ TEST_F(MainShimTest, LeShimAclConnection_local_disconnect) { 
                                                          remote_address, role);

  auto raw_connection = connection.get();

  acl->OnLeConnectSuccess(remote_address, std::move(connection));

  ASSERT_EQ(nullptr, connection);

  ASSERT_NE(nullptr, raw_connection->callbacks_);



  // Initiate local LE disconnect

  mock_connection_le_on_disconnected_promise = std::promise<uint16_t>();
@@ -595,3 +671,50 @@ TEST_F(MainShimTest, LeShimAclConnection_local_disconnect) { 


  ASSERT_EQ(0x1234, disconnect_future.get());

}



TEST_F(MainShimTestWithClassicConnection, nop) {}



TEST_F(MainShimTestWithClassicConnection, read_extended_feature) {

  int read_remote_extended_feature_call_count = 0;

  raw_connection_->read_remote_extended_features_function_ =

      [&read_remote_extended_feature_call_count](uint8_t page_number) {

        read_remote_extended_feature_call_count++;

      };



  // Handle typical case

  {

    read_remote_extended_feature_call_count = 0;

    const uint8_t max_page = 3;

    raw_connection_->callbacks_->OnReadRemoteExtendedFeaturesComplete(

        1, max_page, 0xabcdef9876543210);

    raw_connection_->callbacks_->OnReadRemoteExtendedFeaturesComplete(

        2, max_page, 0xbcdef9876543210a);

    raw_connection_->callbacks_->OnReadRemoteExtendedFeaturesComplete(

        3, max_page, 0xcdef9876543210ab);

    ASSERT_EQ(static_cast<int>(max_page) - 1,

              read_remote_extended_feature_call_count);

  }



  // Handle extreme case

  {

    read_remote_extended_feature_call_count = 0;

    const uint8_t max_page = 255;

    for (int page = 1; page < static_cast<int>(max_page) + 1; page++) {

      raw_connection_->callbacks_->OnReadRemoteExtendedFeaturesComplete(

          static_cast<uint8_t>(page), max_page, 0xabcdef9876543210);

    }

    ASSERT_EQ(static_cast<int>(max_page - 1),

              read_remote_extended_feature_call_count);

  }



  // Handle case where device returns max page of zero

  {

    read_remote_extended_feature_call_count = 0;

    const uint8_t max_page = 0;

    raw_connection_->callbacks_->OnReadRemoteExtendedFeaturesComplete(

        1, max_page, 0xabcdef9876543210);

    ASSERT_EQ(0, read_remote_extended_feature_call_count);

  }



  raw_connection_->read_remote_extended_features_function_ = {};

}

system/stack/Android.bp

+67 −0
Original line number Diff line number Diff line
@@ -1146,3 +1146,70 @@ cc_test { 
        },

    },

}



cc_test {

    name: "net_test_stack_acl",

    test_suites: ["device-tests"],

    host_supported: true,

    defaults: ["fluoride_defaults"],

    local_include_dirs: [

        "include",

        "test/common",

    ],

    include_dirs: [

        "packages/modules/Bluetooth/system",

        "packages/modules/Bluetooth/system/gd",

        "packages/modules/Bluetooth/system/utils/include",

    ],

    generated_headers: [

        "BluetoothGeneratedDumpsysDataSchema_h",

        "BluetoothGeneratedPackets_h",

    ],

    srcs: [

        ":OsiCompatSources",

        ":TestCommonMainHandler",

        ":TestCommonMockFunctions",

        ":TestCommonStackConfig",

        ":TestMockBta",

        ":TestMockBtif",

        ":TestMockDevice",

        ":TestMockHci",

        ":TestMockLegacyHciCommands",

        ":TestMockLegacyHciInterface",

        ":TestMockMainShim",

        ":TestMockStackBtm",

        ":TestMockStackBtu",

        ":TestMockStackCryptotoolbox",

        ":TestMockStackGatt",

        ":TestMockStackHcic",

        ":TestMockStackL2cap",

        ":TestMockStackMetrics",

        ":TestMockStackSdp",

        ":TestMockStackSmp",

        "acl/*.cc",

        "test/stack_acl_test.cc",

    ],

    static_libs: [

        "libbt-common",

        "libbt-protos-lite",

        "libgmock",

        "liblog",

        "libosi",

    ],

    shared_libs: [

        "libbinder_ndk",

        "libcrypto",

        "libflatbuffers-cpp",

        "libprotobuf-cpp-lite",

    ],

    sanitize: {

        address: true,

        all_undefined: true,

        cfi: true,

        integer_overflow: true,

        scs: true,

        diag: {

            undefined : true

        },

    },

}

system/stack/acl/btm_acl.cc

+1 −1
Original line number Diff line number Diff line
@@ -2897,7 +2897,7 @@ void acl_process_extended_features(uint16_t handle, uint8_t current_page_number, 
      bd_features_text(p_acl->peer_lmp_feature_pages[current_page_number])

          .c_str());



  if (max_page_number == current_page_number) {

  if (max_page_number == 0 || max_page_number == current_page_number) {

    NotifyAclFeaturesReadComplete(*p_acl, max_page_number);

  }

}

system/stack/test/stack_acl_test.cc

0 → 100644
+129 −0
Original line number Diff line number Diff line 
/*

 * Copyright 2022 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0(the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */

#include <gmock/gmock.h>

#include <gtest/gtest.h>



#include <cstdint>

#include <deque>



#include "common/init_flags.h"

#include "osi/include/log.h"

#include "stack/acl/acl.h"

#include "stack/btm/btm_int_types.h"

#include "stack/btm/security_device_record.h"

#include "stack/include/acl_api.h"

#include "stack/include/acl_hci_link_interface.h"

#include "stack/include/hci_error_code.h"

#include "test/common/mock_functions.h"

#include "test/mock/mock_main_shim_acl_api.h"

#include "types/ble_address_with_type.h"

#include "types/hci_role.h"

#include "types/raw_address.h"



tBTM_CB btm_cb;



void LogMsg(uint32_t trace_set_mask, const char* fmt_str, ...) {}



namespace {

const char* test_flags[] = {

    "INIT_logging_debug_enabled_for_all=true",

    nullptr,

};



const RawAddress kRawAddress = RawAddress({0x11, 0x22, 0x33, 0x44, 0x55, 0x66});

}  // namespace



namespace bluetooth {

namespace testing {



std::set<const RawAddress> copy_of_connected_with_both_public_and_random_set();



}  // namespace testing

}  // namespace bluetooth



void BTM_update_version_info(const RawAddress& bd_addr,

                             const remote_version_info& remote_version_info) {}



void btm_sec_role_changed(tHCI_STATUS hci_status, const RawAddress& bd_addr,

                          tHCI_ROLE new_role) {}



class StackAclTest : public testing::Test {

 protected:

  void SetUp() override {

    mock_function_count_map.clear();

    bluetooth::common::InitFlags::Load(test_flags);

  }

  void TearDown() override {}



  tBTM_SEC_DEV_REC device_record_;

};



TEST_F(StackAclTest, nop) {}



TEST_F(StackAclTest, acl_process_extended_features) {

  const uint16_t hci_handle = 0x123;

  const tBT_TRANSPORT transport = BT_TRANSPORT_LE;

  const tHCI_ROLE link_role = HCI_ROLE_CENTRAL;



  btm_acl_created(kRawAddress, hci_handle, link_role, transport);

  tACL_CONN* p_acl = btm_acl_for_bda(kRawAddress, transport);

  ASSERT_NE(nullptr, p_acl);



  // Handle typical case

  {

    const uint8_t max_page = 3;

    memset((void*)p_acl->peer_lmp_feature_valid, 0,

           HCI_EXT_FEATURES_PAGE_MAX + 1);

    acl_process_extended_features(hci_handle, 1, max_page, 0xf123456789abcde);

    acl_process_extended_features(hci_handle, 2, max_page, 0xef123456789abcd);

    acl_process_extended_features(hci_handle, 3, max_page, 0xdef123456789abc);



    /* page 0 is the standard feature set */

    ASSERT_FALSE(p_acl->peer_lmp_feature_valid[0]);

    ASSERT_TRUE(p_acl->peer_lmp_feature_valid[1]);

    ASSERT_TRUE(p_acl->peer_lmp_feature_valid[2]);

    ASSERT_TRUE(p_acl->peer_lmp_feature_valid[3]);

  }



  // Handle extreme case

  {

    const uint8_t max_page = 255;

    memset((void*)p_acl->peer_lmp_feature_valid, 0,

           HCI_EXT_FEATURES_PAGE_MAX + 1);

    for (int i = 1; i < HCI_EXT_FEATURES_PAGE_MAX + 1; i++) {

      acl_process_extended_features(hci_handle, static_cast<uint8_t>(i),

                                    max_page, 0x123456789abcdef);

    }

    /* page 0 is the standard feature set */

    ASSERT_FALSE(p_acl->peer_lmp_feature_valid[0]);

    ASSERT_TRUE(p_acl->peer_lmp_feature_valid[1]);

    ASSERT_TRUE(p_acl->peer_lmp_feature_valid[2]);

    ASSERT_TRUE(p_acl->peer_lmp_feature_valid[3]);

  }



  // Handle case where device returns max page of zero

  {

    memset((void*)p_acl->peer_lmp_feature_valid, 0,

           HCI_EXT_FEATURES_PAGE_MAX + 1);

    acl_process_extended_features(hci_handle, 1, 0, 0xdef123456789abc);

    ASSERT_FALSE(p_acl->peer_lmp_feature_valid[0]);

    ASSERT_TRUE(p_acl->peer_lmp_feature_valid[1]);

    ASSERT_FALSE(p_acl->peer_lmp_feature_valid[2]);

    ASSERT_FALSE(p_acl->peer_lmp_feature_valid[3]);

  }



  btm_acl_removed(hci_handle);

}