[PATCH] Bluetooth: virtio_bt: clamp rx length before skb_put

[PATCH] Bluetooth: virtio_bt: clamp rx length before skb_put

[Michael Bommarito]

virtbt_rx_work() calls skb_put(skb, len) where len comes directly
from virtqueue_get_buf() with no validation against the skb we
posted.  The RX skb is allocated as alloc_skb(1000) in
virtbt_add_inbuf().  A malicious or buggy virtio-bt backend that
reports used.len larger than the skb's tailroom causes skb_put() to
call skb_over_panic() in net/core/skbuff.c, which triggers
BUG() and panics the guest.

Reproduced on a QEMU 9.0 whose virtio-bt backend reports
used.len = 4096 into a 1000-byte rx skb:

  skbuff: skb_over_panic: text:ffffffff83958e84 len:4096 put:4096
      head:ffff88800c071000 data:ffff88800c071000 tail:0x1000
      end:0x6c0 dev:<NULL>
  ------------[ cut here ]------------
  kernel BUG at net/core/skbuff.c:214!
  Call Trace:
   skb_panic+0x160/0x162
   skb_put.cold+0x31/0x31
   virtbt_rx_work+0x94/0x250
   process_one_work+0x80d/0x1510
   worker_thread+0x4af/0xd20
   kthread+0x2cc/0x3a0

Reject any len that exceeds skb_tailroom().  Drop the skb on the
error path; virtbt_add_inbuf() reposts a fresh one for the next
iteration.  With the check in place the same harness runs without
BUG(); the driver logs "rx reply len %u exceeds skb tailroom %u"
and the device keeps running.

Same class of bug as commit c04db81cd028 ("net/9p: Fix buffer overflow in USB transport layer"),
which hardened the USB 9p transport against unchecked device-reported length.

Fixes: 160fbcf3bfb9 ("Bluetooth: virtio_bt: Use skb_put to set length")
Cc: stable@vger.kernel.org
Cc: Soenke Huster <soenke.huster@eknoes.de>
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
 drivers/bluetooth/virtio_bt.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
index 76d61af8a275..157e68b6e75f 100644
--- a/drivers/bluetooth/virtio_bt.c
+++ b/drivers/bluetooth/virtio_bt.c
@@ -227,8 +227,15 @@ static void virtbt_rx_work(struct work_struct *work)
 	if (!skb)
 		return;
 
-	skb_put(skb, len);
-	virtbt_rx_handle(vbt, skb);
+	if (len > skb_tailroom(skb)) {
+		bt_dev_err(vbt->hdev,
+			   "rx reply len %u exceeds skb tailroom %u\n",
+			   len, skb_tailroom(skb));
+		kfree_skb(skb);
+	} else {
+		skb_put(skb, len);
+		virtbt_rx_handle(vbt, skb);
+	}
 
 	if (virtbt_add_inbuf(vbt) < 0)
 		return;
-- 
2.53.0

RE: Bluetooth: virtio_bt: clamp rx length before skb_put

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2496 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=1082712

---Test result---

Test Summary:
CheckPatch                    FAIL      0.73 seconds
GitLint                       FAIL      0.32 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      26.04 seconds
CheckAllWarning               PASS      28.12 seconds
CheckSparse                   PASS      27.11 seconds
BuildKernel32                 PASS      25.25 seconds
TestRunnerSetup               PASS      561.56 seconds
IncrementalBuild              PASS      23.96 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: virtio_bt: clamp rx length before skb_put
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#133: 
Same class of bug as commit c04db81cd028 ("net/9p: Fix buffer overflow in USB transport layer"),

WARNING: Non-standard signature: Assisted-by:
#140: 
Assisted-by: Claude:claude-opus-4-7

ERROR: Unrecognized email address: 'Claude:claude-opus-4-7'
#140: 
Assisted-by: Claude:claude-opus-4-7

total: 1 errors, 2 warnings, 17 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/patch/14529066.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: virtio_bt: clamp rx length before skb_put

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
33: B1 Line exceeds max length (96>80): "Same class of bug as commit c04db81cd028 ("net/9p: Fix buffer overflow in USB transport layer"),"


https://github.com/bluez/bluetooth-next/pull/101

---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: virtio_bt: clamp rx length before skb_put

[Luiz Augusto von Dentz]

Hi Michael,

On Fri, Apr 17, 2026 at 8:01 PM Michael Bommarito
<michael.bommarito@gmail.com> wrote:
>
> virtbt_rx_work() calls skb_put(skb, len) where len comes directly
> from virtqueue_get_buf() with no validation against the skb we
> posted.  The RX skb is allocated as alloc_skb(1000) in
> virtbt_add_inbuf().  A malicious or buggy virtio-bt backend that
> reports used.len larger than the skb's tailroom causes skb_put() to
> call skb_over_panic() in net/core/skbuff.c, which triggers
> BUG() and panics the guest.
>
> Reproduced on a QEMU 9.0 whose virtio-bt backend reports
> used.len = 4096 into a 1000-byte rx skb:
>
>   skbuff: skb_over_panic: text:ffffffff83958e84 len:4096 put:4096
>       head:ffff88800c071000 data:ffff88800c071000 tail:0x1000
>       end:0x6c0 dev:<NULL>
>   ------------[ cut here ]------------
>   kernel BUG at net/core/skbuff.c:214!
>   Call Trace:
>    skb_panic+0x160/0x162
>    skb_put.cold+0x31/0x31
>    virtbt_rx_work+0x94/0x250
>    process_one_work+0x80d/0x1510
>    worker_thread+0x4af/0xd20
>    kthread+0x2cc/0x3a0
>
> Reject any len that exceeds skb_tailroom().  Drop the skb on the
> error path; virtbt_add_inbuf() reposts a fresh one for the next
> iteration.  With the check in place the same harness runs without
> BUG(); the driver logs "rx reply len %u exceeds skb tailroom %u"
> and the device keeps running.
>
> Same class of bug as commit c04db81cd028 ("net/9p: Fix buffer overflow in USB transport layer"),
> which hardened the USB 9p transport against unchecked device-reported length.
>
> Fixes: 160fbcf3bfb9 ("Bluetooth: virtio_bt: Use skb_put to set length")
> Cc: stable@vger.kernel.org
> Cc: Soenke Huster <soenke.huster@eknoes.de>
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> Assisted-by: Claude:claude-opus-4-7
> ---
>  drivers/bluetooth/virtio_bt.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
> index 76d61af8a275..157e68b6e75f 100644
> --- a/drivers/bluetooth/virtio_bt.c
> +++ b/drivers/bluetooth/virtio_bt.c
> @@ -227,8 +227,15 @@ static void virtbt_rx_work(struct work_struct *work)
>         if (!skb)
>                 return;
>
> -       skb_put(skb, len);
> -       virtbt_rx_handle(vbt, skb);
> +       if (len > skb_tailroom(skb)) {
> +               bt_dev_err(vbt->hdev,
> +                          "rx reply len %u exceeds skb tailroom %u\n",
> +                          len, skb_tailroom(skb));
> +               kfree_skb(skb);
> +       } else {
> +               skb_put(skb, len);
> +               virtbt_rx_handle(vbt, skb);
> +       }
>
>         if (virtbt_add_inbuf(vbt) < 0)
>                 return;
> --
> 2.53.0

https://sashiko.dev/#/patchset/20260418000138.1848813-1-michael.bommarito%40gmail.com

All seem like valid comments to me, first one is odd to me thought,
never would have though that skb_tailroom wouldn't be enough to check
if using `skb_put` is safe.

-- 
Luiz Augusto von Dentz

[PATCH v2] Bluetooth: virtio_bt: clamp rx length before skb_put

[Michael Bommarito]

virtbt_rx_work() calls skb_put(skb, len) where len comes directly
from virtqueue_get_buf() with no validation against the buffer we
posted to the device. The RX skb is allocated in virtbt_add_inbuf()
and exposed to virtio as exactly 1000 bytes via sg_init_one().

Checking len against skb_tailroom(skb) is not sufficient because
alloc_skb() can leave more tailroom than the 1000 bytes actually
handed to the device. A malicious or buggy backend can therefore
report used.len between 1001 and skb_tailroom(skb), causing skb_put()
to include uninitialized kernel heap bytes that were never written by
the device.

