2023-09-27 14:48:22

by Iulia Tanasescu

[permalink] [raw]
Subject: [PATCH 0/1] Bluetooth: ISO: Fix invalid context error

This patch fixes the error introduced by commit a0bfde167b50
("Bluetooth: ISO: Add support for connecting multiple BISes"):

BUG: sleeping function called from invalid context in __hci_cmd_sync_sk

When handling the Create BIG Complete event, in case no bound BISes
have been found for the BIG handle, the hci_le_terminate_big_sync
call should be made from the cmd_sync_work, not from the rx_work.

Iulia Tanasescu (1):
Bluetooth: ISO: Fix invalid context error

net/bluetooth/hci_event.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)


base-commit: 091e25d6b54992d1d702ae91cbac139d4c243251
--
2.39.2


2023-09-27 17:11:02

by Iulia Tanasescu

[permalink] [raw]
Subject: [PATCH 1/1] Bluetooth: ISO: Fix invalid context error

This moves the hci_le_terminate_big_sync call from rx_work
to cmd_sync_work, to avoid calling sleeping function from
an invalid context.

Signed-off-by: Iulia Tanasescu <[email protected]>
---
net/bluetooth/hci_event.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d242f956dea8..640921358e5f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -7020,12 +7020,26 @@ static void hci_le_cis_req_evt(struct hci_dev *hdev, void *data,
hci_dev_unlock(hdev);
}

+static int hci_iso_term_big_sync(struct hci_dev *hdev, void *data)
+{
+ __u8 *handle = data;
+
+ return hci_le_terminate_big_sync(hdev, *handle,
+ HCI_ERROR_LOCAL_HOST_TERM);
+}
+
+static void hci_iso_term_big_destroy(struct hci_dev *hdev, void *data, int err)
+{
+ kfree(data);
+}
+
static void hci_le_create_big_complete_evt(struct hci_dev *hdev, void *data,
struct sk_buff *skb)
{
struct hci_evt_le_create_big_complete *ev = data;
struct hci_conn *conn;
__u8 i = 0;
+ __u8 *big_handle;

BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);

@@ -7064,16 +7078,25 @@ static void hci_le_create_big_complete_evt(struct hci_dev *hdev, void *data,
rcu_read_lock();
}

- if (!ev->status && !i)
+ rcu_read_unlock();
+
+ if (!ev->status && !i) {
/* If no BISes have been connected for the BIG,
* terminate. This is in case all bound connections
* have been closed before the BIG creation
* has completed.
*/
- hci_le_terminate_big_sync(hdev, ev->handle,
- HCI_ERROR_LOCAL_HOST_TERM);
+ big_handle = kzalloc(sizeof(*big_handle), GFP_KERNEL);
+ if (!big_handle)
+ goto unlock;

- rcu_read_unlock();
+ *big_handle = ev->handle;
+
+ hci_cmd_sync_queue(hdev, hci_iso_term_big_sync, big_handle,
+ hci_iso_term_big_destroy);
+ }
+
+unlock:
hci_dev_unlock(hdev);
}

--
2.39.2

2023-09-28 00:12:17

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/1] Bluetooth: ISO: Fix invalid context error

Hi Iulia,

On Wed, Sep 27, 2023 at 4:37 AM Iulia Tanasescu <[email protected]> wrote:
>
> This moves the hci_le_terminate_big_sync call from rx_work
> to cmd_sync_work, to avoid calling sleeping function from
> an invalid context.
>
> Signed-off-by: Iulia Tanasescu <[email protected]>
> ---
> net/bluetooth/hci_event.c | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index d242f956dea8..640921358e5f 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -7020,12 +7020,26 @@ static void hci_le_cis_req_evt(struct hci_dev *hdev, void *data,
> hci_dev_unlock(hdev);
> }
>
> +static int hci_iso_term_big_sync(struct hci_dev *hdev, void *data)
> +{
> + __u8 *handle = data;
> +
> + return hci_le_terminate_big_sync(hdev, *handle,
> + HCI_ERROR_LOCAL_HOST_TERM);
> +}
> +
> +static void hci_iso_term_big_destroy(struct hci_dev *hdev, void *data, int err)
> +{
> + kfree(data);
> +}
> +
> static void hci_le_create_big_complete_evt(struct hci_dev *hdev, void *data,
> struct sk_buff *skb)
> {
> struct hci_evt_le_create_big_complete *ev = data;
> struct hci_conn *conn;
> __u8 i = 0;
> + __u8 *big_handle;
>
> BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
>
> @@ -7064,16 +7078,25 @@ static void hci_le_create_big_complete_evt(struct hci_dev *hdev, void *data,
> rcu_read_lock();
> }
>
> - if (!ev->status && !i)
> + rcu_read_unlock();
> +
> + if (!ev->status && !i) {
> /* If no BISes have been connected for the BIG,
> * terminate. This is in case all bound connections
> * have been closed before the BIG creation
> * has completed.
> */
> - hci_le_terminate_big_sync(hdev, ev->handle,
> - HCI_ERROR_LOCAL_HOST_TERM);
> + big_handle = kzalloc(sizeof(*big_handle), GFP_KERNEL);
> + if (!big_handle)
> + goto unlock;

You don't need to allocate a pointer to the handle, just pass it with UINT_PTR.

>
> - rcu_read_unlock();
> + *big_handle = ev->handle;
> +
> + hci_cmd_sync_queue(hdev, hci_iso_term_big_sync, big_handle,
> + hci_iso_term_big_destroy);
> + }
> +
> +unlock:
> hci_dev_unlock(hdev);
> }
>
> --
> 2.39.2
>


--
Luiz Augusto von Dentz