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

Commit e33e3828 authored by Pavlin Radoslavov's avatar Pavlin Radoslavov
Browse files

Fix a race condition in the HCI module start_up() 

* Fix a race condition when using the static startup_future
  inside hci_layer.c
  The future_new() allocation happens inside function start_up()
  and the allocated "startup_future" value is returned, so the caller can
  call future_await(future).
  However, if firmware_config_callback() is called on another thread
  BEFORE the "return startup_future;" statement is reached, then
  "startup_future" is reset to NULL. As a result, the caller
  will call future_await(NULL), and we have memory leak: startup_future
  is never freed.

* Fix other similar potential race conditions inside stack_manager.c
  where the static "hack_future" variable could be reassigned between
  the future_new() and future_await() calls.

Bug: 25766403
Change-Id: I0ef1165efba7412c190dfa2a7660189b28fa78a6
parent e532824b

system/btif/src/stack_manager.c

+9 −6
Original line number Diff line number Diff line
@@ -126,13 +126,14 @@ static void event_start_up_stack(UNUSED_ATTR void *context) { 
  ensure_stack_is_initialized();



  LOG_DEBUG(LOG_TAG, "%s is bringing up the stack.", __func__);

  hack_future = future_new();

  future_t *local_hack_future = future_new();

  hack_future = local_hack_future;



  // Include this for now to put btif config into a shutdown-able state

  module_start_up(get_module(BTIF_CONFIG_MODULE));

  bte_main_enable();



  if (future_await(hack_future) != FUTURE_SUCCESS) {

  if (future_await(local_hack_future) != FUTURE_SUCCESS) {

    stack_is_running = true; // So stack shutdown actually happens

    event_shut_down_stack(NULL);

    return;
@@ -151,13 +152,14 @@ static void event_shut_down_stack(UNUSED_ATTR void *context) { 
  }



  LOG_DEBUG(LOG_TAG, "%s is bringing down the stack.", __func__);

  hack_future = future_new();

  future_t *local_hack_future = future_new();

  hack_future = local_hack_future;

  stack_is_running = false;



  btif_disable_bluetooth();

  module_shut_down(get_module(BTIF_CONFIG_MODULE));



  future_await(hack_future);

  future_await(local_hack_future);

  module_shut_down(get_module(CONTROLLER_MODULE)); // Doesn't do any work, just puts it in a restartable state



  LOG_DEBUG(LOG_TAG, "%s finished.", __func__);
@@ -181,14 +183,15 @@ static void event_clean_up_stack(UNUSED_ATTR void *context) { 
  ensure_stack_is_not_running();



  LOG_DEBUG(LOG_TAG, "%s is cleaning up the stack.", __func__);

  hack_future = future_new();

  future_t *local_hack_future = future_new();

  hack_future = local_hack_future;

  stack_is_initialized = false;



  btif_shutdown_bluetooth();

  module_clean_up(get_module(BTIF_CONFIG_MODULE));

  module_clean_up(get_module(BT_UTILS_MODULE));



  future_await(hack_future);

  future_await(local_hack_future);

  module_clean_up(get_module(OSI_MODULE));

  module_management_stop();

  LOG_DEBUG(LOG_TAG, "%s finished.", __func__);

system/hci/src/hci_layer.c

+3 −2
Original line number Diff line number Diff line
@@ -274,10 +274,11 @@ static future_t *start_up(void) { 
  power_state = BT_VND_PWR_ON;

  vendor->send_command(VENDOR_CHIP_POWER_CONTROL, &power_state);



  startup_future = future_new();

  future_t *local_startup_future = future_new();

  startup_future = local_startup_future;

  LOG_DEBUG(LOG_TAG, "%s starting async portion", __func__);

  thread_post(thread, event_finish_startup, NULL);

  return startup_future;

  return local_startup_future;

error:;

  shut_down(); // returns NULL so no need to wait for it

  return future_new_immediate(FUTURE_FAIL);