Hi Michael, On Fri, Apr 17, 2026 at 8:01 PM Michael Bommarito <[email protected]> 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: [email protected] > Cc: Soenke Huster <[email protected]> > Signed-off-by: Michael Bommarito <[email protected]> > 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
