netdev - Re: [PATCH] Bluetooth: hci_event: Add support for handling LE BIG Sync Lost event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABBYNZLakMqxtJwzmpi2DuBg9ftzLutBKN8S-UEmwo9k9uek5g@mail.gmail.com>
Date: Tue, 24 Jun 2025 13:17:42 -0400
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: yang.li@...ogic.com
Cc: Marcel Holtmann <marcel@...tmann.org>, Johan Hedberg <johan.hedberg@...il.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Bluetooth: hci_event: Add support for handling LE BIG
 Sync Lost event

Hi,

On Tue, Jun 24, 2025 at 12:56 PM Luiz Augusto von Dentz
<luiz.dentz@...il.com> wrote:
>
> Hi,
>
> On Tue, Jun 24, 2025 at 1:20 AM Yang Li via B4 Relay
> <devnull+yang.li.amlogic.com@...nel.org> wrote:
> >
> > From: Yang Li <yang.li@...ogic.com>
> >
> > When the BIS source stops, the controller sends an LE BIG Sync Lost
> > event (subevent 0x1E). Currently, this event is not handled, causing
> > the BIS stream to remain active in BlueZ and preventing recovery.
> >
> > Signed-off-by: Yang Li <yang.li@...ogic.com>
> > ---
> >  include/net/bluetooth/hci.h |  6 ++++++
> >  net/bluetooth/hci_event.c   | 23 +++++++++++++++++++++++
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 82cbd54443ac..48389a64accb 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -2849,6 +2849,12 @@ struct hci_evt_le_big_sync_estabilished {
> >         __le16  bis[];
> >  } __packed;
> >
> > +#define HCI_EVT_LE_BIG_SYNC_LOST 0x1e
> > +struct hci_evt_le_big_sync_lost {
> > +       __u8    handle;
> > +       __u8    reason;
> > +} __packed;
> > +
> >  #define HCI_EVT_LE_BIG_INFO_ADV_REPORT 0x22
> >  struct hci_evt_le_big_info_adv_report {
> >         __le16  sync_handle;
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 66052d6aaa1d..730deaf1851f 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -7026,6 +7026,24 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
> >         hci_dev_unlock(hdev);
> >  }
> >
> > +static void hci_le_big_sync_lost_evt(struct hci_dev *hdev, void *data,
> > +                                           struct sk_buff *skb)
> > +{
> > +       struct hci_evt_le_big_sync_lost *ev = data;
> > +       struct hci_conn *conn;
> > +
> > +       bt_dev_dbg(hdev, "BIG Sync Lost: big_handle 0x%2.2x", ev->handle);
> > +
> > +       hci_dev_lock(hdev);
> > +
> > +       list_for_each_entry(conn, &hdev->conn_hash.list, list) {
> > +               if (test_bit(HCI_CONN_BIG_SYNC, &conn->flags))
> > +                       hci_disconn_cfm(conn, HCI_ERROR_REMOTE_USER_TERM);
> > +       }
>
> Let's start with the obvious problems:
>
> 1. This does not use the handle, instead it disconnects all the
> connections with HCI_CONN_BIG_SYNC
> 2. It doesn't use the reason either
> 3. hci_disconnect_cfm should be followed with hci_conn_del to free the hci_conn
>
> So this does tell me you don't fully understand what you are doing, I
> hope I am not dealing with some AI generated code otherwise I would
> just do it myself.

Btw, the spec does says the controller shall cleanup the connection
handle and data path:

When the HCI_LE_BIG_Sync_Lost event occurs, the Controller shall
remove the connection handle(s) and data paths of all BIS(s) in the
BIG with which the Controller was synchronized.

I wonder if that shall be interpreted as no HCI_Disconnection_Complete
shall be generated or what, also we might need to implement this into
BlueZ emulator in order to replicate this in our CI tests.

It seems we are not sending anything to the remote devices when
receiving BT_HCI_CMD_LE_BIG_TERM_SYNC:

https://github.com/bluez/bluez/blob/master/emulator/btdev.c#L6661

> > +       hci_dev_unlock(hdev);
> > +}
> > +
> >  static void hci_le_big_info_adv_report_evt(struct hci_dev *hdev, void *data,
> >                                            struct sk_buff *skb)
> >  {
> > @@ -7149,6 +7167,11 @@ static const struct hci_le_ev {
> >                      hci_le_big_sync_established_evt,
> >                      sizeof(struct hci_evt_le_big_sync_estabilished),
> >                      HCI_MAX_EVENT_SIZE),
> > +       /* [0x1e = HCI_EVT_LE_BIG_SYNC_LOST] */
> > +       HCI_LE_EV_VL(HCI_EVT_LE_BIG_SYNC_LOST,
> > +                    hci_le_big_sync_lost_evt,
> > +                    sizeof(struct hci_evt_le_big_sync_lost),
> > +                    HCI_MAX_EVENT_SIZE),
> >         /* [0x22 = HCI_EVT_LE_BIG_INFO_ADV_REPORT] */
> >         HCI_LE_EV_VL(HCI_EVT_LE_BIG_INFO_ADV_REPORT,
> >                      hci_le_big_info_adv_report_evt,
> >
> > ---
> > base-commit: bd35cd12d915bc410c721ba28afcada16f0ebd16
> > change-id: 20250612-handle_big_sync_lost_event-4c7dc64390a2
> >
> > Best regards,
> > --
> > Yang Li <yang.li@...ogic.com>
> >
> >
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz