Re: [PATCH v2] Bluetooth: btusb: Check for unexpected bytes when defragmenting HCI frames

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Arkadiusz,

On Tue, Aug 26, 2025 at 3:31 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Arkadiusz,
>
> On Tue, Aug 26, 2025 at 1:03 PM Arkadiusz Bokowy
> <arkadiusz.bokowy@gmail.com> wrote:
> >
> > Some Barrot based USB Bluetooth dongles erroneously send one extra
> > random byte for the HCI_OP_READ_LOCAL_EXT_FEATURES command. The
> > consequence of that is that the next HCI transfer is misaligned by one
> > byte causing undefined behavior. In most cases the response event for
> > the next command fails with random error code.
> >
> > Since the HCI_OP_READ_LOCAL_EXT_FEATURES command is used during HCI
> > controller initialization, the initialization fails rendering the USB
> > dongle not usable.
> >
> > > [59.464099] usb 1-1.3: new full-speed USB device number 11 using xhci_hcd
> > > [59.561617] usb 1-1.3: New USB device found, idVendor=33fa, idProduct=0012, bcdDevice=88.91
> > > [59.561642] usb 1-1.3: New USB device strings: Mfr=0, Product=2, SerialNumber=0
> > > [59.561656] usb 1-1.3: Product: UGREEN BT6.0 Adapter
> > > [61.720116] Bluetooth: hci1: command 0x1005 tx timeout
> > > [61.720167] Bluetooth: hci1: Opcode 0x1005 failed: -110
> >
> > This patch was tested with the 33fa:0012 device. Now the device is
> > initialized properly:
>
> I'm trying to find any entry with 0x33fa but I couldn't find any, we
> probably want to include the Fixes tag to the commit that introduces
> support for it.

Looks like we are missing:

+       /* Barrot Technology Bluetooth devices */
+       { USB_DEVICE(0x33fa, 0x0010), .driver_info = BTUSB_BARROT },
+       { USB_DEVICE(0x33fa, 0x0012), .driver_info = BTUSB_BARROT },

We can probably add it together in the same patch, so we don't have to
rely on Fixes to inform the changes below needs to go together, but
then we should include the contents of /sys/kernel/debug/usb/devices
before the information why we need the changes to detect extra bytes
in order for it to work.

> >
> > > [43.329852] usb 1-1.4: new full-speed USB device number 4 using dwc_otg
> > > [43.446790] usb 1-1.4: New USB device found, idVendor=33fa, idProduct=0012, bcdDevice=88.91
> > > [43.446813] usb 1-1.4: New USB device strings: Mfr=0, Product=2, SerialNumber=0
> > > [43.446821] usb 1-1.4: Product: UGREEN BT6.0 Adapter
> > > [43.582024] Bluetooth: hci1: Unexpected continuation: 1 bytes
> > > [43.703025] Bluetooth: hci1: Unexpected continuation: 1 bytes
> > > [43.750141] Bluetooth: MGMT ver 1.23
> >
> > Signed-off-by: Arkadiusz Bokowy <arkadiusz.bokowy@gmail.com>
> > Tested-by: Arkadiusz Bokowy <arkadiusz.bokowy@gmail.com>
> > ---
> >  drivers/bluetooth/btusb.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 8085fabad..24a249d1c 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -1192,6 +1192,18 @@ static int btusb_recv_intr(struct btusb_data *data, void *buffer, int count)
> >                 }
> >
> >                 if (!hci_skb_expect(skb)) {
> > +                       /* Each chunk should correct to at least 1 or more
> > +                        * events so if there are still bytes left that doesn't
> > +                        * constitute a new event this is likely a bug in the
> > +                        * controller.
> > +                        */
> > +                       if (count && count < HCI_EVENT_HDR_SIZE) {
> > +                               bt_dev_warn(data->hdev,
> > +                                       "Unexpected continuation: %d bytes",
> > +                                       count);
> > +                               count = 0;
> > +                       }
> > +
> >                         /* Complete frame */
> >                         btusb_recv_event(data, skb);
> >                         skb = NULL;
> > --
> > 2.47.2
> >
> >
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz