On Wed, Dec 4, 2024 at 11:58 AM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>
> The usage of rcu_read_(un)lock while inside list_for_each_entry_rcu is
> not safe since for the most part entries fetched this way shall be
> treated as rcu_dereference:
>
> Note that the value returned by rcu_dereference() is valid
> only within the enclosing RCU read-side critical section [1]_.
> For example, the following is **not** legal::
>
> rcu_read_lock();
> p = rcu_dereference(head.next);
> rcu_read_unlock();
> x = p->address; /* BUG!!! */
> rcu_read_lock();
> y = p->data; /* BUG!!! */
> rcu_read_unlock();
>
> Fixes: a0bfde167b50 ("Bluetooth: ISO: Add support for connecting multiple BISes")
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> ---
> net/bluetooth/hci_event.c | 33 +++++++++++----------------------
> 1 file changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index aca121408369..0d59f2cfb9a4 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -6872,38 +6872,27 @@ static void hci_le_create_big_complete_evt(struct hci_dev *hdev, void *data,
> return;
>
> hci_dev_lock(hdev);
> - rcu_read_lock();
>
> /* Connect all BISes that are bound to the BIG */
> - list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) {
> - if (bacmp(&conn->dst, BDADDR_ANY) ||
> - conn->type != ISO_LINK ||
> - conn->iso_qos.bcast.big != ev->handle)
> + while ((conn = hci_conn_hash_lookup_big_state(hdev, ev->handle,
> + BT_BOUND))) {
> + if (ev->status) {
> + hci_connect_cfm(conn, ev->status);
> + hci_conn_del(conn);
> continue;
> + }
>
> if (hci_conn_set_handle(conn,
> __le16_to_cpu(ev->bis_handle[i++])))
> continue;
>
> - if (!ev->status) {
> - conn->state = BT_CONNECTED;
> - set_bit(HCI_CONN_BIG_CREATED, &conn->flags);
> - rcu_read_unlock();
> - hci_debugfs_create_conn(conn);
> - hci_conn_add_sysfs(conn);
> - hci_iso_setup_path(conn);
> - rcu_read_lock();
> - continue;
> - }
> -
> - hci_connect_cfm(conn, ev->status);
> - rcu_read_unlock();
> - hci_conn_del(conn);
> - rcu_read_lock();
> + conn->state = BT_CONNECTED;
> + set_bit(HCI_CONN_BIG_CREATED, &conn->flags);
> + hci_debugfs_create_conn(conn);
> + hci_conn_add_sysfs(conn);
> + hci_iso_setup_path(conn);
> }
>
> - rcu_read_unlock();
> -
> if (!ev->status && !i)
> /* If no BISes have been connected for the BIG,
> * terminate. This is in case all bound connections
> --
> 2.47.1
>
--
Luiz Augusto von Dentz
