* [PATCH 1/3] Bluetooth: btmtk: Route firmware debug event to the diag channel
2026-09-11 10:42 [PATCH 0/3] Bluetooth: btmtk: firmware debug event routing and WMT FUNC_CTRL status fixes Chris Lu
@ 2026-09-11 10:42 ` Chris Lu
2026-09-11 14:25 ` Bluetooth: btmtk: firmware debug event routing and WMT FUNC_CTRL status fixes bluez.test.bot
2026-09-11 10:42 ` [PATCH 2/3] Bluetooth: btmtk: fix wrong status for short WMT FUNC_CTRL events Chris Lu
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Chris Lu @ 2026-09-11 10:42 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, Luiz Von Dentz
Cc: Sean Wang, Will Lee, SS Wu, linux-bluetooth, linux-kernel,
linux-mediatek, Chris Lu
MediaTek controllers may emit a firmware debug event on the ACL channel
using the reserved handle 0x0efd, which shows up in the ACL header as
0x2efd once the start fragment flag is included.
Neither btmtk_usb_recv_acl() nor btmtksdio_recv_acl() recognizes it, so
the packet is passed to the HCI core, which has no connection with that
handle and complains:
Bluetooth: hci0: ACL packet for unknown connection handle 3837
Handle it the same way as the existing firmware debug logging packets and
forward it to the diagnostic channel instead.
Verified on MT7922: under the condition that triggers this firmware
debug event, it is now routed to the diag channel instead of reaching
the host as an unknown ACL packet.
Signed-off-by: Chris Lu <chris.lu@mediatek.com>
---
drivers/bluetooth/btmtk.c | 1 +
drivers/bluetooth/btmtksdio.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 660ed5b02841..5b0a131bc9d2 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -1076,6 +1076,7 @@ int btmtk_usb_recv_acl(struct hci_dev *hdev, struct sk_buff *skb)
fallthrough;
case 0x05ff: /* Firmware debug logging 1 */
case 0x05fe: /* Firmware debug logging 2 */
+ case 0x2efd: /* Firmware debug event */
return hci_recv_diag(hdev, skb);
}
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index fe4ca9395aa3..9a1b6f9b0872 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -460,6 +460,7 @@ static int btmtksdio_recv_acl(struct hci_dev *hdev, struct sk_buff *skb)
fallthrough;
case 0x05ff:
case 0x05fe:
+ case 0x2efd: /* Firmware debug event */
/* Firmware debug logging */
return hci_recv_diag(hdev, skb);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] Bluetooth: btmtk: fix wrong status for short WMT FUNC_CTRL events
2026-09-11 10:42 [PATCH 0/3] Bluetooth: btmtk: firmware debug event routing and WMT FUNC_CTRL status fixes Chris Lu
2026-09-11 10:42 ` [PATCH 1/3] Bluetooth: btmtk: Route firmware debug event to the diag channel Chris Lu
@ 2026-09-11 10:42 ` Chris Lu
2026-09-11 10:42 ` [PATCH 3/3] Bluetooth: btmtksdio, btmtkuart: validate WMT event length before struct access Chris Lu
2026-09-11 14:33 ` [PATCH 0/3] Bluetooth: btmtk: firmware debug event routing and WMT FUNC_CTRL status fixes Luiz Augusto von Dentz
3 siblings, 0 replies; 6+ messages in thread
From: Chris Lu @ 2026-09-11 10:42 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, Luiz Von Dentz
Cc: Sean Wang, Will Lee, SS Wu, linux-bluetooth, linux-kernel,
linux-mediatek, Chris Lu
A too-short BTMTK_WMT_FUNC_CTRL event (WMT header only, no trailing
2-byte status word) is always treated as BTMTK_WMT_ON_UNDONE. This
short form is how firmware acks a plain enable/disable request, and
the actual result is carried in the header's own flag byte (0 =
success), not a separate status word. Decode it from there instead of
assuming failure.
Verified setup on MT7920, MT7921, MT7922 and MT7925: no regression.
Fixes: e3ac0d9f1a20 ("Bluetooth: btmtk: accept too short WMT FUNC_CTRL events")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Chris Lu <chris.lu@mediatek.com>
---
drivers/bluetooth/btmtk.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 660ed5b02841..03b99826b52c 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -791,7 +791,12 @@ static int btmtk_usb_hci_wmt_sync(struct hci_dev *hdev,
case BTMTK_WMT_FUNC_CTRL:
if (!skb_pull_data(data->evt_skb,
sizeof(wmt_evt_funcc->status))) {
- status = BTMTK_WMT_ON_UNDONE;
+ /* A plain enable/disable request is acked with just
+ * the WMT header and no trailing status word; the
+ * result is carried in the header's own flag byte.
+ */
+ status = wmt_evt->whdr.flag ? BTMTK_WMT_ON_UNDONE :
+ BTMTK_WMT_ON_DONE;
break;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] Bluetooth: btmtksdio, btmtkuart: validate WMT event length before struct access
2026-09-11 10:42 [PATCH 0/3] Bluetooth: btmtk: firmware debug event routing and WMT FUNC_CTRL status fixes Chris Lu
2026-09-11 10:42 ` [PATCH 1/3] Bluetooth: btmtk: Route firmware debug event to the diag channel Chris Lu
2026-09-11 10:42 ` [PATCH 2/3] Bluetooth: btmtk: fix wrong status for short WMT FUNC_CTRL events Chris Lu
@ 2026-09-11 10:42 ` Chris Lu
2026-09-11 14:33 ` [PATCH 0/3] Bluetooth: btmtk: firmware debug event routing and WMT FUNC_CTRL status fixes Luiz Augusto von Dentz
3 siblings, 0 replies; 6+ messages in thread
From: Chris Lu @ 2026-09-11 10:42 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, Luiz Von Dentz
Cc: Sean Wang, Will Lee, SS Wu, linux-bluetooth, linux-kernel,
linux-mediatek, Chris Lu
btmtksdio.c and btmtkuart.c cast a received WMT event straight to
struct btmtk_hci_wmt_evt and read its op/flag fields without checking
the event is long enough to contain them, unlike btmtk.c. The
FUNC_CTRL case then further casts to struct btmtk_hci_wmt_evt_funcc
and reads its 2-byte status field, again without a length check.
Firmware that sends a short or malformed WMT event makes both drivers
read past the end of the received SKB.
Mirror btmtk.c: validate the base WMT header with skb_pull_data()
before touching any of its fields, and when a FUNC_CTRL event turns
out to be the short, header-only form (a plain enable/disable ack
with no status word), decode the result from the header's own flag
byte instead (0 = success, otherwise failure).
Verified setup on MT7920, MT7921, MT7922 and MT7925: no regression.
Fixes: 9aebfd4a2200 ("Bluetooth: mediatek: add support for MediaTek MT7663S and MT7668S SDIO devices")
Fixes: e0b67035a90b ("Bluetooth: mediatek: update the common setup between MT7622 and other devices")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Chris Lu <chris.lu@mediatek.com>
---
drivers/bluetooth/btmtksdio.c | 20 +++++++++++++++++++-
drivers/bluetooth/btmtkuart.c | 20 +++++++++++++++++++-
2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index fe4ca9395aa3..a8ebcd0c005c 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -217,7 +217,14 @@ static int mtk_hci_wmt_sync(struct hci_dev *hdev,
}
/* Parse and handle the return WMT event */
- wmt_evt = (struct btmtk_hci_wmt_evt *)bdev->evt_skb->data;
+ wmt_evt = skb_pull_data(bdev->evt_skb, sizeof(*wmt_evt));
+ if (!wmt_evt) {
+ bt_dev_err(hdev, "WMT event too short (%u bytes)",
+ bdev->evt_skb->len);
+ err = -EINVAL;
+ goto err_free_skb;
+ }
+
if (wmt_evt->whdr.op != hdr->op) {
bt_dev_err(hdev, "Wrong op received %d expected %d",
wmt_evt->whdr.op, hdr->op);
@@ -233,6 +240,17 @@ static int mtk_hci_wmt_sync(struct hci_dev *hdev,
status = BTMTK_WMT_PATCH_DONE;
break;
case BTMTK_WMT_FUNC_CTRL:
+ if (!skb_pull_data(bdev->evt_skb,
+ sizeof(wmt_evt_funcc->status))) {
+ /* A plain enable/disable request is acked with just
+ * the WMT header and no trailing status word; the
+ * result is carried in the header's own flag byte.
+ */
+ status = wmt_evt->whdr.flag ? BTMTK_WMT_ON_UNDONE :
+ BTMTK_WMT_ON_DONE;
+ break;
+ }
+
wmt_evt_funcc = (struct btmtk_hci_wmt_evt_funcc *)wmt_evt;
if (be16_to_cpu(wmt_evt_funcc->status) == 0x404)
status = BTMTK_WMT_ON_DONE;
diff --git a/drivers/bluetooth/btmtkuart.c b/drivers/bluetooth/btmtkuart.c
index 27aa48ff3ac2..4af6fbbbd302 100644
--- a/drivers/bluetooth/btmtkuart.c
+++ b/drivers/bluetooth/btmtkuart.c
@@ -151,7 +151,14 @@ static int mtk_hci_wmt_sync(struct hci_dev *hdev,
}
/* Parse and handle the return WMT event */
- wmt_evt = (struct btmtk_hci_wmt_evt *)bdev->evt_skb->data;
+ wmt_evt = skb_pull_data(bdev->evt_skb, sizeof(*wmt_evt));
+ if (!wmt_evt) {
+ bt_dev_err(hdev, "WMT event too short (%u bytes)",
+ bdev->evt_skb->len);
+ err = -EINVAL;
+ goto err_free_wc;
+ }
+
if (wmt_evt->whdr.op != hdr->op) {
bt_dev_err(hdev, "Wrong op received %d expected %d",
wmt_evt->whdr.op, hdr->op);
@@ -167,6 +174,17 @@ static int mtk_hci_wmt_sync(struct hci_dev *hdev,
status = BTMTK_WMT_PATCH_DONE;
break;
case BTMTK_WMT_FUNC_CTRL:
+ if (!skb_pull_data(bdev->evt_skb,
+ sizeof(wmt_evt_funcc->status))) {
+ /* A plain enable/disable request is acked with just
+ * the WMT header and no trailing status word; the
+ * result is carried in the header's own flag byte.
+ */
+ status = wmt_evt->whdr.flag ? BTMTK_WMT_ON_UNDONE :
+ BTMTK_WMT_ON_DONE;
+ break;
+ }
+
wmt_evt_funcc = (struct btmtk_hci_wmt_evt_funcc *)wmt_evt;
if (be16_to_cpu(wmt_evt_funcc->status) == 0x404)
status = BTMTK_WMT_ON_DONE;
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] Bluetooth: btmtk: firmware debug event routing and WMT FUNC_CTRL status fixes
2026-09-11 10:42 [PATCH 0/3] Bluetooth: btmtk: firmware debug event routing and WMT FUNC_CTRL status fixes Chris Lu
` (2 preceding siblings ...)
2026-09-11 10:42 ` [PATCH 3/3] Bluetooth: btmtksdio, btmtkuart: validate WMT event length before struct access Chris Lu
@ 2026-09-11 14:33 ` Luiz Augusto von Dentz
3 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2026-09-11 14:33 UTC (permalink / raw)
To: Chris Lu
Cc: Marcel Holtmann, Johan Hedberg, Sean Wang, Will Lee, SS Wu,
linux-bluetooth, linux-kernel, linux-mediatek
Hi Chris,
On Fri, Sep 11, 2026 at 6:42 AM Chris Lu <chris.lu@mediatek.com> wrote:
>
> This series bundles three independent MediaTek Bluetooth driver fixes:
>
> Patch 1 is a resend of a fix submitted on 25 Aug 2026
> ("Bluetooth: btmtk: Route firmware debug event to the diag channel")
> that received no review feedback. There are no code changes since
> that submission; resending it alongside the two related fixes below.
>
> Patches 2-3 fix how btmtk_usb_hci_wmt_sync() (and its btmtksdio.c /
> btmtkuart.c counterparts) interpret a WMT FUNC_CTRL event that carries
> only the WMT header and no trailing 2-byte status word. Such an event
> is a normal firmware ack for a plain enable/disable request, with the
> result carried in the header's own flag byte, not a failure as the
> current code assumes:
>
> - Patch 2 fixes this for btmtk.c, where a bounds check already
> existed (added by e3ac0d9f1a20) but defaulted to the wrong
> result.
> - Patch 3 applies the same fix to btmtksdio.c and btmtkuart.c, which
> never had a bounds check for this event at all and read 2 bytes
> past the end of the received SKB whenever firmware sent the short
> form. While there, it also adds the missing base WMT header length
> check that btmtk.c already has (skb_pull_data() before touching
> wmt_evt->whdr.op), since these two files were unconditionally
> dereferencing that field with no length validation at all.
>
> Chris Lu (3):
> Bluetooth: btmtk: Route firmware debug event to the diag channel
> Bluetooth: btmtk: fix wrong status for short WMT FUNC_CTRL events
> Bluetooth: btmtksdio, btmtkuart: validate WMT event length before
> struct access
>
> drivers/bluetooth/btmtk.c | 8 +++++++-
> drivers/bluetooth/btmtksdio.c | 20 +++++++++++++++++++-
> drivers/bluetooth/btmtkuart.c | 19 ++++++++++++++++++-
> 3 files changed, 44 insertions(+), 3 deletions(-)
>
> --
> 2.45.2
Sashiko flagged a problem regarding the usage of ACL connection handle
without masking the PB field:
https://sashiko.dev/#/patchset/20260911104234.2276126-1-chris.lu%40mediatek.com
If the HCI fragmentation doesn't apply to these handles, please add a
comment regarding it; otherwise, users like Sashiko will keep flagging
it going forward.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread