| 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
| ||
|
Message-ID: <bc2bbcbc-75d8-4070-a133-8909c9b859dc@amlogic.com>
Date: Thu, 8 May 2025 10:06:46 +0800
From: Yang Li <yang.li@...ogic.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
Cc: Marcel Holtmann <marcel@...tmann.org>,
Johan Hedberg <johan.hedberg@...il.com>,
Luiz Augusto von Dentz <luiz.dentz@...il.com>,
linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Bluetooth: fix socket matching ambiguity between BIS and
CIS
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@...ogic.com>
>
> 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@...ogic.com>
>> ---
>> 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:
Powered by blists - more mailing lists