bap: Fix not checking if request fits when grouping
authorLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
Thu, 26 Jan 2023 00:04:01 +0000 (16:04 -0800)
committerAyush Garg <ayush.garg@samsung.com>
Fri, 5 Jan 2024 10:11:34 +0000 (15:41 +0530)
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

index 4ba65cb..22f2e67 100644 (file)
@@ -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 */