Re: [PATCH v2] Bluetooth: btmtk: add event filter to filter specific event

[Luiz Augusto von Dentz]

Hi Chris,

On Tue, Apr 7, 2026 at 7:48 AM Chris Lu <chris.lu@xxxxxxxxxxxx> wrote:
>
> Add an event filter to filter event with specific opcode to prevent BT
> stack from receiving unexpected event.
>
> Event with opcode 0xfc5d is generated when MediaTek's Bluetooth enable
> firmware logs and is not expected to be sent to userspace.
>
> Signed-off-by: Chris Lu <chris.lu@xxxxxxxxxxxx>
> ---
> v1 -> v2: update commit message
> ---
>  drivers/bluetooth/btmtk.c | 22 ++++++++++++++++++++++
>  drivers/bluetooth/btmtk.h |  7 +++++++
>  drivers/bluetooth/btusb.c |  2 ++
>  3 files changed, 31 insertions(+)
>
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> index 55516b4602db..302d6ddf9062 100644
> --- a/drivers/bluetooth/btmtk.c
> +++ b/drivers/bluetooth/btmtk.c
> @@ -1503,6 +1503,28 @@ int btmtk_usb_shutdown(struct hci_dev *hdev)
>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(btmtk_usb_shutdown);
> +
> +int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       struct hci_event_hdr *hdr = (void *)skb->data;
> +
> +       if (hdr->evt == HCI_EV_CMD_COMPLETE) {
> +               struct hci_ev_cmd_complete *ec;
> +               u16 opcode;
> +
> +               ec = (void *)(skb->data + HCI_EVENT_HDR_SIZE);
> +               opcode = __le16_to_cpu(ec->opcode);
> +
> +               /* Filter vendor opcode */
> +               if (opcode == 0xfc5d) {
> +                       kfree_skb(skb);
> +                       return 0;
> +               }
> +       }
> +
> +       return hci_recv_frame(hdev, skb);
> +}
> +EXPORT_SYMBOL_GPL(btmtk_recv_event);
>  #endif
>
>  MODULE_AUTHOR("Sean Wang <sean.wang@xxxxxxxxxxxx>");
> diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
> index adaf385626ee..08b73927c8f3 100644
> --- a/drivers/bluetooth/btmtk.h
> +++ b/drivers/bluetooth/btmtk.h
> @@ -218,6 +218,8 @@ int btmtk_usb_suspend(struct hci_dev *hdev);
>  int btmtk_usb_setup(struct hci_dev *hdev);
>
>  int btmtk_usb_shutdown(struct hci_dev *hdev);
> +
> +int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb);
>  #else
>
>  static inline int btmtk_set_bdaddr(struct hci_dev *hdev,
> @@ -296,4 +298,9 @@ static inline int btmtk_usb_shutdown(struct hci_dev *hdev)
>  {
>         return -EOPNOTSUPP;
>  }
> +
> +static inline int btmtk_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       return -EOPNOTSUPP;
> +}
>  #endif
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index f9d515ee9124..daf8a387e660 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -4150,6 +4150,8 @@ static int btusb_probe(struct usb_interface *intf,
>         } else if (id->driver_info & BTUSB_MEDIATEK) {
>                 /* Allocate extra space for Mediatek device */
>                 priv_size += sizeof(struct btmtk_data);
> +
> +               data->recv_event = btmtk_recv_event;
>         }
>
>         data->recv_acl = hci_recv_frame;
> --
> 2.45.2


https://sashiko.dev/#/patchset/20260407114851.3087886-1-chris.lu%40mediatek.com

Both issues seem valid to me. That said, I wonder if it wouldn't have
been better to allow drivers to register with its own struct
hci_ev/struct hci_cc tables, that way the driver can extend event
parsing rather than intercepting it and performing custom parsing
which can lead to bugs like the ones mentioned above.


-- 
Luiz Augusto von Dentz