The same path also accepts len == 0, in which case skb_put(skb, 0)
leaves the skb empty but virtbt_rx_handle() still reads the pkt_type
byte from skb->data, consuming uninitialized memory.

A single-byte completion also reaches hci_recv_frame() with skb->len
already pulled to 0. If the byte happened to be HCI_ACLDATA_PKT, the
ACL-vs-ISO classification fast-path in hci_dev_classify_pkt_type()
dereferences hci_acl_hdr(skb)->handle whenever the HCI device has an
active CIS_LINK, BIS_LINK, or PA_LINK connection, reading two bytes
of uninitialized RX-buffer data. The same hazard exists for every
packet type the driver accepts because none of the switch cases in
virtbt_rx_handle() check skb->len against the per-type minimum HCI
header size before handing the frame to the core.

Close all three paths:

  - define VIRTBT_RX_BUF_SIZE once, reuse it in alloc_skb() and
    sg_init_one() and as the upper bound on used.len, so the gate
    matches the buffer actually exposed to the device
  - reject used.len == 0 before virtbt_rx_handle() reads pkt_type
  - after stripping pkt_type, require skb->len to cover the fixed
    header size for the selected type (event/ACL/SCO/ISO) before
    calling hci_recv_frame(); drop ratelimited otherwise

Use bt_dev_err_ratelimited() because the length and packet-type
values come from an untrusted backend that can otherwise flood the
kernel log.

Same class of bug as commit c04db81cd028 ("net/9p: Fix buffer
overflow in USB transport layer"), which hardened the USB 9p
transport against unchecked device-reported length.

Fixes: 160fbcf3bfb9 ("Bluetooth: virtio_bt: Use skb_put to set length")
Cc: stable@vger.kernel.org
Cc: Soenke Huster <soenke.huster@eknoes.de>
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
Changes in v2:
- validate used.len against VIRTBT_RX_BUF_SIZE (the 1000 bytes
  actually exposed to the device via sg_init_one()) rather than
  skb_tailroom(), which can exceed 1000 due to slab padding
- reject used.len == 0 before virtbt_rx_handle() reads the pkt_type
  byte, so zero-length completions can no longer pull an empty skb
  into hci_recv_frame()
- in virtbt_rx_handle(), require skb->len to cover the fixed HCI
  header size for the selected pkt_type (event 2, ACL 4, SCO 3,
  ISO 4) before calling hci_recv_frame(); this prevents a one-byte
  HCI_ACLDATA_PKT completion from reaching
  hci_dev_classify_pkt_type() and dereferencing hci_acl_hdr(skb)
  over uninitialized RX buffer data when CIS/BIS/PA connections
  are present
- switch the error log to bt_dev_err_ratelimited() because the
  length and pkt_type values come from an untrusted backend that
  can otherwise flood the kernel log

 drivers/bluetooth/virtio_bt.c | 39 ++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
index 76d61af8a275..140ab55c9fc5 100644
--- a/drivers/bluetooth/virtio_bt.c
+++ b/drivers/bluetooth/virtio_bt.c
@@ -12,6 +12,7 @@
 #include <net/bluetooth/hci_core.h>
 
 #define VERSION "0.1"
+#define VIRTBT_RX_BUF_SIZE 1000
 
 enum {
 	VIRTBT_VQ_TX,
@@ -33,11 +34,11 @@ static int virtbt_add_inbuf(struct virtio_bluetooth *vbt)
 	struct sk_buff *skb;
 	int err;
 
-	skb = alloc_skb(1000, GFP_KERNEL);
+	skb = alloc_skb(VIRTBT_RX_BUF_SIZE, GFP_KERNEL);
 	if (!skb)
 		return -ENOMEM;
 
-	sg_init_one(sg, skb->data, 1000);
+	sg_init_one(sg, skb->data, VIRTBT_RX_BUF_SIZE);
 
 	err = virtqueue_add_inbuf(vq, sg, 1, skb, GFP_KERNEL);
 	if (err < 0) {
@@ -197,6 +198,7 @@ static int virtbt_shutdown_generic(struct hci_dev *hdev)
 
 static void virtbt_rx_handle(struct virtio_bluetooth *vbt, struct sk_buff *skb)
 {
+	size_t min_hdr;
 	__u8 pkt_type;
 
 	pkt_type = *((__u8 *) skb->data);
@@ -204,16 +206,32 @@ static void virtbt_rx_handle(struct virtio_bluetooth *vbt, struct sk_buff *skb)
 
 	switch (pkt_type) {
 	case HCI_EVENT_PKT:
+		min_hdr = sizeof(struct hci_event_hdr);
+		break;
 	case HCI_ACLDATA_PKT:
+		min_hdr = sizeof(struct hci_acl_hdr);
+		break;
 	case HCI_SCODATA_PKT:
+		min_hdr = sizeof(struct hci_sco_hdr);
+		break;
 	case HCI_ISODATA_PKT:
-		hci_skb_pkt_type(skb) = pkt_type;
-		hci_recv_frame(vbt->hdev, skb);
+		min_hdr = sizeof(struct hci_iso_hdr);
 		break;
 	default:
 		kfree_skb(skb);
-		break;
+		return;
+	}
+
+	if (skb->len < min_hdr) {
+		bt_dev_err_ratelimited(vbt->hdev,
+				       "rx pkt_type 0x%02x payload %u < hdr %zu\n",
+				       pkt_type, skb->len, min_hdr);
+		kfree_skb(skb);
+		return;
 	}
+
+	hci_skb_pkt_type(skb) = pkt_type;
+	hci_recv_frame(vbt->hdev, skb);
 }
 
 static void virtbt_rx_work(struct work_struct *work)
@@ -227,8 +245,15 @@ static void virtbt_rx_work(struct work_struct *work)
 	if (!skb)
 		return;
 
-	skb_put(skb, len);
-	virtbt_rx_handle(vbt, skb);
+	if (!len || len > VIRTBT_RX_BUF_SIZE) {
+		bt_dev_err_ratelimited(vbt->hdev,
+				       "rx reply len %u outside [1, %u]\n",
+				       len, VIRTBT_RX_BUF_SIZE);
+		kfree_skb(skb);
+	} else {
+		skb_put(skb, len);
+		virtbt_rx_handle(vbt, skb);
+	}
 
 	if (virtbt_add_inbuf(vbt) < 0)
 		return;
-- 
2.53.0

RE: [v2] Bluetooth: virtio_bt: clamp rx length before skb_put

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1680 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=1083916

---Test result---

Test Summary:
CheckPatch                    FAIL      0.58 seconds
GitLint                       PASS      0.20 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      26.45 seconds
CheckAllWarning               PASS      28.79 seconds
CheckSparse                   PASS      27.55 seconds
BuildKernel32                 PASS      25.63 seconds
TestRunnerSetup               PASS      570.69 seconds
IncrementalBuild              PASS      25.48 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[v2] Bluetooth: virtio_bt: clamp rx length before skb_put
WARNING: Non-standard signature: Assisted-by:
#156: 
Assisted-by: Claude:claude-opus-4-7

ERROR: Unrecognized email address: 'Claude:claude-opus-4-7'
#156: 
Assisted-by: Claude:claude-opus-4-7

total: 1 errors, 1 warnings, 79 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/patch/14532726.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.




https://github.com/bluez/bluetooth-next/pull/112

---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: virtio_bt: clamp rx length before skb_put

[Michael Bommarito]

On Mon, Apr 20, 2026 at 3:17 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> All seem like valid comments to me, first one is odd to me thought,
> never would have though that skb_tailroom wouldn't be enough to check
> if using `skb_put` is safe.

Thanks, those were definitely gnarly.  Went back in with
cscope/libclang and found another spot to guard too.

Let me know if you want me to re-organize/separate these into a
patchset instead of one bigger patch.

Re: [PATCH] Bluetooth: virtio_bt: clamp rx length before skb_put

[Luiz Augusto von Dentz]

Hi Michael,

On Tue, Apr 21, 2026 at 11:19 AM Michael Bommarito
<michael.bommarito@gmail.com> wrote:
>
> On Mon, Apr 20, 2026 at 3:17 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > All seem like valid comments to me, first one is odd to me thought,
> > never would have though that skb_tailroom wouldn't be enough to check
> > if using `skb_put` is safe.
>
> Thanks, those were definitely gnarly.  Went back in with
> cscope/libclang and found another spot to guard too.
>
> Let me know if you want me to re-organize/separate these into a
> patchset instead of one bigger patch.

Yeah, let's have it split into different patches, otherwise the commit
message gets convoluted trying to explain the what and why of each
change.


-- 
Luiz Augusto von Dentz

[PATCH v3 0/2] Bluetooth: virtio_bt: harden rx against untrusted backend

[Michael Bommarito]

Respin of the virtio_bt rx hardening patch, split per Luiz's
request on the v2 thread:

  https://lore.kernel.org/linux-bluetooth/20260421151659.3326690-1-michael.bommarito@gmail.com/

v2 bundled two independent hazards in one commit and the commit
message got convoluted trying to explain them together. This v3
keeps each hazard in its own patch so the rationale for each is
self-contained.

Patch 1/2 keeps the original v1/v2 concern: virtbt_rx_work() calls
skb_put() with a used.len the device controls, with no validation
against the 1000-byte buffer we actually exposed to virtio via
sg_init_one(). Checking against skb_tailroom() is not enough
because alloc_skb() can leave more tailroom than 1000 bytes due to
slab padding. len == 0 is also accepted, which lets an empty skb
reach virtbt_rx_handle() where the pkt_type byte is then read from
uninitialized memory. 1/2 defines VIRTBT_RX_BUF_SIZE, reuses it in
alloc_skb() and sg_init_one(), and gates virtbt_rx_work() on
[1, VIRTBT_RX_BUF_SIZE] with bt_dev_err_ratelimited() on the drop
path.

Patch 2/2 closes a residual short-payload hazard found during v2
pre-send review. After 1/2 restricts used.len to [1, 1000], a
one-byte completion still reaches hci_recv_frame() with skb->len
pulled to 0. If that byte was HCI_ACLDATA_PKT, the ACL-vs-ISO
classification fast-path in hci_dev_classify_pkt_type()
dereferences hci_acl_hdr(skb)->handle whenever the HCI device has
an active CIS_LINK, BIS_LINK, or PA_LINK connection, reading two
bytes of uninitialized RX-buffer data. The same hazard shape
exists for every packet type because none of the switch cases in
virtbt_rx_handle() checked skb->len against the per-type minimum
HCI header. 2/2 requires skb->len to cover the fixed header size
for the selected type (event 2, ACL 4, SCO 3, ISO 4) before
calling hci_recv_frame(); drop ratelimited otherwise. Unknown
pkt_type values still take the original kfree_skb() default path.

Both patches carry the same Fixes: 160fbcf3bfb9 and Cc: stable@
as the v2 commit, because both hazards have existed since the
driver switched to skb_put() for used.len handling.

Fresh runtime repro of the original skb_over_panic on an unpatched
tree and a clean reject log with 1/2 applied are attached in the
evidence bundle referenced from the v2 thread; no runtime change
between v2 and v3 beyond the split (the final tree after 1/2+2/2
is byte-identical to the v2 commit).

Prior public versions of this patch on linux-bluetooth:

  v1: <20260418000138.1848813-1-michael.bommarito@gmail.com>
  v2: <20260421151659.3326690-1-michael.bommarito@gmail.com>

Michael Bommarito (2):
  Bluetooth: virtio_bt: clamp rx length before skb_put
  Bluetooth: virtio_bt: validate rx pkt_type header length

 drivers/bluetooth/virtio_bt.c | 39 ++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

-- 
2.53.0

[PATCH v3 1/2] Bluetooth: virtio_bt: clamp rx length before skb_put

[Michael Bommarito]

virtbt_rx_work() calls skb_put(skb, len) where len comes directly
from virtqueue_get_buf() with no validation against the buffer we
posted to the device. The RX skb is allocated in virtbt_add_inbuf()
and exposed to virtio as exactly 1000 bytes via sg_init_one().

Checking len against skb_tailroom(skb) is not sufficient because
alloc_skb() can leave more tailroom than the 1000 bytes actually
handed to the device. A malicious or buggy backend can therefore
report used.len between 1001 and skb_tailroom(skb), causing skb_put()
to include uninitialized kernel heap bytes that were never written by
the device.

The same path also accepts len == 0, in which case skb_put(skb, 0)
leaves the skb empty but virtbt_rx_handle() still reads the pkt_type
byte from skb->data, consuming uninitialized memory.

Define VIRTBT_RX_BUF_SIZE once and reuse it in alloc_skb() and
sg_init_one(), and gate virtbt_rx_work() on that same constant so
the bound checked matches the buffer actually exposed to the device.
Reject used.len == 0 in the same gate so an empty completion can
no longer reach virtbt_rx_handle().

Use bt_dev_err_ratelimited() because the length value comes from an
untrusted backend that can otherwise flood the kernel log.

Same class of bug as commit c04db81cd028 ("net/9p: Fix buffer
overflow in USB transport layer"), which hardened the USB 9p
transport against unchecked device-reported length.

Fixes: 160fbcf3bfb9 ("Bluetooth: virtio_bt: Use skb_put to set length")
Cc: stable@vger.kernel.org
Cc: Soenke Huster <soenke.huster@eknoes.de>
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
Changes in v3:
- split off the per-pkt-type header-length check into its own
  patch (2/2) per Luiz's request; this patch now only covers the
  used.len vs buffer-size hazard and the len == 0 path

Changes in v2:
- validate used.len against VIRTBT_RX_BUF_SIZE (the 1000 bytes
  actually exposed to the device via sg_init_one()) rather than
  skb_tailroom(), which can exceed 1000 due to slab padding
- reject used.len == 0 before virtbt_rx_handle() reads the
  pkt_type byte, so zero-length completions can no longer pull an
  empty skb into hci_recv_frame()
- switch the error log to bt_dev_err_ratelimited() because the
  length value comes from an untrusted backend that can otherwise
  flood the kernel log

 drivers/bluetooth/virtio_bt.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
index 76d61af8a275..2c5c39356a1c 100644
--- a/drivers/bluetooth/virtio_bt.c
+++ b/drivers/bluetooth/virtio_bt.c
@@ -12,6 +12,7 @@
 #include <net/bluetooth/hci_core.h>
 
 #define VERSION "0.1"
+#define VIRTBT_RX_BUF_SIZE 1000
 
 enum {
 	VIRTBT_VQ_TX,
@@ -33,11 +34,11 @@ static int virtbt_add_inbuf(struct virtio_bluetooth *vbt)
 	struct sk_buff *skb;
 	int err;
 
-	skb = alloc_skb(1000, GFP_KERNEL);
+	skb = alloc_skb(VIRTBT_RX_BUF_SIZE, GFP_KERNEL);
 	if (!skb)
 		return -ENOMEM;
 
-	sg_init_one(sg, skb->data, 1000);
+	sg_init_one(sg, skb->data, VIRTBT_RX_BUF_SIZE);
 
 	err = virtqueue_add_inbuf(vq, sg, 1, skb, GFP_KERNEL);
 	if (err < 0) {
@@ -227,8 +228,15 @@ static void virtbt_rx_work(struct work_struct *work)
 	if (!skb)
 		return;
 
-	skb_put(skb, len);
-	virtbt_rx_handle(vbt, skb);
+	if (!len || len > VIRTBT_RX_BUF_SIZE) {
+		bt_dev_err_ratelimited(vbt->hdev,
+				       "rx reply len %u outside [1, %u]\n",
+				       len, VIRTBT_RX_BUF_SIZE);
+		kfree_skb(skb);
+	} else {
+		skb_put(skb, len);
+		virtbt_rx_handle(vbt, skb);
+	}
 
 	if (virtbt_add_inbuf(vbt) < 0)
 		return;
-- 
2.53.0

[PATCH v3 2/2] Bluetooth: virtio_bt: validate rx pkt_type header length

[Michael Bommarito]

virtbt_rx_handle() reads the leading pkt_type byte from the RX skb
and forwards the remainder to hci_recv_frame() for every
event/ACL/SCO/ISO type, without checking that the remaining payload
is at least the fixed HCI header for that type.

After the preceding patch bounds the backend-supplied used.len to
[1, VIRTBT_RX_BUF_SIZE], a one-byte completion still reaches
hci_recv_frame() with skb->len already pulled to 0. If the byte
happened to be HCI_ACLDATA_PKT, the ACL-vs-ISO classification
fast-path in hci_dev_classify_pkt_type() dereferences
hci_acl_hdr(skb)->handle whenever the HCI device has an active
CIS_LINK, BIS_LINK, or PA_LINK connection, reading two bytes of
uninitialized RX-buffer data. The same hazard exists for every
packet type the driver accepts because none of the switch cases in
virtbt_rx_handle() check skb->len against the per-type minimum HCI
header size before handing the frame to the core.

After stripping pkt_type, require skb->len to cover the fixed
header size for the selected type (event 2, ACL 4, SCO 3, ISO 4)
before calling hci_recv_frame(); drop ratelimited otherwise.
Unknown pkt_type values still take the original kfree_skb() default
path.

Use bt_dev_err_ratelimited() because both the length and pkt_type
values come from an untrusted backend that can otherwise flood the
kernel log.

Fixes: 160fbcf3bfb9 ("Bluetooth: virtio_bt: Use skb_put to set length")
Cc: stable@vger.kernel.org
Cc: Soenke Huster <soenke.huster@eknoes.de>
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
Changes in v3:
- new patch, split out of the v2 commit per Luiz's request on
  the v2 thread so the per-pkt-type header-length check can be
  reviewed on its own

Changes in v2:
- in virtbt_rx_handle(), require skb->len to cover the fixed HCI
  header size for the selected pkt_type (event 2, ACL 4, SCO 3,
  ISO 4) before calling hci_recv_frame(); this prevents a one-byte
  HCI_ACLDATA_PKT completion from reaching
  hci_dev_classify_pkt_type() and dereferencing hci_acl_hdr(skb)
  over uninitialized RX buffer data when CIS/BIS/PA connections
  are present
- switch the error log to bt_dev_err_ratelimited() because the
  length and pkt_type values come from an untrusted backend that
  can otherwise flood the kernel log

 drivers/bluetooth/virtio_bt.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
index 2c5c39356a1c..140ab55c9fc5 100644
--- a/drivers/bluetooth/virtio_bt.c
+++ b/drivers/bluetooth/virtio_bt.c
@@ -198,6 +198,7 @@ static int virtbt_shutdown_generic(struct hci_dev *hdev)
 
 static void virtbt_rx_handle(struct virtio_bluetooth *vbt, struct sk_buff *skb)
 {
+	size_t min_hdr;
 	__u8 pkt_type;
 
 	pkt_type = *((__u8 *) skb->data);
@@ -205,16 +206,32 @@ static void virtbt_rx_handle(struct virtio_bluetooth *vbt, struct sk_buff *skb)
 
 	switch (pkt_type) {
 	case HCI_EVENT_PKT:
+		min_hdr = sizeof(struct hci_event_hdr);
+		break;
 	case HCI_ACLDATA_PKT:
+		min_hdr = sizeof(struct hci_acl_hdr);
+		break;
 	case HCI_SCODATA_PKT:
+		min_hdr = sizeof(struct hci_sco_hdr);
+		break;
 	case HCI_ISODATA_PKT:
-		hci_skb_pkt_type(skb) = pkt_type;
-		hci_recv_frame(vbt->hdev, skb);
+		min_hdr = sizeof(struct hci_iso_hdr);
 		break;
 	default:
 		kfree_skb(skb);
-		break;
+		return;
 	}
+
+	if (skb->len < min_hdr) {
+		bt_dev_err_ratelimited(vbt->hdev,
+				       "rx pkt_type 0x%02x payload %u < hdr %zu\n",
+				       pkt_type, skb->len, min_hdr);
+		kfree_skb(skb);
+		return;
+	}
+
+	hci_skb_pkt_type(skb) = pkt_type;
+	hci_recv_frame(vbt->hdev, skb);
 }
 
 static void virtbt_rx_work(struct work_struct *work)
-- 
2.53.0