[PATCH v1] Bluetooth: btmtk: adjust the position to init iso data anchor

[PATCH v1] Bluetooth: btmtk: adjust the position to init iso data anchor

[Chris Lu]

MediaTek iso data anchor init should be move to where MediaTek
claims iso data interface.
If there is an unexpected usb disconnect during setup flow,
it will cause a NULL pointer crash issue when releasing iso
anchor since the anchor wan't been init yet. Adjust the position
to do iso data anchor init.

Signed-off-by: Chris Lu <chris.lu@mediatek.com>
---
 drivers/bluetooth/btmtk.c | 1 -
 drivers/bluetooth/btusb.c | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index b7fc14aafc74..8a3f7c3fcfec 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -1215,7 +1215,6 @@ static int btmtk_usb_isointf_init(struct hci_dev *hdev)
 	struct sk_buff *skb;
 	int err;
 
-	init_usb_anchor(&btmtk_data->isopkt_anchor);
 	spin_lock_init(&btmtk_data->isorxlock);
 
 	__set_mtk_intr_interface(hdev);
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 9970470c9d15..15c0885c37cd 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2628,6 +2628,8 @@ static void btusb_mtk_claim_iso_intf(struct btusb_data *data)
 	struct btmtk_data *btmtk_data = hci_get_priv(data->hdev);
 	int err;
 
+	init_usb_anchor(&btmtk_data->isopkt_anchor);
+
 	err = usb_driver_claim_interface(&btusb_driver,
 					 btmtk_data->isopkt_intf, data);
 	if (err < 0) {
-- 
2.18.0

Re: [PATCH v1] Bluetooth: btmtk: adjust the position to init iso data anchor

[Luiz Augusto von Dentz]

Hi Chris,

On Wed, Oct 23, 2024 at 7:37 AM Chris Lu <chris.lu@mediatek.com> wrote:
>
> MediaTek iso data anchor init should be move to where MediaTek
> claims iso data interface.
> If there is an unexpected usb disconnect during setup flow,
> it will cause a NULL pointer crash issue when releasing iso
> anchor since the anchor wan't been init yet. Adjust the position
> to do iso data anchor init.
>
> Signed-off-by: Chris Lu <chris.lu@mediatek.com>

Please add the backtrace or a Link tag if there is an issue/bug open.
Also it is important to always include a Fixes tag with tha hash that
introduced the problem, specially in case of a crash since it might be
a good idea to backport to fix.

> ---
>  drivers/bluetooth/btmtk.c | 1 -
>  drivers/bluetooth/btusb.c | 2 ++
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> index b7fc14aafc74..8a3f7c3fcfec 100644
> --- a/drivers/bluetooth/btmtk.c
> +++ b/drivers/bluetooth/btmtk.c
> @@ -1215,7 +1215,6 @@ static int btmtk_usb_isointf_init(struct hci_dev *hdev)
>         struct sk_buff *skb;
>         int err;
>
> -       init_usb_anchor(&btmtk_data->isopkt_anchor);
>         spin_lock_init(&btmtk_data->isorxlock);
>
>         __set_mtk_intr_interface(hdev);
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 9970470c9d15..15c0885c37cd 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2628,6 +2628,8 @@ static void btusb_mtk_claim_iso_intf(struct btusb_data *data)
>         struct btmtk_data *btmtk_data = hci_get_priv(data->hdev);
>         int err;
>
> +       init_usb_anchor(&btmtk_data->isopkt_anchor);
> +
>         err = usb_driver_claim_interface(&btusb_driver,
>                                          btmtk_data->isopkt_intf, data);
>         if (err < 0) {
> --
> 2.18.0
>


-- 
Luiz Augusto von Dentz

Re: [PATCH v1] Bluetooth: btmtk: adjust the position to init iso data anchor

[Chris Lu (陸稚泓)]

Hi Luiz,

On Thu, 2024-10-24 at 10:47 -0400, Luiz Augusto von Dentz wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Hi Chris,
> 
> On Wed, Oct 23, 2024 at 7:37 AM Chris Lu <chris.lu@mediatek.com>
> wrote:
> >
> > MediaTek iso data anchor init should be move to where MediaTek
> > claims iso data interface.
> > If there is an unexpected usb disconnect during setup flow,
> > it will cause a NULL pointer crash issue when releasing iso
> > anchor since the anchor wan't been init yet. Adjust the position
> > to do iso data anchor init.
> >
> > Signed-off-by: Chris Lu <chris.lu@mediatek.com>
> 
> Please add the backtrace or a Link tag if there is an issue/bug open.
> Also it is important to always include a Fixes tag with tha hash that
> introduced the problem, specially in case of a crash since it might
> be
> a good idea to backport to fix.
> 
Sorry about that, I will make sure to include those information if
MediaTek fixes such Kernel crash issue in the future.

I've submitted v2 to add backtrace and Fixes tag information to commit
message. BTW, I also moved the position to do anchro init to the end of
same function in v2 which will make the flow more make sense.

Thanks a lot,
Chris Lu
> > ---
> >  drivers/bluetooth/btmtk.c | 1 -
> >  drivers/bluetooth/btusb.c | 2 ++
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
> > index b7fc14aafc74..8a3f7c3fcfec 100644
> > --- a/drivers/bluetooth/btmtk.c
> > +++ b/drivers/bluetooth/btmtk.c
> > @@ -1215,7 +1215,6 @@ static int btmtk_usb_isointf_init(struct
> hci_dev *hdev)
> >         struct sk_buff *skb;
> >         int err;
> >
> > -       init_usb_anchor(&btmtk_data->isopkt_anchor);
> >         spin_lock_init(&btmtk_data->isorxlock);
> >
> >         __set_mtk_intr_interface(hdev);
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 9970470c9d15..15c0885c37cd 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -2628,6 +2628,8 @@ static void btusb_mtk_claim_iso_intf(struct
> btusb_data *data)
> >         struct btmtk_data *btmtk_data = hci_get_priv(data->hdev);
> >         int err;
> >
> > +       init_usb_anchor(&btmtk_data->isopkt_anchor);
> > +
> >         err = usb_driver_claim_interface(&btusb_driver,
> >                                          btmtk_data->isopkt_intf,
> data);
> >         if (err < 0) {
> > --
> > 2.18.0
> >
> 
> 
> -- 
> Luiz Augusto von Dentz