Re: [PATCH 0/3] Bluetooth: btmtk: firmware debug event routing and WMT FUNC_CTRL status fixes

["Chris Lu (陸稚泓)" <Chris.Lu@mediatek.com>]

Hi Luiz,

On Fri, 2026-09-11 at 10:33 -0400, Luiz Augusto von Dentz wrote:
> 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://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260911104234.2276126-1-chris.lu*40mediatek.com__;IyU!!CTRNKA9wMg0ARbw!jwNe1Tfud02zOuV-S3UGvy1vdJQtqffgxH1tvNsv8D4Qk5oGmd-B00PQYhbifDx9HwTx7Kr__RP2ZAjjnro$
>  
> 
> 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.
> 
> 

Confirmed with MediaTek internally: 0x0efd (and the other three
vendor-reserved handles already handled in this switch: 0xfc6f, 0x05ff,
0x05fe) is not a real connection handle -- it's used to multiplex
firmware debug/dump data onto the ACL channel. This event is always
sent as a single, complete ACL_START packet, so HCI fragmentation
(and thus a matching ACL_CONT case, 0x1efd) never applies to it.

Added a comment above the switch statement in both btmtk.c and
btmtksdio.c explaining this for all four handles, so it's clear going
forward. Will include it in the next version.

Thanks for catching this.

BRs,
Chris Lu