* [PATCH v3] Bluetooth: btintel: Use skb_pull_data return for bounds check
@ 2026-05-17 6:01 Quan Sun
2026-05-17 8:24 ` [v3] " bluez.test.bot
2026-05-17 16:54 ` [PATCH v3] " Paul Menzel
0 siblings, 2 replies; 3+ messages in thread
From: Quan Sun @ 2026-05-17 6:01 UTC (permalink / raw)
To: marcel, luiz.dentz, kiran.k, linux-bluetooth; +Cc: Quan Sun
The length check at the top of btintel_print_fseq_info() verifies
the skb has at least 66 bytes (sizeof(u32) * 16 + 2), but the
function actually consumes 74 bytes (2 * 1 + 18 * 4). When firmware
returns a packet of exactly 66 bytes, the last two skb_pull_data()
calls return NULL, which is passed directly to get_unaligned_le32(),
resulting in a NULL pointer dereference.
Remove the insufficient length check and instead validate every
skb_pull_data() return value, branching to a malformed label that
logs the error and frees the skb.
Fixes: a7ba218a44aa ("Bluetooth: btintel: Print Firmware Sequencer information")
Signed-off-by: Quan Sun <2022090917019@std.uestc.edu.cn>
---
drivers/bluetooth/btintel.c | 106 +++++++++++++++++++++++++++---------
1 file changed, 79 insertions(+), 27 deletions(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 5e9cac090bd8..0c42ee53fe2e 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -3358,14 +3358,9 @@ void btintel_print_fseq_info(struct hci_dev *hdev)
return;
}
- if (skb->len < (sizeof(u32) * 16 + 2)) {
- bt_dev_dbg(hdev, "Malformed packet of length %u received",
- skb->len);
- kfree_skb(skb);
- return;
- }
-
p = skb_pull_data(skb, 1);
+ if (!p)
+ goto malformed;
if (*p) {
bt_dev_dbg(hdev, "Failed to get fseq status (0x%2.2x)", *p);
kfree_skb(skb);
@@ -3373,6 +3368,8 @@ void btintel_print_fseq_info(struct hci_dev *hdev)
}
p = skb_pull_data(skb, 1);
+ if (!p)
+ goto malformed;
switch (*p) {
case 0:
str = "Success";
@@ -3396,65 +3393,120 @@ void btintel_print_fseq_info(struct hci_dev *hdev)
bt_dev_info(hdev, "Fseq status: %s (0x%2.2x)", str, *p);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Reason: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Global version: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Installed version: 0x%8.8x", val);
- p = skb->data;
- skb_pull_data(skb, 4);
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
bt_dev_info(hdev, "Fseq executed: %2.2u.%2.2u.%2.2u.%2.2u", p[0], p[1],
p[2], p[3]);
- p = skb->data;
- skb_pull_data(skb, 4);
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
bt_dev_info(hdev, "Fseq BT Top: %2.2u.%2.2u.%2.2u.%2.2u", p[0], p[1],
p[2], p[3]);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq Top init version: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq Cnvio init version: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq MBX Wifi file version: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq BT version: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq Top reset address: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq MBX timeout: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq MBX ack: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq CNVi id: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq CNVr id: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq Error handle: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq Magic noalive indication: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq OTP version: 0x%8.8x", val);
- val = get_unaligned_le32(skb_pull_data(skb, 4));
+ p = skb_pull_data(skb, 4);
+ if (!p)
+ goto malformed;
+ val = get_unaligned_le32(p);
bt_dev_dbg(hdev, "Fseq MBX otp version: 0x%8.8x", val);
kfree_skb(skb);
+ return;
+
+malformed:
+ bt_dev_dbg(hdev, "Malformed packet received");
+ kfree_skb(skb);
}
EXPORT_SYMBOL_GPL(btintel_print_fseq_info);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* RE: [v3] Bluetooth: btintel: Use skb_pull_data return for bounds check
2026-05-17 6:01 [PATCH v3] Bluetooth: btintel: Use skb_pull_data return for bounds check Quan Sun
@ 2026-05-17 8:24 ` bluez.test.bot
2026-05-17 16:54 ` [PATCH v3] " Paul Menzel
1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2026-05-17 8:24 UTC (permalink / raw)
To: linux-bluetooth, 2022090917019
[-- Attachment #1: Type: text/plain, Size: 882 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1095975
---Test result---
Test Summary:
CheckPatch PASS 0.78 seconds
GitLint PASS 0.32 seconds
SubjectPrefix PASS 0.13 seconds
BuildKernel PASS 25.20 seconds
CheckAllWarning PASS 27.50 seconds
CheckSparse PASS 28.68 seconds
BuildKernel32 PASS 24.76 seconds
TestRunnerSetup PASS 525.64 seconds
IncrementalBuild PASS 23.95 seconds
https://github.com/bluez/bluetooth-next/pull/204
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] Bluetooth: btintel: Use skb_pull_data return for bounds check
2026-05-17 6:01 [PATCH v3] Bluetooth: btintel: Use skb_pull_data return for bounds check Quan Sun
2026-05-17 8:24 ` [v3] " bluez.test.bot
@ 2026-05-17 16:54 ` Paul Menzel
1 sibling, 0 replies; 3+ messages in thread
From: Paul Menzel @ 2026-05-17 16:54 UTC (permalink / raw)
To: Quan Sun; +Cc: marcel, luiz.dentz, kiran.k, linux-bluetooth
Dear Quan,
Thank you for your patch.
Am 17.05.26 um 08:01 schrieb Quan Sun:
> The length check at the top of btintel_print_fseq_info() verifies
> the skb has at least 66 bytes (sizeof(u32) * 16 + 2), but the
> function actually consumes 74 bytes (2 * 1 + 18 * 4). When firmware
> returns a packet of exactly 66 bytes, the last two skb_pull_data()
> calls return NULL, which is passed directly to get_unaligned_le32(),
> resulting in a NULL pointer dereference.
How did you find this? Manual review, or some tool?
> Remove the insufficient length check and instead validate every
> skb_pull_data() return value, branching to a malformed label that
> logs the error and frees the skb.
Where you able to test the new code on hardware?
> Fixes: a7ba218a44aa ("Bluetooth: btintel: Print Firmware Sequencer information")
> Signed-off-by: Quan Sun <2022090917019@std.uestc.edu.cn>
> ---
> drivers/bluetooth/btintel.c | 106 +++++++++++++++++++++++++++---------
> 1 file changed, 79 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 5e9cac090bd8..0c42ee53fe2e 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -3358,14 +3358,9 @@ void btintel_print_fseq_info(struct hci_dev *hdev)
> return;
> }
>
> - if (skb->len < (sizeof(u32) * 16 + 2)) {
> - bt_dev_dbg(hdev, "Malformed packet of length %u received",
> - skb->len);
> - kfree_skb(skb);
> - return;
> - }
> -
> p = skb_pull_data(skb, 1);
> + if (!p)
> + goto malformed;
> if (*p) {
> bt_dev_dbg(hdev, "Failed to get fseq status (0x%2.2x)", *p);
> kfree_skb(skb);
> @@ -3373,6 +3368,8 @@ void btintel_print_fseq_info(struct hci_dev *hdev)
> }
>
> p = skb_pull_data(skb, 1);
> + if (!p)
> + goto malformed;
> switch (*p) {
> case 0:
> str = "Success";
> @@ -3396,65 +3393,120 @@ void btintel_print_fseq_info(struct hci_dev *hdev)
>
> bt_dev_info(hdev, "Fseq status: %s (0x%2.2x)", str, *p);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Reason: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Global version: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Installed version: 0x%8.8x", val);
>
> - p = skb->data;
> - skb_pull_data(skb, 4);
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> bt_dev_info(hdev, "Fseq executed: %2.2u.%2.2u.%2.2u.%2.2u", p[0], p[1],
> p[2], p[3]);
>
> - p = skb->data;
> - skb_pull_data(skb, 4);
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> bt_dev_info(hdev, "Fseq BT Top: %2.2u.%2.2u.%2.2u.%2.2u", p[0], p[1],
> p[2], p[3]);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq Top init version: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq Cnvio init version: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq MBX Wifi file version: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq BT version: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq Top reset address: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq MBX timeout: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq MBX ack: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq CNVi id: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq CNVr id: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq Error handle: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq Magic noalive indication: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq OTP version: 0x%8.8x", val);
>
> - val = get_unaligned_le32(skb_pull_data(skb, 4));
> + p = skb_pull_data(skb, 4);
> + if (!p)
> + goto malformed;
> + val = get_unaligned_le32(p);
> bt_dev_dbg(hdev, "Fseq MBX otp version: 0x%8.8x", val);
>
> kfree_skb(skb);
> + return;
> +
> +malformed:
> + bt_dev_dbg(hdev, "Malformed packet received");
It’d be great to log more information, wouldn’t it?
> + kfree_skb(skb);
> }
> EXPORT_SYMBOL_GPL(btintel_print_fseq_info);
>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-17 16:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-17 6:01 [PATCH v3] Bluetooth: btintel: Use skb_pull_data return for bounds check Quan Sun
2026-05-17 8:24 ` [v3] " bluez.test.bot
2026-05-17 16:54 ` [PATCH v3] " Paul Menzel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox