diff mbox series

[v3,4/5] bap: Fix not checking if request fits when grouping

Message ID 20230126201242.4110305-4-luiz.dentz@gmail.com (mailing list archive)
State Accepted
Commit 814e3535a1bc165e05d91c470b38e835c5006ac9
Headers show
Series [v3,1/5] shared/bap: Fix not detaching streams when PAC is removed | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch warning WARNING:REPEATED_WORD: Possible repeated word: 'that' #86: without attempt to check that that would fit in the ATT MTU causing the /github/workspace/src/src/13117701.patch total: 0 errors, 1 warnings, 69 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13117701.patch has style problems, please review. NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
tedd_an/GitLint success Gitlint PASS
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Luiz Augusto von Dentz Jan. 26, 2023, 8:12 p.m. UTC
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

When grouping requests with the same opcode the code was queueing them
without attempt to check that that would fit in the ATT MTU causing the
following trace:

stack-buffer-overflow on address 0x7fffdba951f0 at pc 0x7fc15fc49d21 bp
0x7fffdba95020 sp 0x7fffdba947d0
WRITE of size 9 at 0x7fffdba951f0 thread T0
   #0 0x7fc15fc49d20 in __interceptor_memcpy
(/lib64/libasan.so.8+0x49d20)
   #1 0x71f698 in util_iov_push_mem src/shared/util.c:266
   #2 0x7b9312 in append_group src/shared/bap.c:3424
   #3 0x71ba01 in queue_foreach src/shared/queue.c:207
   #4 0x7b9b66 in bap_send src/shared/bap.c:3459
   #5 0x7ba594 in bap_process_queue src/shared/bap.c:351

Fixes: https://github.com/bluez/bluez/issues/457#issuecomment-1403924708
---
 src/shared/bap.c | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 4ba65cbaa8f9..22f2e67146fb 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -3425,20 +3425,34 @@  static void append_group(void *data, void *user_data)
 					req->iov[i].iov_base);
 }
 
+static uint16_t bap_req_len(struct bt_bap_req *req)
+{
+	uint16_t len = 0;
+	size_t i;
+	const struct queue_entry *e;
+
+	for (i = 0; i < req->len; i++)
+		len += req->iov[i].iov_len;
+
+	e = queue_get_entries(req->group);
+	for (; e; e = e->next)
+		len += bap_req_len(e->data);
+
+	return len;
+}
+
 static bool bap_send(struct bt_bap *bap, struct bt_bap_req *req)
 {
 	struct bt_ascs *ascs = bap_get_ascs(bap);
 	int ret;
 	uint16_t handle;
-	uint8_t buf[64];
 	struct bt_ascs_ase_hdr hdr;
-	struct iovec iov  = {
-		.iov_base = buf,
-		.iov_len = 0,
-	};
+	struct iovec iov;
 	size_t i;
 
-	DBG(bap, "req %p", req);
+	iov.iov_len = sizeof(hdr) + bap_req_len(req);
+
+	DBG(bap, "req %p len %u", req, iov.iov_len);
 
 	if (!gatt_db_attribute_get_char_data(ascs->ase_cp, NULL, &handle,
 						NULL, NULL, NULL)) {
@@ -3446,6 +3460,9 @@  static bool bap_send(struct bt_bap *bap, struct bt_bap_req *req)
 		return false;
 	}
 
+	iov.iov_base = alloca(iov.iov_len);
+	iov.iov_len = 0;
+
 	hdr.op = req->op;
 	hdr.num = 1 + queue_length(req->group);
 
@@ -3531,9 +3548,19 @@  static bool bap_queue_req(struct bt_bap *bap, struct bt_bap_req *req)
 {
 	struct bt_bap_req *pend;
 	struct queue *queue;
+	struct bt_att *att = bt_bap_get_att(bap);
+	uint16_t mtu = bt_att_get_mtu(att);
+	uint16_t len = 2 + bap_req_len(req);
+
+	if (len > mtu) {
+		DBG(bap, "Unable to queue request: req len %u > %u mtu", len,
+									mtu);
+		return false;
+	}
 
 	pend = queue_find(bap->reqs, match_req, req);
-	if (pend) {
+	/* Check if req can be grouped together and it fits in the MTU */
+	if (pend && (bap_req_len(pend) + len < mtu)) {
 		if (!pend->group)
 			pend->group = queue_new();
 		/* Group requests with the same opcode */