Re: [PATCH] Bluetooth: fix socket matching ambiguity between BIS and CIS

[Yang Li <yang.li@xxxxxxxxxxx>]

Hi Paul,

> [ EXTERNAL EMAIL ]
>
> Dear Yang,
>
>
> Thank you for your patch.
>
> Am 07.05.25 um 09:30 schrieb Yang Li via B4 Relay:
> > From: Yang Li <yang.li@xxxxxxxxxxx>
>
> It’d be great if you could start by describing the problem.

Sorry for the confusion—I initially thought the linked BlueZ issue 
provided enough detail.

To clarify: the problem occurs when the DUT is acting as a sink device. 
If a BIS already exists, and a CIS  connection is then created, the 
kernel mistakenly references the BIS socket. This happens because the 
socket selection only checks for |state == BT_LISTEN|, without further 
distinguishing between BIS and CIS contexts.

To resolve this, I believe the destination address (|dst addr|) should 
also be matched when retrieving the ISO socket, so BIS and CIS sockets 
can be properly differentiated.

> > The iso_get_sock function adds dst address matching to
> > distinguish BIS and CIS sockets.
> >
> > Link: https://github.com/bluez/bluez/issues/1224
>
> How can this patch be tested?


DUT environment：BlueZ 5.82+pipewire 1.3.83+kernel 6.12

Assistant：Redmi K70 phone

BIS Source：1BIG 2BISes

Steps to reproduce:
1> Use K70 mobile phone to connect to DUT, and configure BIS source for 
DUT as Assistant, and DUT audio plays normally;
2> Do not end listening, K70 mobile phone plays music, DUT still uses 
BIS source audio, check HCI log, CIS link is disconnected；

After testing, the modification resolves the issue I encountered. 
However, I'm not certain whether this approach might introduce new 
issues elsewhere.

