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

Commit 8a5c418d authored by Himanshu Rawat's avatar Himanshu Rawat
Browse files

Race condition in opening/closing of uhid file descriptor 

When HID host is connected, uhid file descriptor is opened from bta_hh
in main thread. However, when it is disconnected, uhid file descriptor
is closed from btif_hh in jni thread.
If the HID device gets disconnected and connected in quick succession,
the race condition can cause connection to fail.

Bug: 264988156
Test: Manual
Change-Id: I9ff6196eeebdd0a1dd5245f29a174c4b28dbe1b6
parent 9b561328

system/bta/hh/bta_hh_act.cc

+19 −21
Original line number Diff line number Diff line
@@ -617,13 +617,12 @@ void bta_hh_open_cmpl_act(tBTA_HH_DEV_CB* p_cb, const tBTA_HH_DATA* p_data) { 
  /* increase connection number */

  bta_hh_cb.cnt_num++;



  /* initialize device driver */

  bta_hh_co_open(p_cb->hid_handle, p_cb->sub_class, p_cb->attr_mask,

                 p_cb->app_id);



  conn.status = p_cb->status;

  conn.le_hid = p_cb->is_le_device;

  conn.scps_supported = p_cb->scps_supported;

  conn.sub_class = p_cb->sub_class;

  conn.attr_mask = p_cb->attr_mask;

  conn.app_id = p_cb->app_id;



  BTM_LogHistory(kBtmLogTag, p_cb->addr, "Opened",

                 base::StringPrintf(
@@ -634,26 +633,25 @@ void bta_hh_open_cmpl_act(tBTA_HH_DEV_CB* p_cb, const tBTA_HH_DATA* p_data) { 
  {

    /* inform role manager */

    bta_sys_conn_open(BTA_ID_HH, p_cb->app_id, p_cb->addr);

  }



    /* set protocol mode when not default report mode */

  if (p_cb->mode != BTA_HH_PROTO_RPT_MODE

      && !p_cb->is_le_device

      ) {

    if ((HID_HostWriteDev(dev_handle, HID_TRANS_SET_PROTOCOL,

                          HID_PAR_PROTOCOL_BOOT_MODE, 0, 0, NULL)) !=

        HID_SUCCESS) {

    if (p_cb->mode != BTA_HH_PROTO_RPT_MODE) {

      tHID_STATUS status =

          HID_HostWriteDev(dev_handle, HID_TRANS_SET_PROTOCOL,

                           HID_PAR_PROTOCOL_BOOT_MODE, 0, 0, NULL);



      if (status == HID_SUCCESS) {

        p_cb->w4_evt = BTA_HH_SET_PROTO_EVT;

      } else {

        /* HID connection is up, while SET_PROTO fail */

        conn.status = BTA_HH_ERR_PROTO;

      (*bta_hh_cb.p_cback)(BTA_HH_OPEN_EVT, (tBTA_HH*)&conn);

    } else {

      conn.status = BTA_HH_OK;

      p_cb->w4_evt = BTA_HH_OPEN_EVT;

      }

  } else

    (*bta_hh_cb.p_cback)(BTA_HH_OPEN_EVT, (tBTA_HH*)&conn);



    }

  }

  p_cb->incoming_conn = false;

  p_cb->incoming_hid_handle = BTA_HH_INVALID_HANDLE;



  (*bta_hh_cb.p_cback)(BTA_HH_OPEN_EVT, (tBTA_HH*)&conn);

}

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

 *

system/bta/include/bta_hh_api.h

+5 −3
Original line number Diff line number Diff line
@@ -241,7 +241,9 @@ typedef struct { 
  uint8_t handle;        /* device handle            */

  bool le_hid;           /* is LE devices? */

  bool scps_supported;   /* scan parameter service supported */



  uint8_t sub_class;     /* Cod sub class */

  uint16_t attr_mask;    /* attribute mask */

  uint8_t app_id;

} tBTA_HH_CONN;



typedef tBTA_HH_CONN tBTA_HH_DEV_INFO;

system/bta/include/bta_hh_co.h

+2 −2
Original line number Diff line number Diff line
@@ -62,10 +62,10 @@ void bta_hh_co_data(uint8_t dev_handle, uint8_t* p_rpt, uint16_t len, 
 *                  opened, and application may do some device specific

 *                  initialization.

 *

 * Returns          void.

 * Returns          True if platform specific initialization is successful

 *

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

void bta_hh_co_open(uint8_t dev_handle, uint8_t sub_class, uint16_t attr_mask,

bool bta_hh_co_open(uint8_t dev_handle, uint8_t sub_class, uint16_t attr_mask,

                    uint8_t app_id);



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

system/btif/co/bta_hh_co.cc

+37 −36
Original line number Diff line number Diff line
@@ -352,6 +352,22 @@ int bta_hh_co_write(int fd, uint8_t* rpt, uint16_t len) { 
  return uhid_write(fd, &ev);

}



static bool uhid_fd_open(btif_hh_device_t* p_dev) {

  if (p_dev->fd < 0) {

    p_dev->fd = open(dev_path, O_RDWR | O_CLOEXEC);

    if (p_dev->fd < 0) {

      LOG_ERROR("Failed to open uhid, err:%s", strerror(errno));

      return false;

    }

  }



  if (p_dev->hh_keep_polling == 0) {

    p_dev->hh_keep_polling = 1;

    p_dev->hh_poll_thread_id = create_thread(btif_hh_poll_event_thread, p_dev);

  }

  return true;

}



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

 *

 * Function      bta_hh_co_open
@@ -359,17 +375,16 @@ int bta_hh_co_write(int fd, uint8_t* rpt, uint16_t len) { 
 * Description   When connection is opened, this call-out function is executed

 *               by HH to do platform specific initialization.

 *

 * Returns       void.

 * Returns       True if platform specific initialization is successful

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

void bta_hh_co_open(uint8_t dev_handle, uint8_t sub_class,

bool bta_hh_co_open(uint8_t dev_handle, uint8_t sub_class,

                    tBTA_HH_ATTR_MASK attr_mask, uint8_t app_id) {

  uint32_t i;

  btif_hh_device_t* p_dev = NULL;



  if (dev_handle == BTA_HH_INVALID_HANDLE) {

    APPL_TRACE_WARNING("%s: Oops, dev_handle (%d) is invalid...", __func__,

                       dev_handle);

    return;

    LOG_WARN("dev_handle (%d) is invalid", dev_handle);

    return false;

  }



  for (i = 0; i < BTIF_HH_MAX_HID; i++) {
@@ -377,25 +392,15 @@ void bta_hh_co_open(uint8_t dev_handle, uint8_t sub_class, 
    if (p_dev->dev_status != BTHH_CONN_STATE_UNKNOWN &&

        p_dev->dev_handle == dev_handle) {

      // We found a device with the same handle. Must be a device reconnected.

      APPL_TRACE_WARNING(

          "%s: Found an existing device with the same handle dev_status=%d, "

      LOG_INFO(

          "Found an existing device with the same handle dev_status=%d, "

          "address=%s, attr_mask=0x%04x, sub_class=0x%02x, app_id=%d",

          __func__, p_dev->dev_status, ADDRESS_TO_LOGGABLE_CSTR(p_dev->bd_addr),

          p_dev->dev_status, ADDRESS_TO_LOGGABLE_CSTR(p_dev->bd_addr),

          p_dev->attr_mask, p_dev->sub_class, p_dev->app_id);



      if (p_dev->fd < 0) {

        p_dev->fd = open(dev_path, O_RDWR | O_CLOEXEC);

        if (p_dev->fd < 0) {

          APPL_TRACE_ERROR("%s: Error: failed to open uhid, err:%s", __func__,

                           strerror(errno));

          return;

        } else

          APPL_TRACE_DEBUG("%s: uhid fd = %d", __func__, p_dev->fd);

      if (!uhid_fd_open(p_dev)) {

        return false;

      }



      p_dev->hh_keep_polling = 1;

      p_dev->hh_poll_thread_id =

          create_thread(btif_hh_poll_event_thread, p_dev);

      break;

    }

    p_dev = NULL;
@@ -406,6 +411,14 @@ void bta_hh_co_open(uint8_t dev_handle, uint8_t sub_class, 
    for (i = 0; i < BTIF_HH_MAX_HID; i++) {

      if (btif_hh_cb.devices[i].dev_status == BTHH_CONN_STATE_UNKNOWN) {

        p_dev = &btif_hh_cb.devices[i];

        p_dev->fd = -1;

        p_dev->hh_keep_polling = 0;



        // This is a new device, open the uhid driver now.

        if (!uhid_fd_open(p_dev)) {

          return false;

        }



        p_dev->dev_handle = dev_handle;

        p_dev->attr_mask = attr_mask;

        p_dev->sub_class = sub_class;
@@ -413,27 +426,14 @@ void bta_hh_co_open(uint8_t dev_handle, uint8_t sub_class, 
        p_dev->local_vup = false;



        btif_hh_cb.device_num++;

        // This is a new device,open the uhid driver now.

        p_dev->fd = open(dev_path, O_RDWR | O_CLOEXEC);

        if (p_dev->fd < 0) {

          APPL_TRACE_ERROR("%s: Error: failed to open uhid, err:%s", __func__,

                           strerror(errno));

          return;

        } else {

          APPL_TRACE_DEBUG("%s: uhid fd = %d", __func__, p_dev->fd);

          p_dev->hh_keep_polling = 1;

          p_dev->hh_poll_thread_id =

              create_thread(btif_hh_poll_event_thread, p_dev);

        }



        break;

      }

    }

  }



  if (p_dev == NULL) {

    APPL_TRACE_ERROR("%s: Error: too many HID devices are connected", __func__);

    return;

    LOG_ERROR("Too many HID devices are connected");

    return false;

  }



  p_dev->dev_status = BTHH_CONN_STATE_CONNECTED;
@@ -444,7 +444,8 @@ void bta_hh_co_open(uint8_t dev_handle, uint8_t sub_class, 
  CHECK(p_dev->set_rpt_id_queue);

#endif  // ENABLE_UHID_SET_REPORT



  APPL_TRACE_DEBUG("%s: Return device status %d", __func__, p_dev->dev_status);

  LOG_DEBUG("Return device status %d", p_dev->dev_status);

  return true;

}



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

system/btif/src/btif_hh.cc

+63 −64
Original line number Diff line number Diff line
@@ -63,9 +63,6 @@ 


static int btif_hh_keylockstates = 0;  // The current key state of each key



// TODO This is duplicated in header file with different value

#define BTIF_HH_DEV_DISCONNECTED 3



#define BTIF_TIMEOUT_VUP_MS (3 * 1000)



/* HH request events */
@@ -360,7 +357,7 @@ static btif_hh_device_t* btif_hh_find_connected_dev_by_bda( 
 *

 * Returns      void

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

void btif_hh_stop_vup_timer(RawAddress* bd_addr) {

static void btif_hh_stop_vup_timer(RawAddress* bd_addr) {

  btif_hh_device_t* p_dev = btif_hh_find_connected_dev_by_bda(*bd_addr);



  if (p_dev != NULL) {
@@ -377,7 +374,7 @@ void btif_hh_stop_vup_timer(RawAddress* bd_addr) { 
 *

 * Returns      void

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

void btif_hh_start_vup_timer(const RawAddress* bd_addr) {

static void btif_hh_start_vup_timer(const RawAddress* bd_addr) {

  BTIF_TRACE_DEBUG("%s", __func__);



  btif_hh_device_t* p_dev = btif_hh_find_connected_dev_by_bda(*bd_addr);
@@ -389,6 +386,66 @@ void btif_hh_start_vup_timer(const RawAddress* bd_addr) { 
                     btif_hh_timer_timeout, p_dev);

}



static void hh_connect_complete(uint8_t handle, RawAddress& bda,

                                BTIF_HH_STATUS status) {

  bthh_connection_state_t state = BTHH_CONN_STATE_CONNECTED;

  btif_hh_cb.status = status;



  if (status != BTIF_HH_DEV_CONNECTED) {

    state = BTHH_CONN_STATE_DISCONNECTED;

    BTA_HhClose(handle);

  }

  HAL_CBACK(bt_hh_callbacks, connection_state_cb, &bda, state);

}



static void hh_open_handler(tBTA_HH_CONN& conn) {

  LOG_DEBUG("status = %d, handle = %d", conn.status, conn.handle);



  HAL_CBACK(bt_hh_callbacks, connection_state_cb, (RawAddress*)&conn.bda,

            BTHH_CONN_STATE_CONNECTING);

  btif_hh_cb.pending_conn_address = RawAddress::kEmpty;



  if (conn.status != BTA_HH_OK) {

    btif_dm_hh_open_failed(&conn.bda);

    btif_hh_device_t* p_dev = btif_hh_find_dev_by_bda(conn.bda);

    if (p_dev != NULL) {

      btif_hh_stop_vup_timer(&(p_dev->bd_addr));

      p_dev->dev_status = BTHH_CONN_STATE_DISCONNECTED;

    }

    hh_connect_complete(conn.handle, conn.bda, BTIF_HH_DEV_DISCONNECTED);

    return;

  }



  /* Initialize device driver */

  if (!bta_hh_co_open(conn.handle, conn.sub_class, conn.attr_mask,

                      conn.app_id)) {

    LOG_WARN("Failed to find the uhid driver");

    hh_connect_complete(conn.handle, conn.bda, BTIF_HH_DEV_DISCONNECTED);

    return;

  }



  btif_hh_device_t* p_dev = btif_hh_find_connected_dev_by_handle(conn.handle);

  if (p_dev == NULL) {

    /* The connect request must have come from device side and exceeded the

     * connected HID device number. */

    LOG_WARN("Cannot find device with handle %d", conn.handle);

    hh_connect_complete(conn.handle, conn.bda, BTIF_HH_DEV_DISCONNECTED);

    return;

  }



  LOG_INFO("Found device, getting dscp info for handle %d", conn.handle);



  p_dev->bd_addr = conn.bda;

  p_dev->dev_status = BTHH_CONN_STATE_CONNECTED;

  hh_connect_complete(conn.handle, conn.bda, BTIF_HH_DEV_CONNECTED);

  // Send set_idle if the peer_device is a keyboard

  if (check_cod(&conn.bda, COD_HID_KEYBOARD) ||

      check_cod(&conn.bda, COD_HID_COMBO)) {

    BTA_HhSetIdle(conn.handle, 0);

  }

  BTA_HhGetDscpInfo(conn.handle);

}



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

 *

 * Function         btif_hh_add_added_dev
@@ -801,65 +858,7 @@ static void btif_hh_upstreams_evt(uint16_t event, char* p_param) { 
      break;



    case BTA_HH_OPEN_EVT:

      BTIF_TRACE_DEBUG("BTA_HH_OPEN_EVT: status = %d, handle = %d",

                       p_data->dev_status.status, p_data->dev_status.handle);

      HAL_CBACK(bt_hh_callbacks, connection_state_cb,

                (RawAddress*)&p_data->conn.bda, BTHH_CONN_STATE_CONNECTING);

      btif_hh_cb.pending_conn_address = RawAddress::kEmpty;

      if (p_data->conn.status == BTA_HH_OK) {

        p_dev = btif_hh_find_connected_dev_by_handle(p_data->conn.handle);

        if (p_dev == NULL) {

          BTIF_TRACE_WARNING(

              "BTA_HH_OPEN_EVT: Error, cannot find device with handle %d",

              p_data->conn.handle);

          btif_hh_cb.status = (BTIF_HH_STATUS)BTIF_HH_DEV_DISCONNECTED;

          // The connect request must come from device side and exceeded the

          // connected

          // HID device number.

          BTA_HhClose(p_data->conn.handle);

          HAL_CBACK(bt_hh_callbacks, connection_state_cb,

                    (RawAddress*)&p_data->conn.bda,

                    BTHH_CONN_STATE_DISCONNECTED);

        } else if (p_dev->fd < 0) {

          BTIF_TRACE_WARNING(

              "BTA_HH_OPEN_EVT: Error, failed to find the uhid driver...");

          p_dev->bd_addr = p_data->conn.bda;

          // remove the connection  and then try again to reconnect from the

          // mouse side to recover

          btif_hh_cb.status = (BTIF_HH_STATUS)BTIF_HH_DEV_DISCONNECTED;

          BTA_HhClose(p_data->conn.handle);

        } else {

          BTIF_TRACE_WARNING(

              "BTA_HH_OPEN_EVT: Found device...Getting dscp info for handle "

              "... %d",

              p_data->conn.handle);

          p_dev->bd_addr = p_data->conn.bda;

          btif_hh_cb.status = (BTIF_HH_STATUS)BTIF_HH_DEV_CONNECTED;

          // Send set_idle if the peer_device is a keyboard

          if (check_cod(&p_data->conn.bda, COD_HID_KEYBOARD) ||

              check_cod(&p_data->conn.bda, COD_HID_COMBO))

            BTA_HhSetIdle(p_data->conn.handle, 0);

          BTA_HhGetDscpInfo(p_data->conn.handle);

          p_dev->dev_status = BTHH_CONN_STATE_CONNECTED;

          HAL_CBACK(bt_hh_callbacks, connection_state_cb, &(p_dev->bd_addr),

                    p_dev->dev_status);

        }

      } else {

        RawAddress* bdaddr = &p_data->conn.bda;

        btif_dm_hh_open_failed(bdaddr);

        p_dev = btif_hh_find_dev_by_bda(*bdaddr);

        if (p_dev != NULL) {

          btif_hh_stop_vup_timer(&(p_dev->bd_addr));

          if (p_dev->fd >= 0) {

            bta_hh_co_destroy(p_dev->fd);

            p_dev->fd = -1;

          }

          p_dev->dev_status = BTHH_CONN_STATE_DISCONNECTED;

        }

        HAL_CBACK(bt_hh_callbacks, connection_state_cb,

                  (RawAddress*)&p_data->conn.bda, BTHH_CONN_STATE_DISCONNECTED);

        btif_hh_cb.status = (BTIF_HH_STATUS)BTIF_HH_DEV_DISCONNECTED;

      }

      hh_open_handler(p_data->conn);

      break;



    case BTA_HH_CLOSE_EVT: