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

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

Prevent LE CoC from dropping data when remote MPS>8087

When using LE Coc through Java socket, one can read MPS from
getMaxTransmitPacketSize(). If this value is bigger than
local device L2CAP_MAX_SDU_LENGTH, data sent to remote will be
truncated. This patch fixes that by using properly big buffer for
receiving data from socket on native side.

Test: connect with device with MPS bigger than MPS, alternatively set
L2CAP_MAX_SDU_LENGTH to i.e. 30 on device during test and try to write
"remote MPS" of bytes to remote device.
Bug: 68359837
Change-Id: I02bef80f0dd0f0d6850704ac7787c5f3f5b9b3ab
parent 6ebe7717
Loading
Loading
Loading
Loading
+30 −28
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@
#include "btu.h"
#include "hcimsgs.h"
#include "l2c_api.h"
#include "l2c_int.h"
#include "l2cdefs.h"
#include "port_api.h"
#include "sdp_api.h"
@@ -76,13 +77,13 @@ typedef struct l2cap_socket {
  struct packet* first_packet;  // fist packet to be delivered to app
  struct packet* last_packet;   // last packet to be delivered to app

  fixed_queue_t* incoming_que;    // data that came in but has not yet been read
  unsigned fixed_chan : 1;        // fixed channel (or psm?)
  unsigned server : 1;            // is a server? (or connecting?)
  unsigned connected : 1;         // is connected?
  unsigned outgoing_congest : 1;  // should we hold?
  unsigned server_psm_sent : 1;   // The server shall only send PSM once.
  bool is_le_coc;                 // is le connection oriented channel?
  uint16_t mps;
} l2cap_socket;

static bt_status_t btSock_start_l2cap_server_l(l2cap_socket* sock);
@@ -308,6 +309,8 @@ static l2cap_socket* btsock_l2cap_alloc_l(const char* name,
  sock->first_packet = NULL;
  sock->last_packet = NULL;

  sock->mps = L2CAP_LE_MIN_MPS;

  sock->next = socks;
  sock->prev = NULL;
  if (socks) socks->prev = sock;
@@ -424,8 +427,7 @@ static void on_cl_l2cap_init(tBTA_JV_L2CAP_CL_INIT* p_init, uint32_t id) {

/**
 * Here we allocate a new sock instance to mimic the BluetoothSocket. The socket
 * will be a clone
 * of the sock representing the BluetoothServerSocket.
 * will be a clone of the sock representing the BluetoothServerSocket.
 * */
static void on_srv_l2cap_psm_connect_l(tBTA_JV_L2CAP_OPEN* p_open,
                                       l2cap_socket* sock) {
@@ -443,6 +445,7 @@ static void on_srv_l2cap_psm_connect_l(tBTA_JV_L2CAP_OPEN* p_open,
  sock->handle =
      -1; /* We should no longer associate this handle with the server socket */
  accept_rs->is_le_coc = sock->is_le_coc;
  accept_rs->mps = sock->mps;

  /* Swap IDs to hand over the GAP connection to the accepted socket, and start
     a new server on
@@ -493,6 +496,7 @@ static void on_srv_l2cap_le_connect_l(tBTA_JV_L2CAP_LE_OPEN* p_open,
    accept_rs->fixed_chan = sock->fixed_chan;
    accept_rs->channel = sock->channel;
    accept_rs->app_uid = sock->app_uid;
    accept_rs->mps = sock->mps;

    // if we do not set a callback, this socket will be dropped */
    *(p_open->p_p_cback) = (void*)btsock_l2cap_cbk;
@@ -572,6 +576,7 @@ static void on_l2cap_connect(tBTA_JV* p_data, uint32_t id) {
    return;
  }

  sock->mps = le_open->tx_mtu;
  if (sock->fixed_chan && le_open->status == BTA_JV_SUCCESS) {
    if (!sock->server)
      on_cl_l2cap_le_connect_l(le_open, sock);
@@ -984,45 +989,42 @@ static bool flush_incoming_que_on_wr_signal_l(l2cap_socket* sock) {
}

void btsock_l2cap_signaled(int fd, int flags, uint32_t user_id) {
  l2cap_socket* sock;
  char drop_it = false;

  /* We use MSG_DONTWAIT when sending data to JAVA, hence it can be accepted to
   * hold the lock. */
  std::unique_lock<std::mutex> lock(state_lock);
  sock = btsock_l2cap_find_by_id_l(user_id);
  l2cap_socket* sock = btsock_l2cap_find_by_id_l(user_id);
  if (!sock) return;

  if ((flags & SOCK_THREAD_FD_RD) && !sock->server) {
    // app sending data
    if (sock->connected) {
      int size = 0;
      bool ioctl_success = ioctl(sock->our_fd, FIONREAD, &size) == 0;
      if (!(flags & SOCK_THREAD_FD_EXCEPTION) || (ioctl_success && size)) {
        /* FIONREAD return number of bytes that are immediately available for
           reading, might be bigger than awaiting packet.

           BluetoothSocket.write(...) guarantees that any packet send to this
           socket is broken into pieces no bigger than MPS bytes (as requested
           by BT spec). */
        size = std::min(size, (int)sock->mps);

      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);
        uint8_t* buffer = (uint8_t*)osi_malloc(size);
        /* 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,
         * hence even if we only use a fraction of the memory it should not
         * block for others to use the memory. As the definition of
         * ioctl(FIONREAD) do not clearly define what value will be returned if
         * multiple messages are written to the socket before any message is
         * read from the socket, we could potentially risk to allocate way more
         * memory than needed. One of the use cases for this socket is obex
         * where multiple 64kbyte messages are typically written to the socket
         * 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 - 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. */
         * at the time. */
        ssize_t count;
        OSI_NO_INTR(count = recv(fd, buffer, L2CAP_MAX_SDU_LENGTH,
                                 MSG_NOSIGNAL | MSG_DONTWAIT));
        APPL_TRACE_DEBUG(
            "btsock_l2cap_signaled - %d bytes received from socket", count);
        OSI_NO_INTR(count = recv(fd, buffer, size,
                                 MSG_NOSIGNAL | MSG_DONTWAIT | MSG_TRUNC));
        if (count > L2CAP_LE_MAX_MPS) {
          /* This can't happen thanks to check in BluetoothSocket.java but leave
           * this in case this socket is ever used anywhere else*/
          LOG(ERROR) << "recv more than MPS. Data will be lost: " << count;
          count = L2CAP_LE_MAX_MPS;
        }

        DVLOG(2) << __func__ << ": bytes received from socket: " << count;

        if (sock->fixed_chan) {
          if (BTA_JvL2capWriteFixed(sock->channel, sock->addr,