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

Commit 04833343 authored by Jakub Pawlowski's avatar Jakub Pawlowski
Browse files

Revert "Use vector instead of raw pointer in l2cap socket write handling" 

This reverts commit a9264e43.

Change-Id: Id8a399ec6f7474b92579c4647899e424feeb0d33
parent a9264e43

system/bta/include/bta_jv_api.h

+11 −4
Original line number Diff line number Diff line
@@ -244,7 +244,9 @@ typedef struct { 
typedef struct {

  tBTA_JV_STATUS status; /* Whether the operation succeeded or failed. */

  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;
@@ -253,6 +255,8 @@ typedef struct { 
  tBTA_JV_STATUS status; /* Whether the operation succeeded or failed. */

  uint16_t channel;      /* The connection channel */

  RawAddress 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;
@@ -676,7 +680,8 @@ tBTA_JV_STATUS BTA_JvL2capReady(uint32_t handle, uint32_t* p_data_size); 
 *                  BTA_JV_FAILURE, otherwise.

 *

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

tBTA_JV_STATUS BTA_JvL2capWrite(uint32_t handle, std::vector<uint8_t> data,

tBTA_JV_STATUS BTA_JvL2capWrite(uint32_t handle, uint32_t req_id,

                                uint8_t* p_data, uint16_t len,

                                uint32_t user_id);



/*******************************************************************************
@@ -692,9 +697,11 @@ tBTA_JV_STATUS BTA_JvL2capWrite(uint32_t handle, std::vector<uint8_t> data, 
 *                  BTA_JV_FAILURE, otherwise.

 *

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

void BTA_JvL2capWriteFixed(uint16_t channel, const RawAddress& addr,

tBTA_JV_STATUS BTA_JvL2capWriteFixed(uint16_t channel, const RawAddress& addr,

                                     uint32_t req_id,

                                     tBTA_JV_L2CAP_CBACK* p_cback,

                           std::vector<uint8_t> data, uint32_t user_id);

                                     uint8_t* p_data, uint16_t len,

                                     uint32_t user_id);



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

 *

system/bta/jv/bta_jv_act.cc

+16 −10
Original line number Diff line number Diff line
@@ -1132,8 +1132,8 @@ void bta_jv_l2cap_stop_server(uint16_t local_psm, uint32_t l2cap_socket_id) { 
}



/* Write data to an L2CAP connection */

void bta_jv_l2cap_write(uint32_t handle, const std::vector<uint8_t>& data,

                        uint32_t user_id, tBTA_JV_L2C_CB* p_cb) {

void bta_jv_l2cap_write(uint32_t handle, uint32_t req_id, uint8_t* p_data,

                        uint16_t len, uint32_t user_id, tBTA_JV_L2C_CB* p_cb) {

  /* As we check this callback exists before the tBTA_JV_API_L2CAP_WRITE can be

   * send through the API this check should not be needed. But the API is not

   * designed to be used (safely at least) in a multi-threaded scheduler, hence
@@ -1144,7 +1144,11 @@ void bta_jv_l2cap_write(uint32_t handle, const std::vector<uint8_t>& data, 
   * of 4 disconnects, as a disconnect on the server channel causes a disconnect

   * to be send on the client (notification) channel, but at the peer typically

   * disconnects both the OBEX disconnect request crosses the incoming l2cap

   * disconnect. If p_cback is cleared, we simply discard the data.*/

   * disconnect. If p_cback is cleared, we simply discard the data. RISK: The

   * caller must handle any cleanup based on another signal than

   * BTA_JV_L2CAP_WRITE_EVT, which is typically not possible, as the pointer to

   * the allocated buffer is stored in this message, and can therefore not be

   * freed, hence we have a mem-leak-by-design.*/

  if (!p_cb->p_cback) {

    /* As this pointer is checked in the API function, this occurs only when the

     * channel is disconnected after the API function is called, but before the
@@ -1156,12 +1160,13 @@ void bta_jv_l2cap_write(uint32_t handle, const std::vector<uint8_t>& data, 
  tBTA_JV_L2CAP_WRITE evt_data;

  evt_data.status = BTA_JV_FAILURE;

  evt_data.handle = handle;

  evt_data.req_id = req_id;

  evt_data.p_data = p_data;

  evt_data.cong = p_cb->cong;

  evt_data.len = 0;

  bta_jv_pm_conn_busy(p_cb->p_pm_cb);

  if (!evt_data.cong &&

      BT_PASS ==

          GAP_ConnWriteData(handle, data.data(), data.size(), &evt_data.len)) {

      BT_PASS == GAP_ConnWriteData(handle, p_data, len, &evt_data.len)) {

    evt_data.status = BTA_JV_SUCCESS;

  }

  tBTA_JV bta_jv;
@@ -1171,18 +1176,19 @@ void bta_jv_l2cap_write(uint32_t handle, const std::vector<uint8_t>& data, 


/* Write data to an L2CAP connection using Fixed channels */

void bta_jv_l2cap_write_fixed(uint16_t channel, const RawAddress& addr,

                              const std::vector<uint8_t>& data,

                              uint32_t req_id, uint8_t* p_data, uint16_t len,

                              uint32_t user_id, tBTA_JV_L2CAP_CBACK* p_cback) {

  tBTA_JV_L2CAP_WRITE_FIXED evt_data;

  evt_data.status = BTA_JV_FAILURE;

  evt_data.channel = channel;

  evt_data.addr = addr;

  evt_data.req_id = req_id;

  evt_data.p_data = p_data;

  evt_data.len = 0;



  BT_HDR* msg =

      (BT_HDR*)osi_malloc(sizeof(BT_HDR) + data.size() + L2CAP_MIN_OFFSET);

  memcpy(((uint8_t*)(msg + 1)) + L2CAP_MIN_OFFSET, data.data(), data.size());

  msg->len = data.size();

  BT_HDR* msg = (BT_HDR*)osi_malloc(sizeof(BT_HDR) + len + L2CAP_MIN_OFFSET);

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

  msg->len = len;

  msg->offset = L2CAP_MIN_OFFSET;



  L2CA_SendFixedChnlData(channel, addr, msg);

system/bta/jv/bta_jv_api.cc

+15 −7
Original line number Diff line number Diff line
@@ -512,15 +512,16 @@ tBTA_JV_STATUS BTA_JvL2capReady(uint32_t handle, uint32_t* p_data_size) { 
 *                  BTA_JV_FAILURE, otherwise.

 *

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

tBTA_JV_STATUS BTA_JvL2capWrite(uint32_t handle, std::vector<uint8_t> data,

tBTA_JV_STATUS BTA_JvL2capWrite(uint32_t handle, uint32_t req_id,

                                uint8_t* p_data, uint16_t len,

                                uint32_t user_id) {

  VLOG(2) << __func__;



  if (handle >= BTA_JV_MAX_L2C_CONN || !bta_jv_cb.l2c_cb[handle].p_cback)

    return BTA_JV_FAILURE;



  do_in_bta_thread(FROM_HERE, Bind(&bta_jv_l2cap_write, handle, std::move(data),

                                   user_id, &bta_jv_cb.l2c_cb[handle]));

  do_in_bta_thread(FROM_HERE, Bind(&bta_jv_l2cap_write, handle, req_id, p_data,

                                   len, user_id, &bta_jv_cb.l2c_cb[handle]));

  return BTA_JV_SUCCESS;

}
@@ -533,13 +534,20 @@ tBTA_JV_STATUS BTA_JvL2capWrite(uint32_t handle, std::vector<uint8_t> data, 
 *                  called with BTA_JV_L2CAP_WRITE_EVT. Works for

 *                  fixed-channel connections

 *

 * Returns          BTA_JV_SUCCESS, if the request is being processed.

 *                  BTA_JV_FAILURE, otherwise.

 *

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

void BTA_JvL2capWriteFixed(uint16_t channel, const RawAddress& addr,

tBTA_JV_STATUS BTA_JvL2capWriteFixed(uint16_t channel, const RawAddress& addr,

                                     uint32_t req_id,

                                     tBTA_JV_L2CAP_CBACK* p_cback,

                           std::vector<uint8_t> data, uint32_t user_id) {

                                     uint8_t* p_data, uint16_t len,

                                     uint32_t user_id) {

  VLOG(2) << __func__;



  do_in_bta_thread(FROM_HERE, Bind(&bta_jv_l2cap_write_fixed, channel, addr,

                                   std::move(data), user_id, p_cback));

                                   req_id, p_data, len, user_id, p_cback));

  return BTA_JV_SUCCESS;

}



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

system/bta/jv/bta_jv_int.h

+5 −5
Original line number Diff line number Diff line
@@ -166,9 +166,9 @@ extern void bta_jv_l2cap_start_server( 
    uint32_t l2cap_socket_id);

extern void bta_jv_l2cap_stop_server(uint16_t local_psm,

                                     uint32_t l2cap_socket_id);

extern void bta_jv_l2cap_write(uint32_t handle,

                               const std::vector<uint8_t>& data,

                               uint32_t user_id, tBTA_JV_L2C_CB* p_cb);

extern void bta_jv_l2cap_write(uint32_t handle, uint32_t req_id,

                               uint8_t* p_data, uint16_t len, uint32_t user_id,

                               tBTA_JV_L2C_CB* p_cb);

extern void bta_jv_rfcomm_connect(tBTA_SEC sec_mask, tBTA_JV_ROLE role,

                                  uint8_t remote_scn,

                                  const RawAddress& peer_bd_addr,
@@ -193,8 +193,8 @@ extern void bta_jv_l2cap_start_server_le(uint16_t local_chan, 
                                         uint32_t l2cap_socket_id);

extern void bta_jv_l2cap_stop_server_le(uint16_t local_chan);

extern void bta_jv_l2cap_write_fixed(uint16_t channel, const RawAddress& addr,

                                     const std::vector<uint8_t>& data,

                                     uint32_t user_id,

                                     uint32_t req_id, uint8_t* p_data,

                                     uint16_t len, uint32_t user_id,

                                     tBTA_JV_L2CAP_CBACK* p_cback);

extern void bta_jv_l2cap_close_fixed(uint32_t handle);

system/btif/src/btif_sock_l2cap.cc

+33 −13
Original line number Diff line number Diff line
@@ -622,9 +622,13 @@ static void on_l2cap_outgoing_congest(tBTA_JV_L2CAP_CONG* p, uint32_t id) { 
  }

}



static void on_l2cap_write_done(uint16_t len, uint32_t id) {

static void on_l2cap_write_done(void* req_id, uint16_t len, uint32_t id) {

  l2cap_socket* sock;



  if (req_id != NULL) {

    osi_free(req_id);  // free the buffer

  }



  int app_uid = -1;



  std::unique_lock<std::mutex> lock(state_lock);
@@ -642,9 +646,13 @@ static void on_l2cap_write_done(uint16_t len, uint32_t id) { 
  uid_set_add_tx(uid_set, app_uid, len);

}



static void on_l2cap_write_fixed_done(uint16_t len, uint32_t id) {

static void on_l2cap_write_fixed_done(void* req_id, uint16_t len, uint32_t id) {

  l2cap_socket* sock;



  if (req_id != NULL) {

    osi_free(req_id);  // free the buffer

  }



  int app_uid = -1;

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

  sock = btsock_l2cap_find_by_id_l(id);
@@ -747,12 +755,14 @@ 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", l2cap_socket_id);

      on_l2cap_write_done(p_data->l2c_write.len, l2cap_socket_id);

      on_l2cap_write_done(p_data->l2c_write.p_data, p_data->l2c_write.len,

                          l2cap_socket_id);

      break;



    case BTA_JV_L2CAP_WRITE_FIXED_EVT:

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

      on_l2cap_write_fixed_done(p_data->l2c_write.len, l2cap_socket_id);

      on_l2cap_write_fixed_done(p_data->l2c_write_fixed.p_data,

                                p_data->l2c_write.len, l2cap_socket_id);

      break;



    case BTA_JV_L2CAP_CONG_EVT:
@@ -990,7 +1000,7 @@ 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)) {

        std::vector<uint8_t> buffer(L2CAP_MAX_SDU_LENGTH);

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

        /* 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,
@@ -1004,19 +1014,29 @@ void btsock_l2cap_signaled(int fd, int flags, uint32_t user_id) { 
         * in a tight loop, hence we risk the ioctl will return the total amount

         * of data in the buffer, which could be multiple 64kbyte chunks.

         * UPDATE: As the stack cannot handle 64kbyte buffers, the size is

         * reduced to around 8kbyte */

         * reduced to around 8kbyte - and using malloc for buffer allocation

         * here seems to be wrong

         * UPDATE: Since we are responsible for freeing the buffer in the

         * write_complete_ind, it is OK to use malloc. */

        ssize_t count;

        OSI_NO_INTR(count = recv(fd, buffer.data(), buffer.size(),

        OSI_NO_INTR(count = recv(fd, buffer, L2CAP_MAX_SDU_LENGTH,

                                 MSG_NOSIGNAL | MSG_DONTWAIT));

        APPL_TRACE_DEBUG("%s: %d bytes received from socket", __func__, count);



        buffer.resize((count == -1) ? 0 : count);

        APPL_TRACE_DEBUG(

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



        if (sock->fixed_chan) {

          BTA_JvL2capWriteFixed(sock->channel, sock->addr, btsock_l2cap_cbk,

                                std::move(buffer), user_id);

          if (BTA_JvL2capWriteFixed(sock->channel, sock->addr,

                                    PTR_TO_UINT(buffer), btsock_l2cap_cbk,

                                    buffer, count, user_id) != BTA_JV_SUCCESS) {

            // On fail, free the buffer

            on_l2cap_write_fixed_done(buffer, count, user_id);

          }

        } else {

          BTA_JvL2capWrite(sock->handle, std::move(buffer), user_id);

          if (BTA_JvL2capWrite(sock->handle, PTR_TO_UINT(buffer), buffer, count,

                               user_id) != BTA_JV_SUCCESS) {

            // On fail, free the buffer

            on_l2cap_write_done(buffer, count, user_id);

          }

        }

      }

    } else