[PATCH v2] Bluetooth: btmtk: don't generate bogus ISO packets from zero padding

[PATCH v2] Bluetooth: btmtk: don't generate bogus ISO packets from zero padding

[Pauli Virtanen]

With MT7925, received URBs for ISO have urb->actual_length of
MTK_ISO_THRESHOLD (264) and contain ISO packet data followed by zero
padding.

btmtk driver ISO handling added in commit ceac1cb0259d ("Bluetooth:
btusb: mediatek: add ISO data transmission functions") assumes the data
contains only payload, and generates bogus packets to hci_core from the
padding, resulting to printk errors "Bluetooth: hci0: ISO packet for
unknown connection handle 0"

Add check in btmtk that a new packet is not started if we have only
trailing zeros left in the URB. If so, skip them and emit an error once
per hdev, since this doesn't seem to be the intended behavior. This
reduces spam to user logs.

Limit this behavior to the 7925 model.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
    v2:
    - limit this to the 7925 model, as that is the only one tested
    
    If there's confirmation from Mediatek the zero padding instead of short
    USB read is intentional, the warning is not needed.

 drivers/bluetooth/btmtk.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/bluetooth/btmtk.h |  2 ++
 2 files changed, 37 insertions(+)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index ab34f1dd42bc..934daf004dd6 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -1088,11 +1088,34 @@ struct urb *alloc_mtk_intr_urb(struct hci_dev *hdev, struct sk_buff *skb,
 }
 EXPORT_SYMBOL_GPL(alloc_mtk_intr_urb);
 
+static bool check_isopkt(struct hci_dev *hdev, void *buffer, int count,
+			 int pktsize)
+{
+	struct btmtk_data *btmtk_data = hci_get_priv(hdev);
+
+	/* With MT7925 we receive URBs that have size MTK_ISO_THRESHOLD, with
+	 * trailing zero padding following the ISO packet data. This appears not
+	 * intentional, so flag an error, and skip rest of the URB to not
+	 * generate ISO packets from the padding.
+	 */
+	if (btmtk_data->isopkt_rx_pad && pktsize == MTK_ISO_THRESHOLD &&
+	    count < pktsize && mem_is_zero(buffer, count)) {
+		if (!btmtk_data->isopkt_padding_seen) {
+			btmtk_data->isopkt_padding_seen = true;
+			bt_dev_err(hdev, "Zero ISO data (only first reported)");
+		}
+		return false;
+	}
+
+	return true;
+}
+
 static int btmtk_recv_isopkt(struct hci_dev *hdev, void *buffer, int count)
 {
 	struct btmtk_data *btmtk_data = hci_get_priv(hdev);
 	struct sk_buff *skb;
 	unsigned long flags;
+	int pktsize = count;
 	int err = 0;
 
 	spin_lock_irqsave(&btmtk_data->isorxlock, flags);
@@ -1102,6 +1125,9 @@ static int btmtk_recv_isopkt(struct hci_dev *hdev, void *buffer, int count)
 		int len;
 
 		if (!skb) {
+			if (!check_isopkt(hdev, buffer, count, pktsize))
+				break;
+
 			skb = bt_skb_alloc(HCI_MAX_ISO_SIZE, GFP_ATOMIC);
 			if (!skb) {
 				err = -ENOMEM;
@@ -1250,6 +1276,15 @@ static int btmtk_usb_isointf_init(struct hci_dev *hdev)
 
 	spin_lock_init(&btmtk_data->isorxlock);
 
+	switch (btmtk_data->dev_id) {
+	case 0x7925:
+		btmtk_data->isopkt_rx_pad = true;
+		break;
+	default:
+		btmtk_data->isopkt_rx_pad = false;
+		break;
+	}
+
 	__set_mtk_intr_interface(hdev);
 
 	err = btmtk_submit_intr_urb(hdev, GFP_KERNEL);
diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index c83c24897c95..bd97e37afd99 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -177,6 +177,8 @@ struct btmtk_data {
 	struct usb_interface *isopkt_intf;
 	struct usb_anchor isopkt_anchor;
 	struct sk_buff *isopkt_skb;
+	bool isopkt_rx_pad;
+	bool isopkt_padding_seen;
 
 	/* spinlock for ISO data transmission */
 	spinlock_t isorxlock;
-- 
2.53.0

RE: [v2] Bluetooth: btmtk: don't generate bogus ISO packets from zero padding

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      0.79 seconds
GitLint                       FAIL      0.26 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      27.00 seconds
CheckAllWarning               PASS      28.81 seconds
CheckSparse                   PASS      27.85 seconds
BuildKernel32                 PASS      25.95 seconds
TestRunnerSetup               PASS      568.80 seconds
IncrementalBuild              PASS      24.21 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v2] Bluetooth: btmtk: don't generate bogus ISO packets from zero padding

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
26: B2 Line has trailing whitespace: "    "


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

---
Regards,
Linux Bluetooth