[PATCH 0/2] Bluetooth: btmtk: fix ISO data reception

[PATCH 0/2] Bluetooth: btmtk: fix ISO data reception

[Chris Lu]

Two problems in btmtk_recv_isopkt(), both found while debugging LE Audio
on MT7925. The function is shared by every MediaTek USB controller that
supports ISO data, so neither is specific to that part.

Patch 1 stops the zero padding emitted by the ISO interrupt IN endpoint
from being parsed as ISO packets. The endpoint delivers one packet per
264-byte transfer and zero-fills the rest; the driver turns every four
zero bytes into a handle 0 packet with Data_Total_Length 0 and reports it
to the stack. A single transfer can inject up to 64 of them, which is
what fills the HCI log with malformed ISO frames, and a valid SDU is lost
whenever handle 0 belongs to a live CIS/BIS.

Patch 2 corrects the receive skb being allocated 4 bytes too small, so
that a 248..251 byte payload would be rejected with -EILSEQ.
HCI_MAX_ISO_SIZE is a payload limit rather than a frame limit, and the
allocation should reflect that.

The two hunks are independent, so patch 1 applies on its own if you would
rather not take patch 2.

Based on bluetooth-next.

Chris Lu (2):
  Bluetooth: btmtk: drop zero-length ISO packets from endpoint padding
  Bluetooth: btmtk: size the ISO receive skb to hold header plus payload

 drivers/bluetooth/btmtk.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

-- 
2.45.2

[PATCH 1/2] Bluetooth: btmtk: drop zero-length ISO packets from endpoint padding

[Chris Lu]

The ISO interrupt IN endpoint delivers one HCI ISO packet per 264-byte
transfer and zero-fills the remainder. btmtk_recv_isopkt() keeps parsing
that padding: every four zero bytes look like an ISO header with handle 0
and Data_Total_Length 0, and are passed to hci_recv_frame() as a complete
frame. A single transfer can inject up to 64 of them, which is what fills
the HCI log with malformed ISO packets. If handle 0 belongs to a live
CIS/BIS, iso_recv() also drops the SDU it is reassembling.

A zero Data_Total_Length carries no ISO Data Load at all, and the
controller never emits one -- these are produced purely by parsing the
padding. The padding always runs to the end of the transfer, so stop
parsing once one is seen.

This was observed on MT7925, but btmtk_recv_isopkt() is shared by every
MediaTek USB controller that supports ISO data.

Fixes: ceac1cb0259d ("Bluetooth: btusb: mediatek: add ISO data transmission functions")
Co-developed-by: Hao Qin <hao.qin@mediatek.com>
Signed-off-by: Hao Qin <hao.qin@mediatek.com>
Co-developed-by: Wenlong Tian <ot_wenlong.tian@mediatek.com>
Signed-off-by: Wenlong Tian <ot_wenlong.tian@mediatek.com>
Signed-off-by: Chris Lu <chris.lu@mediatek.com>
---
 drivers/bluetooth/btmtk.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index bdc85147b97f..869eb2176379 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -1201,6 +1201,19 @@ static int btmtk_recv_isopkt(struct hci_dev *hdev, void *buffer, int count)
 			/* Complete ISO header */
 			hci_skb_expect(skb) = __le16_to_cpu(dlen);
 
+			/* The interrupt endpoint zero-fills the remainder of
+			 * each transfer, and that padding parses as a header
+			 * with a zero Data_Total_Length. Such a packet carries
+			 * no ISO Data Load at all and is never emitted by the
+			 * controller, so stop parsing here rather than hand a
+			 * bogus handle 0 packet to the stack.
+			 */
+			if (!hci_skb_expect(skb)) {
+				kfree_skb(skb);
+				skb = NULL;
+				break;
+			}
+
 			if (skb_tailroom(skb) < hci_skb_expect(skb)) {
 				kfree_skb(skb);
 				skb = NULL;
-- 
2.45.2

RE: Bluetooth: btmtk: fix ISO data reception

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1019 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/series/1184506/

---Test result---

Test Summary:
CheckPatch                    PASS      1.34 seconds
VerifyFixes                   PASS      0.11 seconds
VerifySignedoff               PASS      0.11 seconds
GitLint                       PASS      0.59 seconds
SubjectPrefix                 PASS      0.20 seconds
BuildKernel                   PASS      27.73 seconds
CheckAllWarning               PASS      31.68 seconds
CheckSparse                   PASS      34.89 seconds
BuildKernel32                 PASS      28.14 seconds
CheckKernelLLVM               PASS      31.53 seconds
TestRunnerSetup               PASS      653.17 seconds
IncrementalBuild              PASS      28.03 seconds



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

---
Regards,
Linux Bluetooth

[PATCH 2/2] Bluetooth: btmtk: size the ISO receive skb to hold header plus payload

[Chris Lu]

btmtk_recv_isopkt() allocates HCI_MAX_ISO_SIZE (251) bytes and stores the
4-byte HCI ISO header into the same skb, leaving only 247 bytes for the
payload. A Data_Total_Length of 248..251 would fail the skb_tailroom()
check and be dropped with -EILSEQ, taking the rest of the URB transfer
with it.

HCI_MAX_ISO_SIZE is the maximum payload length, not the maximum frame
length, so allocate HCI_MAX_ISO_SIZE + HCI_ISO_HDR_SIZE, matching how the
ACL path uses HCI_MAX_FRAME_SIZE (HCI_MAX_ACL_SIZE + 4).

Fixes: ceac1cb0259d ("Bluetooth: btusb: mediatek: add ISO data transmission functions")
Co-developed-by: Hao Qin <hao.qin@mediatek.com>
Signed-off-by: Hao Qin <hao.qin@mediatek.com>
Co-developed-by: Wenlong Tian <ot_wenlong.tian@mediatek.com>
Signed-off-by: Wenlong Tian <ot_wenlong.tian@mediatek.com>
Signed-off-by: Chris Lu <chris.lu@mediatek.com>
---
 drivers/bluetooth/btmtk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 869eb2176379..1b501b64d71f 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -1178,7 +1178,8 @@ static int btmtk_recv_isopkt(struct hci_dev *hdev, void *buffer, int count)
 		int len;
 
 		if (!skb) {
-			skb = bt_skb_alloc(HCI_MAX_ISO_SIZE, GFP_ATOMIC);
+			skb = bt_skb_alloc(HCI_MAX_ISO_SIZE + HCI_ISO_HDR_SIZE,
+					   GFP_ATOMIC);
 			if (!skb) {
 				err = -ENOMEM;
 				break;
-- 
2.45.2