Use vector instead of raw pointer in l2cap socket write handling (cd86d76d) · Commits · e / os / android_packages_modules_Bluetooth

system/bta/include/bta_jv_api.h

+4 −11

Original line number	Diff line number	Diff line
		@@ -244,9 +244,7 @@ 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;

		@@ -255,8 +253,6 @@ 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;
		@@ -680,8 +676,7 @@ 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, uint32_t req_id,
		uint8_t* p_data, uint16_t len,
		tBTA_JV_STATUS BTA_JvL2capWrite(uint32_t handle, std::vector<uint8_t> data,
		uint32_t user_id);

		/*******************************************************************************
		@@ -697,11 +692,9 @@ tBTA_JV_STATUS BTA_JvL2capWrite(uint32_t handle, uint32_t req_id,
		* BTA_JV_FAILURE, otherwise.
		*
		******************************************************************************/
		tBTA_JV_STATUS BTA_JvL2capWriteFixed(uint16_t channel, const RawAddress& addr,
		uint32_t req_id,
		void BTA_JvL2capWriteFixed(uint16_t channel, const RawAddress& addr,
		tBTA_JV_L2CAP_CBACK* p_cback,
		uint8_t* p_data, uint16_t len,
		uint32_t user_id);
		std::vector<uint8_t> data, uint32_t user_id);

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

system/bta/jv/bta_jv_act.cc

+10 −16

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, uint32_t req_id, uint8_t* p_data,
		uint16_t len, uint32_t user_id, tBTA_JV_L2C_CB* p_cb) {
		void bta_jv_l2cap_write(uint32_t handle, const std::vector<uint8_t>& data,
		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,11 +1144,7 @@ void bta_jv_l2cap_write(uint32_t handle, uint32_t req_id, uint8_t* p_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. 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.*/
		* disconnect. If p_cback is cleared, we simply discard the data.*/
		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
		@@ -1160,13 +1156,12 @@ void bta_jv_l2cap_write(uint32_t handle, uint32_t req_id, uint8_t* p_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, p_data, len, &evt_data.len)) {
		BT_PASS ==
		GAP_ConnWriteData(handle, data.data(), data.size(), &evt_data.len)) {
		evt_data.status = BTA_JV_SUCCESS;
		}
		tBTA_JV bta_jv;
		@@ -1176,19 +1171,18 @@ void bta_jv_l2cap_write(uint32_t handle, uint32_t req_id, uint8_t* p_data,

		/* Write data to an L2CAP connection using Fixed channels */
		void bta_jv_l2cap_write_fixed(uint16_t channel, const RawAddress& addr,
		uint32_t req_id, uint8_t* p_data, uint16_t len,
		const std::vector<uint8_t>& data,
		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) + len + L2CAP_MIN_OFFSET);
		memcpy(((uint8_t*)(msg + 1)) + L2CAP_MIN_OFFSET, p_data, len);
		msg->len = len;
		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();
		msg->offset = L2CAP_MIN_OFFSET;

		L2CA_SendFixedChnlData(channel, addr, msg);

system/bta/jv/bta_jv_api.cc

+7 −15

Original line number	Diff line number	Diff line
		@@ -512,16 +512,15 @@ 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, uint32_t req_id,
		uint8_t* p_data, uint16_t len,
		tBTA_JV_STATUS BTA_JvL2capWrite(uint32_t handle, std::vector<uint8_t> data,
		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, req_id, p_data,
		len, user_id, &bta_jv_cb.l2c_cb[handle]));
		do_in_bta_thread(FROM_HERE, Bind(&bta_jv_l2cap_write, handle, std::move(data),
		user_id, &bta_jv_cb.l2c_cb[handle]));
		return BTA_JV_SUCCESS;
		}

		@@ -534,20 +533,13 @@ tBTA_JV_STATUS BTA_JvL2capWrite(uint32_t handle, uint32_t req_id,
		* 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.
		*
		******************************************************************************/
		tBTA_JV_STATUS BTA_JvL2capWriteFixed(uint16_t channel, const RawAddress& addr,
		uint32_t req_id,
		void BTA_JvL2capWriteFixed(uint16_t channel, const RawAddress& addr,
		tBTA_JV_L2CAP_CBACK* p_cback,
		uint8_t* p_data, uint16_t len,
		uint32_t user_id) {
		std::vector<uint8_t> data, uint32_t user_id) {
		VLOG(2) << __func__;

		do_in_bta_thread(FROM_HERE, Bind(&bta_jv_l2cap_write_fixed, channel, addr,
		req_id, p_data, len, user_id, p_cback));
		return BTA_JV_SUCCESS;
		std::move(data), user_id, p_cback));
		}

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

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, 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_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_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,
		uint32_t req_id, uint8_t* p_data,
		uint16_t len, uint32_t user_id,
		const std::vector<uint8_t>& data,
		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

+13 −33

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

		static void on_l2cap_write_done(void* req_id, uint16_t len, uint32_t id) {
		static void on_l2cap_write_done(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);
		@@ -646,13 +642,9 @@ static void on_l2cap_write_done(void* req_id, uint16_t len, uint32_t id) {
		uid_set_add_tx(uid_set, app_uid, len);
		}

		static void on_l2cap_write_fixed_done(void* req_id, uint16_t len, uint32_t id) {
		static void on_l2cap_write_fixed_done(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);
		@@ -755,14 +747,12 @@ 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.p_data, p_data->l2c_write.len,
		l2cap_socket_id);
		on_l2cap_write_done(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_fixed.p_data,
		p_data->l2c_write.len, l2cap_socket_id);
		on_l2cap_write_fixed_done(p_data->l2c_write.len, l2cap_socket_id);
		break;

		case BTA_JV_L2CAP_CONG_EVT:
		@@ -1000,7 +990,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)) {
		uint8_t* buffer = (uint8_t*)osi_malloc(L2CAP_MAX_SDU_LENGTH);
		std::vector<uint8_t> buffer(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,
		@@ -1014,29 +1004,19 @@ 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 - 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. */
		* reduced to around 8kbyte */
		ssize_t count;
		OSI_NO_INTR(count = recv(fd, buffer, L2CAP_MAX_SDU_LENGTH,
		OSI_NO_INTR(count = recv(fd, buffer.data(), buffer.size(),
		MSG_NOSIGNAL \| MSG_DONTWAIT));
		APPL_TRACE_DEBUG(
		"btsock_l2cap_signaled - %d bytes received from socket", count);
		APPL_TRACE_DEBUG("%s: %d bytes received from socket", __func__, count);

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

		if (sock->fixed_chan) {
		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);
		}
		BTA_JvL2capWriteFixed(sock->channel, sock->addr, btsock_l2cap_cbk,
		std::move(buffer), user_id);
		} else {
		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);
		}
		BTA_JvL2capWrite(sock->handle, std::move(buffer), user_id);
		}
		}
		} else