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

Commit 5ac5dce3 authored by Ajay Panicker's avatar Ajay Panicker
Browse files

Remove pointer to integer cast in btif_sock_l2cap to prevent crash on 64 bit systems 

Bug: 24400656
Test: Previously crashed Bluetooth, does not crash after patch
Change-Id: Ia381abca72e46d369a0de1749e42d1b6273a321a
parent 136d320e

system/bta/include/bta_jv_api.h

+2 −0
Original line number Diff line number Diff line
@@ -250,6 +250,7 @@ typedef struct { 
  uint32_t handle;       /* The connection handle */

  uint32_t req_id;       /* The req_id in the associated BTA_JvL2capWrite() */

  uint16_t len;          /* The length of the data written. */

  uint8_t* p_data;       /* The buffer where data is held */

  bool cong;             /* congestion status */

} tBTA_JV_L2CAP_WRITE;
@@ -259,6 +260,7 @@ typedef struct { 
  uint16_t channel;      /* The connection channel */

  BD_ADDR addr;          /* The peer address */

  uint32_t req_id;       /* The req_id in the associated BTA_JvL2capWrite() */

  uint8_t* p_data;       /* The buffer where data is held */

  uint16_t len;          /* The length of the data written. */

  bool cong;             /* congestion status */

} tBTA_JV_L2CAP_WRITE_FIXED;

system/bta/jv/bta_jv_act.cc

+2 −0
Original line number Diff line number Diff line
@@ -1317,6 +1317,7 @@ void bta_jv_l2cap_write(tBTA_JV_MSG* p_data) { 
    evt_data.status = BTA_JV_FAILURE;

    evt_data.handle = ls->handle;

    evt_data.req_id = ls->req_id;

    evt_data.p_data = ls->p_data;

    evt_data.cong = ls->p_cb->cong;

    evt_data.len = 0;

    bta_jv_pm_conn_busy(ls->p_cb->p_pm_cb);
@@ -1355,6 +1356,7 @@ void bta_jv_l2cap_write_fixed(tBTA_JV_MSG* p_data) { 
  evt_data.channel = ls->channel;

  memcpy(evt_data.addr, ls->addr, sizeof(evt_data.addr));

  evt_data.req_id = ls->req_id;

  evt_data.p_data = ls->p_data;

  evt_data.len = 0;



  memcpy(((uint8_t*)(msg + 1)) + L2CAP_MIN_OFFSET, ls->p_data, ls->len);

system/btif/src/btif_sock_l2cap.cc

+2 −12
Original line number Diff line number Diff line
@@ -775,13 +775,13 @@ static void btsock_l2cap_cbk(tBTA_JV_EVT event, tBTA_JV* p_data, 


    case BTA_JV_L2CAP_WRITE_EVT:

      APPL_TRACE_DEBUG("BTA_JV_L2CAP_WRITE_EVT: id: %u", sock_id);

      on_l2cap_write_done(UINT_TO_PTR(p_data->l2c_write.req_id),

      on_l2cap_write_done(p_data->l2c_write.p_data,

                          p_data->l2c_write.len, sock_id);

      break;



    case BTA_JV_L2CAP_WRITE_FIXED_EVT:

      APPL_TRACE_DEBUG("BTA_JV_L2CAP_WRITE_FIXED_EVT: id: %u", sock_id);

      on_l2cap_write_fixed_done(UINT_TO_PTR(p_data->l2c_write_fixed.req_id),

      on_l2cap_write_fixed_done(p_data->l2c_write_fixed.p_data,

                                p_data->l2c_write.len, sock_id);

      break;
@@ -1035,9 +1035,6 @@ void btsock_l2cap_signaled(int fd, int flags, uint32_t user_id) { 
      if (!(flags & SOCK_THREAD_FD_EXCEPTION) ||

          (ioctl(sock->our_fd, FIONREAD, &size) == 0 && size)) {

        uint8_t* buffer = (uint8_t*)osi_malloc(L2CAP_MAX_SDU_LENGTH);

        /* Apparently we hijack the req_id (uint32_t) to pass the pointer to the

         * buffer to the write complete callback, which call a free... wonder if

         * this works on a 64 bit platform? */

        /* The socket is created with SOCK_SEQPACKET, hence we read one message

         * at the time. The maximum size of a message is allocated to ensure

         * data is not lost. This is okay to do as Android uses virtual memory,
@@ -1061,13 +1058,6 @@ void btsock_l2cap_signaled(int fd, int flags, uint32_t user_id) { 
        APPL_TRACE_DEBUG(

            "btsock_l2cap_signaled - %d bytes received from socket", count);



        // TODO(armansito): |buffer|, which is created above via

        // malloc, is being cast below to uint32_t to be used as

        // the |req_id| parameter of BTA_JvL2capWriteFixed and

        // BTA_JvL2capWrite. The "id" then gets freed in an

        // obscure callback elsewhere. We need to watch out for

        // this type of unsafe practice, as this is error prone

        // and difficult to follow.

        if (sock->fixed_chan) {

          if (BTA_JvL2capWriteFixed(sock->channel, (BD_ADDR*)&sock->addr,

                                    PTR_TO_UINT(buffer), btsock_l2cap_cbk,