linux-kernel - Re: [PATCH v3] Bluetooth: iso: fix socket matching ambiguity between BIS and CIS

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] [day] [month] [year] [list]
Message-ID: <5874e307-fa81-4baf-b270-478128d30412@amlogic.com>
Date: Wed, 3 Sep 2025 15:58:14 +0800
From: Yang Li <yang.li@...ogic.com>
To: Luiz Augusto von Dentz <luiz.dentz@...il.com>
Cc: Marcel Holtmann <marcel@...tmann.org>,
 Johan Hedberg <johan.hedberg@...il.com>, linux-bluetooth@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] Bluetooth: iso: fix socket matching ambiguity between
 BIS and CIS

Hi Luiz,
> [ EXTERNAL EMAIL ]
>
> Hi Yang,
>
> On Sun, Aug 3, 2025 at 9:07 PM Yang Li <yang.li@...ogic.com> wrote:
>> Hi Luiz,
>>> [ EXTERNAL EMAIL ]
>>>
>>> Hi Yang,
>>>
>>> On Thu, Jul 31, 2025 at 4:00 AM Yang Li via B4 Relay
>>> <devnull+yang.li.amlogic.com@...nel.org> wrote:
>>>> From: Yang Li <yang.li@...ogic.com>
>>>>
>>>> When both BIS and CIS links exist, their sockets are in
>>>> the BT_LISTEN state.
>>> We probably need to introduce tests to iso-test that setup both then
>>> to avoid reintroducing the problem.
>>
>> Since the coexistence of BIS sink and CIS sink is determined by
>> application-level logic, it may be difficult to reproduce this scenario
>> using iso-test.
> Looks like you haven't look at what iso-tester tools tests do, that is
> not tight to bluetoothd, it directly operates at the socket layer so
> we can create any scenario we want.


Based on the current structure of iso-tester, it is not possible to 
implement test cases where CIS and BIS listen simultaneously. There are 
several issues:

 1.

    In struct iso_client_data, although both CIS and BIS related
    elements are defined, they are mutually exclusive. CIS and BIS
    cannot be used at the same time. For example, .bcast must explicitly
    specify whether it is broadcast or unicast.

 2.

    The setup_listen_many function also identifies BIS or CIS through
    .bcast.

Therefore, if we want to add test cases for the coexistence of BIS and 
CIS, the current data structure needs to be modified to completely 
separate the elements for BIS and CIS.


>> Do you have any suggestions on how to simulate or test this case more
>> effectively?
>>
>>>> dump sock:
>>>>     sk 000000001977ef51 state 6
>>>>     src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>>>>     sk 0000000031d28700 state 7
>>>>     src 10:a5:62:31:05:cf dst00:00:00:00:00:00
>>>>     sk 00000000613af00e state 4   # listen sock of bis
>>>>     src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>>>>     sk 000000001710468c state 9
>>>>     src 10:a5:62:31:05:cf dst 54:00:00:d4:99:30
>>>>     sk 000000005d97dfde state 4   #listen sock of cis
>>>>     src 10:a5:62:31:05:cf dst 00:00:00:00:00:00
>>>>
>>>> To locate the CIS socket correctly, check both the BT_LISTEN
>>>> state and whether dst addr is BDADDR_ANY.
>>>>
>>>> Link: https://github.com/bluez/bluez/issues/1224
>>>>
>>>> Signed-off-by: Yang Li <yang.li@...ogic.com>
>>>> ---
>>>>    net/bluetooth/iso.c | 9 +++++++--
>>>>    1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
>>>> index eaffd25570e3..9a4dea03af8c 100644
>>>> --- a/net/bluetooth/iso.c
>>>> +++ b/net/bluetooth/iso.c
>>>> @@ -1919,6 +1919,11 @@ static bool iso_match_pa_sync_flag(struct sock *sk, void *data)
>>>>           return test_bit(BT_SK_PA_SYNC, &iso_pi(sk)->flags);
>>>>    }
>>>>
>>>> +static bool iso_match_dst(struct sock *sk, void *data)
>>>> +{
>>>> +       return !bacmp(&iso_pi(sk)->dst, (bdaddr_t *)data);
>>>> +}
>>>> +
>>>>    static void iso_conn_ready(struct iso_conn *conn)
>>>>    {
>>>>           struct sock *parent = NULL;
>>>> @@ -1981,7 +1986,7 @@ static void iso_conn_ready(struct iso_conn *conn)
>>>>
>>>>                   if (!parent)
>>>>                           parent = iso_get_sock(&hcon->src, BDADDR_ANY,
>>>> -                                             BT_LISTEN, NULL, NULL);
>>>> +                                             BT_LISTEN, iso_match_dst, BDADDR_ANY);
>>>>
>>>>                   if (!parent)
>>>>                           return;
>>>> @@ -2220,7 +2225,7 @@ 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);
>>>> +                                 BT_LISTEN, iso_match_dst, BDADDR_ANY);
>>> Perhaps we should add helper function that wrap the iso_get_sock (e.g.
>>> iso_get_sock_cis and iso_get_sock_bis) to make it clearer what is the
>>> expected socket type, also if the hcon has been set perhaps that
>>> should be matched as well with CIS_LINK/BIS_LINK, or perhaps we
>>> introduce a socket type to differentiate since the use of the address
>>> can make the logic a little confusing when the socket types are mixed
>>> together.
>>>
>>> Now looking at the source code perhaps we can have a separate list for
>>> cis and bis sockets instead of global iso_sk_list (e.g. cis_sk_list
>>> and bis_sk_list), that way we don't need a type and there is no risk
>>> of confusing the sockets since they would never be in the same list.
>>
>> Alright, I will give it a try.
>>
>>>>           }
>>>>
>>>>    done:
>>>>
>>>> ---
>>>> base-commit: 9c533991fe15be60ad9f9a7629c25dbc5b09788d
>>>> change-id: 20250731-bis_cis_coexist-717a442d5c42
>>>>
>>>> Best regards,
>>>> --
>>>> Yang Li <yang.li@...ogic.com>
>>>>
>>>>
>>> --
>>> Luiz Augusto von Dentz
>
>
> --
> Luiz Augusto von Dentz