> > Signed-off-by: Yang Li <yang.li@xxxxxxxxxxx>
> > ---
> >   net/bluetooth/hci_event.c | 35 ++++++++++++++++++++---------------
> >   net/bluetooth/iso.c       | 12 +++++++++---
> >   2 files changed, 29 insertions(+), 18 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 66052d6aaa1d..c1f32e98ef8a 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -6413,6 +6413,8 @@ static void 
> > hci_le_pa_sync_estabilished_evt(struct hci_dev *hdev, void *data,
> >
> >       conn->sync_handle = le16_to_cpu(ev->handle);
> >       conn->sid = HCI_SID_INVALID;
> > +     conn->dst = ev->bdaddr;
> > +     conn->dst_type = ev->bdaddr_type;
> >
> >       mask |= hci_proto_connect_ind(hdev, &ev->bdaddr, BIS_LINK,
> >                                     &flags);
> > @@ -6425,7 +6427,8 @@ static void 
> > hci_le_pa_sync_estabilished_evt(struct hci_dev *hdev, void *data,
> >               goto unlock;
> >
> >       /* Add connection to indicate PA sync event */
> > -     pa_sync = hci_conn_add_unset(hdev, BIS_LINK, BDADDR_ANY,
> > +
>
> Why the extra blank line?


Sorry, I will remove this modification in the next version.

> > +     pa_sync = hci_conn_add_unset(hdev, BIS_LINK, &ev->bdaddr,
> >                                    HCI_ROLE_SLAVE);
> >
> >       if (IS_ERR(pa_sync))
>
>
> Kind regards,
>
> Paul
>
>
> > @@ -6456,13 +6459,6 @@ static void hci_le_per_adv_report_evt(struct 
> > hci_dev *hdev, void *data,
> >
> >       hci_dev_lock(hdev);
> >
> > -     mask |= hci_proto_connect_ind(hdev, BDADDR_ANY, BIS_LINK, &flags);
> > -     if (!(mask & HCI_LM_ACCEPT))
> > -             goto unlock;
> > -
> > -     if (!(flags & HCI_PROTO_DEFER))
> > -             goto unlock;
> > -
> >       pa_sync = hci_conn_hash_lookup_pa_sync_handle
> >                       (hdev,
> >                       le16_to_cpu(ev->sync_handle));
> > @@ -6470,6 +6466,13 @@ static void hci_le_per_adv_report_evt(struct 
> > hci_dev *hdev, void *data,
> >       if (!pa_sync)
> >               goto unlock;
> >
> > +     mask |= hci_proto_connect_ind(hdev, &pa_sync->dst, ISO_LINK, 
> > &flags);
> > +     if (!(mask & HCI_LM_ACCEPT))
> > +             goto unlock;
> > +
> > +     if (!(flags & HCI_PROTO_DEFER))
> > +             goto unlock;
> > +
> >       if (ev->data_status == LE_PA_DATA_COMPLETE &&
> >           !test_and_set_bit(HCI_CONN_PA_SYNC, &pa_sync->flags)) {
> >               /* Notify iso layer */
> > @@ -6993,6 +6996,8 @@ static void 
> > hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data,
> >                       set_bit(HCI_CONN_PA_SYNC, &bis->flags);
> >
> >               bis->sync_handle = conn->sync_handle;
> > +             bis->dst = conn->dst;
> > +             bis->dst_type = conn->dst_type;
> >               bis->iso_qos.bcast.big = ev->handle;
> >               memset(&interval, 0, sizeof(interval));
> >               memcpy(&interval, ev->latency, sizeof(ev->latency));
> > @@ -7038,13 +7043,6 @@ static void 
> > hci_le_big_info_adv_report_evt(struct hci_dev *hdev, void *data,
> >
> >       hci_dev_lock(hdev);
> >
> > -     mask |= hci_proto_connect_ind(hdev, BDADDR_ANY, BIS_LINK, &flags);
> > -     if (!(mask & HCI_LM_ACCEPT))
> > -             goto unlock;
> > -
> > -     if (!(flags & HCI_PROTO_DEFER))
> > -             goto unlock;
> > -
> >       pa_sync = hci_conn_hash_lookup_pa_sync_handle
> >                       (hdev,
> >                       le16_to_cpu(ev->sync_handle));
> > @@ -7054,6 +7052,13 @@ static void 
> > hci_le_big_info_adv_report_evt(struct hci_dev *hdev, void *data,
> >
> >       pa_sync->iso_qos.bcast.encryption = ev->encryption;
> >
> > +     mask |= hci_proto_connect_ind(hdev, &pa_sync->dst, ISO_LINK, 
> > &flags);
> > +     if (!(mask & HCI_LM_ACCEPT))
> > +             goto unlock;
> > +
> > +     if (!(flags & HCI_PROTO_DEFER))
> > +             goto unlock;
> > +
> >       /* Notify iso layer */
> >       hci_connect_cfm(pa_sync, 0);
> >
> > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> > index 6e2c752aaa8f..1dc233f04dbe 100644
> > --- a/net/bluetooth/iso.c
> > +++ b/net/bluetooth/iso.c
> > @@ -641,11 +641,12 @@ static struct sock *iso_get_sock(bdaddr_t *src, 
> > bdaddr_t *dst,
> >                       continue;
> >
> >               /* Exact match. */
> > -             if (!bacmp(&iso_pi(sk)->src, src)) {
> > +             if (!bacmp(&iso_pi(sk)->src, src)
> > +                  && !bacmp(&iso_pi(sk)->dst, dst)
> > +                     ){
> >                       sock_hold(sk);
> >                       break;
> >               }
> > -
> >               /* Closest match */
> >               if (!bacmp(&iso_pi(sk)->src, BDADDR_ANY)) {
> >                       if (sk1)
> > @@ -1962,7 +1963,7 @@ static void iso_conn_ready(struct iso_conn *conn)
> >               }
> >
> >               if (!parent)
> > -                     parent = iso_get_sock(&hcon->src, BDADDR_ANY,
> > +                     parent = iso_get_sock(&hcon->src, &hcon->dst,
> >                                             BT_LISTEN, NULL, NULL);
> >
> >               if (!parent)
> > @@ -2203,6 +2204,11 @@ int iso_connect_ind(struct hci_dev *hdev, 
> > bdaddr_t *bdaddr, __u8 *flags)
> >       } else {
> >               sk = iso_get_sock(&hdev->bdaddr, BDADDR_ANY,
> >                                 BT_LISTEN, NULL, NULL);
> > +             if (!sk)
> > +                     sk = iso_get_sock(&hdev->bdaddr, bdaddr,
> > +                                       BT_LISTEN, NULL, NULL);
> > +             else
> > +                     iso_pi(sk)->dst = *bdaddr;
> >       }
> >
> >   done: