The USB interface between the host and the bluetooth adapter used for
SCO packets uses an USB isochronous endpoint with a fragmentation scheme
that does not tolerate errors. Except USB isochronous transfers do
not provide a reliable stream with guaranteed delivery. (There is no
retry on error, see USB spec v2.0 5.6 and 8.5.5.)
To fragment a packet, the bluetooth HCI simply splits it in parts and
transfer them as-is. The receiver is expected to reconstruct the packet
by assuming the first fragment contains the header and parsing its size
field. There is no error detection either.
If a fragment is lost, the end result is that the kernel is no longer
synchronized and will pass malformed data to the upper layers, since it
has no way to tell if the first fragment is an actual first fragment or
a continuation fragment. Resynchronization can only happen by luck and
requires an unbounded amount of time.
The typical symptom for a HSP/HFP bluetooth headset is that the
microphone stops working and dmesg contains piles of rate-limited
"Bluetooth: hci0: SCO packet for unknown connection handle XXXX"
errors for an indeterminate amount of time, until the kernel accidentally
resynchronize.
A workaround is to ask the upper layer to prevalidate the first fragment
header. This is not possible with user channels so this workaround is
disabled in this case.
This problem is the most severe when using an ath3k adapter on an i.MX 6
board, where packet loss occur regularly, possibly because it is an USB1
device connected on an USB2 hub and this is a special case requiring
split transactions.
Signed-off-by: Nicolas Cavallari <[email protected]>
---
v3: put everything in btusb.c with a comment
v2 (mistakenly sent without 'v2' in title): fix typos in commit description
and expand it
drivers/bluetooth/btusb.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 15caa6469538..fbf450883601 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -948,6 +948,34 @@ static int btusb_recv_bulk(struct btusb_data *data, void *buffer, int count)
return err;
}
+static bool btusb_validate_sco_handle(struct hci_dev *hdev,
+ struct hci_sco_hdr *hdr)
+{
+ __u16 handle;
+
+ if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL))
+ // Can't validate, userspace controls everything.
+ return true;
+
+ /*
+ * USB isochronous transfers are not designed to be reliable and may
+ * lose fragments. When this happens, the next first fragment
+ * encountered might actually be a continuation fragment.
+ * Validate the handle to detect it and drop it, or else the upper
+ * layer will get garbage for a while.
+ */
+
+ handle = hci_handle(__le16_to_cpu(hdr->handle));
+
+ switch (hci_conn_lookup_type(hdev, handle)) {
+ case SCO_LINK:
+ case ESCO_LINK:
+ return true;
+ default:
+ return false;
+ }
+}
+
static int btusb_recv_isoc(struct btusb_data *data, void *buffer, int count)
{
struct sk_buff *skb;
@@ -980,9 +1008,12 @@ static int btusb_recv_isoc(struct btusb_data *data, void *buffer, int count)
if (skb->len == HCI_SCO_HDR_SIZE) {
/* Complete SCO header */
- hci_skb_expect(skb) = hci_sco_hdr(skb)->dlen;
+ struct hci_sco_hdr *hdr = hci_sco_hdr(skb);
- if (skb_tailroom(skb) < hci_skb_expect(skb)) {
+ hci_skb_expect(skb) = hdr->dlen;
+
+ if (skb_tailroom(skb) < hci_skb_expect(skb) ||
+ !btusb_validate_sco_handle(data->hdev, hdr)) {
kfree_skb(skb);
skb = NULL;
--
2.37.2
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=683249
---Test result---
Test Summary:
CheckPatch PASS 1.32 seconds
GitLint PASS 0.75 seconds
SubjectPrefix PASS 0.57 seconds
BuildKernel PASS 42.23 seconds
BuildKernel32 PASS 37.82 seconds
Incremental Build with patchesPASS 52.59 seconds
TestRunner: Setup PASS 623.02 seconds
TestRunner: l2cap-tester PASS 20.33 seconds
TestRunner: iso-tester PASS 19.78 seconds
TestRunner: bnep-tester PASS 7.42 seconds
TestRunner: mgmt-tester PASS 123.60 seconds
TestRunner: rfcomm-tester PASS 11.94 seconds
TestRunner: sco-tester PASS 11.11 seconds
TestRunner: ioctl-tester PASS 12.68 seconds
TestRunner: mesh-tester PASS 9.28 seconds
TestRunner: smp-tester PASS 10.98 seconds
TestRunner: userchan-tester PASS 7.78 seconds
---
Regards,
Linux Bluetooth
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Wed, 5 Oct 2022 17:06:21 +0200 you wrote:
> The USB interface between the host and the bluetooth adapter used for
> SCO packets uses an USB isochronous endpoint with a fragmentation scheme
> that does not tolerate errors. Except USB isochronous transfers do
> not provide a reliable stream with guaranteed delivery. (There is no
> retry on error, see USB spec v2.0 5.6 and 8.5.5.)
>
> To fragment a packet, the bluetooth HCI simply splits it in parts and
> transfer them as-is. The receiver is expected to reconstruct the packet
> by assuming the first fragment contains the header and parsing its size
> field. There is no error detection either.
>
> [...]
Here is the summary with links:
- [v3] Bluetooth: Work around SCO over USB HCI design defect
https://git.kernel.org/bluetooth/bluetooth-next/c/7df90b55ae9a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html