1. Patchew
  2. linux
  3. View series

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

Chris Lu posted 1 patch 2 months, 1 week ago
There is a newer version of this series
drivers/bluetooth/btmtk.c | 22 ++++++++++++++++++++++
drivers/bluetooth/btmtk.h |  7 +++++++
drivers/bluetooth/btusb.c |  2 ++
3 files changed, 31 insertions(+)
[PATCH v2] Bluetooth: btmtk: add event filter to filter specific event
Posted by Chris Lu 2 months, 1 week ago 
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@mediatek.com>
---
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@mediatek.com>");
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
Re: [PATCH v2] Bluetooth: btmtk: add event filter to filter specific event
Posted by Luiz Augusto von Dentz 2 months, 1 week ago 
Hi Chris,

On Tue, Apr 7, 2026 at 7:48 AM Chris Lu <chris.lu@mediatek.com> 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@mediatek.com>
> ---
> 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@mediatek.com>");
> 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
Re: [PATCH v2] Bluetooth: btmtk: add event filter to filter specific event
Posted by Chris Lu (陸稚泓) 2 months, 1 week ago 
Hi Luiz,

Thanks for your reply.

On Tue, 2026-04-07 at 10:54 -0400, Luiz Augusto von Dentz wrote:
> Hi Chris,
> 
> On Tue, Apr 7, 2026 at 7:48 AM Chris Lu <chris.lu@mediatek.com>
> 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@mediatek.com>
> > ---
> > 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@mediatek.com>");
> > 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://urldefense.com/v3/__https://sashiko.dev/*/patchset/20260407114851.3087886-1-chris.lu*40mediatek.com__;IyU!!CTRNKA9wMg0ARbw!j4c9a9a8N0yC-uPVxzP3piQN8Ga4aEG1gE7ZMtTFeFYFzkNiq08Zd_XHi9ImhiQTSWTjKP-XoRutGQU0yKk$
>  
> 
> 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.
> 
> 

For the approach you mentioned, I think it's a better long-term
direction but it seems that this mechanism hasn't been implement yet,
or is there already existing an structure/API in kernel that I can
refer to?

So far I refer to the methods used by other vendor to attach the event
filter in driver setup stage. I'll send a v3 addressing the review
comment in above website after doing some verification locally.

Thanks,
Chris Lu

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.