Bluetooth: btusb: use skb_pull to avoid unsafe access in QCA dump handling
authorEn-Wei Wu <en-wei.wu@canonical.com>
Thu, 8 May 2025 14:15:20 +0000 (22:15 +0800)
committerLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
Thu, 22 May 2025 17:05:39 +0000 (13:05 -0400)
Use skb_pull() and skb_pull_data() to safely parse QCA dump packets.

This avoids direct pointer math on skb->data, which could lead to
invalid access if the packet is shorter than expected.

Fixes: 20981ce2d5a5 ("Bluetooth: btusb: Add WCN6855 devcoredump support")
Signed-off-by: En-Wei Wu <en-wei.wu@canonical.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
drivers/bluetooth/btusb.c

index 357b18dae8deb3928ebdc312a73e9afc4618ebdb..ef44817cad34ecbc03610cef183665b9085f5b6f 100644 (file)
@@ -2979,9 +2979,8 @@ static void btusb_coredump_qca(struct hci_dev *hdev)
 static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
 {
        int ret = 0;
+       unsigned int skip = 0;
        u8 pkt_type;
-       u8 *sk_ptr;
-       unsigned int sk_len;
        u16 seqno;
        u32 dump_size;
 
@@ -2990,18 +2989,13 @@ static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
        struct usb_device *udev = btdata->udev;
 
        pkt_type = hci_skb_pkt_type(skb);
-       sk_ptr = skb->data;
-       sk_len = skb->len;
+       skip = sizeof(struct hci_event_hdr);
+       if (pkt_type == HCI_ACLDATA_PKT)
+               skip += sizeof(struct hci_acl_hdr);
 
-       if (pkt_type == HCI_ACLDATA_PKT) {
-               sk_ptr += HCI_ACL_HDR_SIZE;
-               sk_len -= HCI_ACL_HDR_SIZE;
-       }
-
-       sk_ptr += HCI_EVENT_HDR_SIZE;
-       sk_len -= HCI_EVENT_HDR_SIZE;
+       skb_pull(skb, skip);
+       dump_hdr = (struct qca_dump_hdr *)skb->data;
 
-       dump_hdr = (struct qca_dump_hdr *)sk_ptr;
        seqno = le16_to_cpu(dump_hdr->seqno);
        if (seqno == 0) {
                set_bit(BTUSB_HW_SSR_ACTIVE, &btdata->flags);
@@ -3021,16 +3015,15 @@ static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
 
                btdata->qca_dump.ram_dump_size = dump_size;
                btdata->qca_dump.ram_dump_seqno = 0;
-               sk_ptr += offsetof(struct qca_dump_hdr, data0);
-               sk_len -= offsetof(struct qca_dump_hdr, data0);
+
+               skb_pull(skb, offsetof(struct qca_dump_hdr, data0));
 
                usb_disable_autosuspend(udev);
                bt_dev_info(hdev, "%s memdump size(%u)\n",
                            (pkt_type == HCI_ACLDATA_PKT) ? "ACL" : "event",
                            dump_size);
        } else {
-               sk_ptr += offsetof(struct qca_dump_hdr, data);
-               sk_len -= offsetof(struct qca_dump_hdr, data);
+               skb_pull(skb, offsetof(struct qca_dump_hdr, data));
        }
 
        if (!btdata->qca_dump.ram_dump_size) {
@@ -3050,7 +3043,6 @@ static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
                return ret;
        }
 
-       skb_pull(skb, skb->len - sk_len);
        hci_devcd_append(hdev, skb);
        btdata->qca_dump.ram_dump_seqno++;
        if (seqno == QCA_LAST_SEQUENCE_NUM) {
@@ -3078,68 +3070,58 @@ out:
 /* Return: true if the ACL packet is a dump packet, false otherwise. */
 static bool acl_pkt_is_dump_qca(struct hci_dev *hdev, struct sk_buff *skb)
 {
-       u8 *sk_ptr;
-       unsigned int sk_len;
-
        struct hci_event_hdr *event_hdr;
        struct hci_acl_hdr *acl_hdr;
        struct qca_dump_hdr *dump_hdr;
+       struct sk_buff *clone = skb_clone(skb, GFP_ATOMIC);
+       bool is_dump = false;
 
-       sk_ptr = skb->data;
-       sk_len = skb->len;
-
-       acl_hdr = hci_acl_hdr(skb);
-       if (le16_to_cpu(acl_hdr->handle) != QCA_MEMDUMP_ACL_HANDLE)
+       if (!clone)
                return false;
 
-       sk_ptr += HCI_ACL_HDR_SIZE;
-       sk_len -= HCI_ACL_HDR_SIZE;
-       event_hdr = (struct hci_event_hdr *)sk_ptr;
-
-       if ((event_hdr->evt != HCI_VENDOR_PKT) ||
-           (event_hdr->plen != (sk_len - HCI_EVENT_HDR_SIZE)))
-               return false;
+       acl_hdr = skb_pull_data(clone, sizeof(*acl_hdr));
+       if (!acl_hdr || (le16_to_cpu(acl_hdr->handle) != QCA_MEMDUMP_ACL_HANDLE))
+               goto out;
 
-       sk_ptr += HCI_EVENT_HDR_SIZE;
-       sk_len -= HCI_EVENT_HDR_SIZE;
+       event_hdr = skb_pull_data(clone, sizeof(*event_hdr));
+       if (!event_hdr || (event_hdr->evt != HCI_VENDOR_PKT))
+               goto out;
 
-       dump_hdr = (struct qca_dump_hdr *)sk_ptr;
-       if ((sk_len < offsetof(struct qca_dump_hdr, data)) ||
-           (dump_hdr->vse_class != QCA_MEMDUMP_VSE_CLASS) ||
-           (dump_hdr->msg_type != QCA_MEMDUMP_MSG_TYPE))
-               return false;
+       dump_hdr = skb_pull_data(clone, sizeof(*dump_hdr));
+       if (!dump_hdr || (dump_hdr->vse_class != QCA_MEMDUMP_VSE_CLASS) ||
+          (dump_hdr->msg_type != QCA_MEMDUMP_MSG_TYPE))
+               goto out;
 
-       return true;
+       is_dump = true;
+out:
+       consume_skb(clone);
+       return is_dump;
 }
 
 /* Return: true if the event packet is a dump packet, false otherwise. */
 static bool evt_pkt_is_dump_qca(struct hci_dev *hdev, struct sk_buff *skb)
 {
-       u8 *sk_ptr;
-       unsigned int sk_len;
-
        struct hci_event_hdr *event_hdr;
        struct qca_dump_hdr *dump_hdr;
+       struct sk_buff *clone = skb_clone(skb, GFP_ATOMIC);
+       bool is_dump = false;
 
-       sk_ptr = skb->data;
-       sk_len = skb->len;
-
-       event_hdr = hci_event_hdr(skb);
-
-       if ((event_hdr->evt != HCI_VENDOR_PKT)
-           || (event_hdr->plen != (sk_len - HCI_EVENT_HDR_SIZE)))
+       if (!clone)
                return false;
 
-       sk_ptr += HCI_EVENT_HDR_SIZE;
-       sk_len -= HCI_EVENT_HDR_SIZE;
+       event_hdr = skb_pull_data(clone, sizeof(*event_hdr));
+       if (!event_hdr || (event_hdr->evt != HCI_VENDOR_PKT))
+               goto out;
 
-       dump_hdr = (struct qca_dump_hdr *)sk_ptr;
-       if ((sk_len < offsetof(struct qca_dump_hdr, data)) ||
-           (dump_hdr->vse_class != QCA_MEMDUMP_VSE_CLASS) ||
-           (dump_hdr->msg_type != QCA_MEMDUMP_MSG_TYPE))
-               return false;
+       dump_hdr = skb_pull_data(clone, sizeof(*dump_hdr));
+       if (!dump_hdr || (dump_hdr->vse_class != QCA_MEMDUMP_VSE_CLASS) ||
+          (dump_hdr->msg_type != QCA_MEMDUMP_MSG_TYPE))
+               goto out;
 
-       return true;
+       is_dump = true;
+out:
+       consume_skb(clone);
+       return is_dump;
 }
 
 static int btusb_recv_acl_qca(struct hci_dev *hdev, struct sk_buff *skb)