| Context |
Check |
Description |
| tedd_an/pre-ci_am |
success
|
Success
|
| tedd_an/SubjectPrefix |
success
|
Gitlint PASS
|
| tedd_an/BuildKernel |
success
|
BuildKernel PASS
|
| tedd_an/CheckAllWarning |
success
|
CheckAllWarning PASS
|
| tedd_an/CheckSparse |
success
|
CheckSparse PASS
|
| tedd_an/BuildKernel32 |
success
|
BuildKernel32 PASS
|
| tedd_an/TestRunnerSetup |
success
|
TestRunnerSetup PASS
|
| tedd_an/TestRunner_l2cap-tester |
success
|
TestRunner PASS
|
| tedd_an/TestRunner_iso-tester |
fail
|
TestRunner_iso-tester: WARNING: possible circular locking dependency detected
|
| tedd_an/TestRunner_bnep-tester |
success
|
TestRunner PASS
|
| tedd_an/TestRunner_mgmt-tester |
success
|
TestRunner PASS
|
| tedd_an/TestRunner_rfcomm-tester |
success
|
TestRunner PASS
|
| tedd_an/TestRunner_sco-tester |
success
|
TestRunner PASS
|
| tedd_an/TestRunner_ioctl-tester |
success
|
TestRunner PASS
|
| tedd_an/TestRunner_mesh-tester |
success
|
TestRunner PASS
|
| tedd_an/TestRunner_smp-tester |
success
|
TestRunner PASS
|
| tedd_an/TestRunner_userchan-tester |
success
|
TestRunner PASS
|
@@ -2930,22 +2930,16 @@ static void btusb_coredump_qca(struct hci_dev *hdev)
bt_dev_err(hdev, "%s: triggle crash failed (%d)", __func__, err);
}
-/*
- * ==0: not a dump pkt.
- * < 0: fails to handle a dump pkt
- * > 0: otherwise.
- */
+/* Return: 0 on success, negative errno on failure. */
static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
{
- int ret = 1;
+ int ret = 0;
u8 pkt_type;
u8 *sk_ptr;
unsigned int sk_len;
u16 seqno;
u32 dump_size;
- struct hci_event_hdr *event_hdr;
- struct hci_acl_hdr *acl_hdr;
struct qca_dump_hdr *dump_hdr;
struct btusb_data *btdata = hci_get_drvdata(hdev);
struct usb_device *udev = btdata->udev;
@@ -2955,30 +2949,14 @@ static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
sk_len = skb->len;
if (pkt_type == HCI_ACLDATA_PKT) {
- acl_hdr = hci_acl_hdr(skb);
- if (le16_to_cpu(acl_hdr->handle) != QCA_MEMDUMP_ACL_HANDLE)
- return 0;
sk_ptr += HCI_ACL_HDR_SIZE;
sk_len -= HCI_ACL_HDR_SIZE;
- event_hdr = (struct hci_event_hdr *)sk_ptr;
- } else {
- event_hdr = hci_event_hdr(skb);
}
- if ((event_hdr->evt != HCI_VENDOR_PKT)
- || (event_hdr->plen != (sk_len - HCI_EVENT_HDR_SIZE)))
- return 0;
-
sk_ptr += HCI_EVENT_HDR_SIZE;
sk_len -= HCI_EVENT_HDR_SIZE;
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 0;
-
- /*it is dump pkt now*/
seqno = le16_to_cpu(dump_hdr->seqno);
if (seqno == 0) {
set_bit(BTUSB_HW_SSR_ACTIVE, &btdata->flags);
@@ -3052,17 +3030,80 @@ static int handle_dump_pkt_qca(struct hci_dev *hdev, struct sk_buff *skb)
return ret;
}
+/* Return: true if the ACL packet is a dump packet, false otherwise. */
+static bool btqca_acl_pkt_is_dump(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct hci_event_hdr *event_hdr;
+ struct hci_acl_hdr *acl_hdr;
+ struct qca_dump_hdr *dump_hdr;
+
+ 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)
+ 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;
+
+ sk_ptr += HCI_EVENT_HDR_SIZE;
+ sk_len -= HCI_EVENT_HDR_SIZE;
+
+ 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;
+
+ return true;
+}
+
+/* Return: true if the event packet is a dump packet, false otherwise. */
+static bool btqca_event_pkt_is_dump(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;
+
+ 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)))
+ return false;
+
+ sk_ptr += HCI_EVENT_HDR_SIZE;
+ sk_len -= HCI_EVENT_HDR_SIZE;
+
+ 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;
+
+ return true;
+}
+
static int btusb_recv_acl_qca(struct hci_dev *hdev, struct sk_buff *skb)
{
- if (handle_dump_pkt_qca(hdev, skb))
- return 0;
+ if (btqca_acl_pkt_is_dump(hdev, skb))
+ return handle_dump_pkt_qca(hdev, skb);
return hci_recv_frame(hdev, skb);
}
static int btusb_recv_evt_qca(struct hci_dev *hdev, struct sk_buff *skb)
{
- if (handle_dump_pkt_qca(hdev, skb))
- return 0;
+ if (btqca_event_pkt_is_dump(hdev, skb))
+ return handle_dump_pkt_qca(hdev, skb);
return hci_recv_frame(hdev, skb);
}
The WCN7851 (0489:e0f3) Bluetooth controller supports firmware crash dump collection through devcoredump. During this process, the crash dump data is queued to a dump queue as skb for further processing. A NULL pointer dereference occurs in skb_dequeue() when processing the dump queue due to improper return value handling: [ 93.672166] Bluetooth: hci0: ACL memdump size(589824) [ 93.672475] BUG: kernel NULL pointer dereference, address: 0000000000000008 [ 93.672517] Workqueue: hci0 hci_devcd_rx [bluetooth] [ 93.672598] RIP: 0010:skb_dequeue+0x50/0x80 The issue stems from handle_dump_pkt_qca() returning the wrong value on success. It currently returns the value from hci_devcd_init() (0 on success), but callers expect > 0 to indicate successful dump handling. This causes hci_recv_frame() to free the skb while it's still queued for dump processing, leading to the NULL pointer dereference when hci_devcd_rx() tries to dequeue it. Fix this by: 1. Making handle_dump_pkt_qca() return 0 on success and negative errno on failure, consistent with other kernel interfaces 2. Splitting dump packet detection into separate ACL and event functions for better code organization and maintainability This prevents premature skb freeing by ensuring proper handling of dump packets. Fixes: 20981ce2d5a5 ("Bluetooth: btusb: Add WCN6855 devcoredump support") Signed-off-by: En-Wei Wu <en-wei.wu@canonical.com> --- drivers/bluetooth/btusb.c | 97 ++++++++++++++++++++++++++++----------- 1 file changed, 69 insertions(+), 28 deletions(-)