Re: [PATCH] Bluetooth: btmtk: accept too short WMT FUNC_CTRL events

[Pauli Virtanen <pav@iki.fi>]

Hi Luiz,

pe, 2026-04-24 kello 15:38 -0400, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli, Tristan,
> 
> On Fri, Apr 24, 2026 at 3:25 PM Pauli Virtanen <pav@iki.fi> wrote:
> > 
> > MT7925 (USB ID 0e8d:e025) on fw version 20260106153314 sends WMT
> > FUNC_CTRL events that are missing the status field.
> > 
> > Prior to commit 006b9943b982 ("Bluetooth: btmtk: validate WMT event SKB
> > length before struct access") the status was read from out-of-bounds of
> > SKB data, which usually would result to success with
> > BTMTK_WMT_ON_UNDONE, although I don't know the intent here.  The bounds
> > check added in that commit returns with error instead, producing
> > "Bluetooth: hci0: Failed to send wmt func ctrl (-22)" and makes the
> > device unusable.
> > 
> > Fix the regression by interpreting too short packet as status
> > BTMTK_WMT_ON_UNDONE, which makes the device work normally again.
> > 
> > Fixes: 006b9943b982 ("Bluetooth: btmtk: validate WMT event SKB length before struct access")
> > Signed-off-by: Pauli Virtanen <pav@iki.fi>
> > ---
> > 
> > Notes:
> >     AFAICS the commit is not yet pulled and is only in bluetooth-next, so
> >     maybe this should be just fixup?
> 
> Yeah, I'll most likely fix it in place and add your Signed-off-by.

Looks like this got pulled to net without this fix, so it's broken now

> 
> >  drivers/bluetooth/btmtk.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> > index ab34f1dd42bc..68a32d11e5ec 100644
> > --- a/drivers/bluetooth/btmtk.c
> > +++ b/drivers/bluetooth/btmtk.c
> > @@ -719,8 +719,8 @@ 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))) {
> > -                       err = -EINVAL;
> > -                       goto err_free_skb;
> > +                       status = BTMTK_WMT_ON_UNDONE;
> > +                       break;
> 
> This probably means the original change was never tested on real
> hardware. We likely need input from the MediaTek team on how to handle
> these events, as I don't think a public spec exists.
> 
> >                 }
> > 
> >                 wmt_evt_funcc = (struct btmtk_hci_wmt_evt_funcc *)wmt_evt;
> > --
> > 2.53.0
> > 
> 

-- 
Pauli Virtanen