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

Commit 6822ce80 authored by Pavlin Radoslavov's avatar Pavlin Radoslavov
Browse files

Fix an HCI race condition when transmitting a packet 

There is a race condition when calling
event_command_ready() -> transmit_fragment() -> hci_transmit()

If right after hci_transmit() there is thread context switch
and another thread executes filter_incoming_event() for the same
command, the corresponding packet/command will be taken off the
commands_pending_response list and free()-ed.

However, after the execution on the first thread continues
within transmit_fragment(), the execution logic will continue using
the "packet" that was already free()-ed by the other thread.

To prevent this from happening, the "commands_pending_response_mutex"
within event_command_ready() has to protect the transmit_fragment()
execution and the update_command_response_timer() function right after it.

Also:
 * Changed the "commands_pending_response_mutex" to recursive_mutex
 * Added "commands_pending_response_mutex" protection in few other
   places where "commands_pending_response" itself is used.

Bug: 36205494
Test: Running ASAN build
Change-Id: I63677ad1f2b28683c321631e9e29e4f01628d269
parent 3fa65a82

system/hci/src/hci_layer.cc

+17 −12
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ static fixed_queue_t* packet_queue; 
// Inbound-related

static alarm_t* command_response_timer;

static list_t* commands_pending_response;

static std::mutex commands_pending_response_mutex;

static std::recursive_mutex commands_pending_response_mutex;



// The hand-off point for data going to a higher layer, set by the higher layer

static fixed_queue_t* upwards_data_queue;
@@ -239,8 +239,11 @@ static future_t* hci_module_shut_down() { 
  command_queue = NULL;

  fixed_queue_free(packet_queue, buffer_allocator->free);

  packet_queue = NULL;

  {

    std::lock_guard<std::recursive_mutex> lock(commands_pending_response_mutex);

    list_free(commands_pending_response);

    commands_pending_response = NULL;

  }



  packet_fragmenter->cleanup();
@@ -316,7 +319,7 @@ static void transmit_downward(data_dispatcher_type_t type, void* data) { 


static void event_finish_startup(UNUSED_ATTR void* context) {

  LOG_INFO(LOG_TAG, "%s", __func__);

  std::lock_guard<std::mutex> lock(commands_pending_response_mutex);

  std::lock_guard<std::recursive_mutex> lock(commands_pending_response_mutex);

  alarm_cancel(startup_timer);

  future_ready(startup_future, FUTURE_SUCCESS);

  startup_future = NULL;
@@ -325,7 +328,7 @@ static void event_finish_startup(UNUSED_ATTR void* context) { 
static void startup_timer_expired(UNUSED_ATTR void* context) {

  LOG_ERROR(LOG_TAG, "%s", __func__);



  std::lock_guard<std::mutex> lock(commands_pending_response_mutex);

  std::lock_guard<std::recursive_mutex> lock(commands_pending_response_mutex);

  future_ready(startup_future, FUTURE_FAIL);

  startup_future = NULL;

}
@@ -341,9 +344,9 @@ static void event_command_ready(fixed_queue_t* queue, 


    // Move it to the list of commands awaiting response

    {

      std::lock_guard<std::mutex> lock(commands_pending_response_mutex);

      std::lock_guard<std::recursive_mutex> lock(

          commands_pending_response_mutex);

      list_append(commands_pending_response, wait_entry);

    }



      // Send it off

      packet_fragmenter->fragment_and_dispatch(wait_entry->command);
@@ -351,6 +354,7 @@ static void event_command_ready(fixed_queue_t* queue, 
      update_command_response_timer();

    }

  }

}



static void event_packet_ready(fixed_queue_t* queue,

                               UNUSED_ATTR void* context) {
@@ -385,7 +389,7 @@ static void fragmenter_transmit_finished(BT_HDR* packet, 
}



static void command_timed_out(UNUSED_ATTR void* context) {

  std::unique_lock<std::mutex> lock(commands_pending_response_mutex);

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



  if (list_is_empty(commands_pending_response)) {

    LOG_ERROR(LOG_TAG, "%s with no commands pending response", __func__);
@@ -500,7 +504,7 @@ static void dispatch_reassembled(BT_HDR* packet) { 
// Misc internal functions



static waiting_command_t* get_waiting_command(command_opcode_t opcode) {

  std::lock_guard<std::mutex> lock(commands_pending_response_mutex);

  std::lock_guard<std::recursive_mutex> lock(commands_pending_response_mutex);



  for (const list_node_t* node = list_begin(commands_pending_response);

       node != list_end(commands_pending_response); node = list_next(node)) {
@@ -518,6 +522,7 @@ static waiting_command_t* get_waiting_command(command_opcode_t opcode) { 
}



static void update_command_response_timer(void) {

  std::lock_guard<std::recursive_mutex> lock(commands_pending_response_mutex);

  if (list_is_empty(commands_pending_response)) {

    alarm_cancel(command_response_timer);

  } else {