[PATCH 1/3] Bluetooth: hci_conn: fix UAF in create_big_sync and create_big_complete

[PATCH 1/3] Bluetooth: hci_conn: fix UAF in create_big_sync and create_big_complete

[Aaron Esau]

From: Aaron Esau <aaron1esau@gmail.com>

hci_connect_bis() queues create_big_sync with a raw conn pointer.
create_big_sync blocks in __hci_cmd_sync_sk while a concurrent
hci_conn_del on hdev->workqueue frees conn. On timeout, freed memory
is dereferenced. The dequeue path also double-frees: hci_conn_del
invokes create_big_complete via hci_cmd_sync_dequeue, which calls
hci_conn_del again.

Take hci_conn_get before queueing, add hci_conn_valid checks, handle
-ECANCELED, drop with hci_conn_put in create_big_complete. Follows
the create_le_conn_complete pattern.

Fixes: eca0ae4aea66 ("Bluetooth: Add initial implementation of BIS connections")
Signed-off-by: Aaron Esau <aaron1esau@gmail.com>
---
 net/bluetooth/hci_conn.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index dc08585..59f5451 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -2119,11 +2119,16 @@ static void hci_iso_qos_setup(struct hci_dev *hdev, struct hci_conn *conn,
 static int create_big_sync(struct hci_dev *hdev, void *data)
 {
 	struct hci_conn *conn = data;
-	struct bt_iso_qos *qos = &conn->iso_qos;
+	struct bt_iso_qos *qos;
 	u16 interval, sync_interval = 0;
 	u32 flags = 0;
 	int err;
 
+	if (!hci_conn_valid(hdev, conn))
+		return -ECANCELED;
+
+	qos = &conn->iso_qos;
+
 	if (qos->bcast.out.phy == 0x02)
 		flags |= MGMT_ADV_FLAG_SEC_2M;
 
@@ -2198,11 +2203,24 @@ static void create_big_complete(struct hci_dev *hdev, void *data, int err)
 
 	bt_dev_dbg(hdev, "conn %p", conn);
 
+	if (err == -ECANCELED)
+		goto done;
+
+	hci_dev_lock(hdev);
+
+	if (!hci_conn_valid(hdev, conn))
+		goto unlock;
+
 	if (err) {
 		bt_dev_err(hdev, "Unable to create BIG: %d", err);
 		hci_connect_cfm(conn, err);
 		hci_conn_del(conn);
 	}
+
+unlock:
+	hci_dev_unlock(hdev);
+done:
+	hci_conn_put(conn);
 }
 
 struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst, __u8 sid,
@@ -2331,9 +2349,11 @@ struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
 				 BT_BOUND, &data);
 
 	/* Queue start periodic advertising and create BIG */
+	hci_conn_get(conn);
 	err = hci_cmd_sync_queue(hdev, create_big_sync, conn,
 				 create_big_complete);
 	if (err < 0) {
+		hci_conn_put(conn);
 		hci_conn_drop(conn);
 		return ERR_PTR(err);
 	}
-- 
2.52.0

[PATCH 3/3] Bluetooth: hci_conn: fix UAF in hci_enhanced_setup_sync

[Aaron Esau]

From: Aaron Esau <aaron1esau@gmail.com>

hci_setup_sync queues hci_enhanced_setup_sync with conn_handle as data
without taking a reference on conn. hci_conn_del tries to dequeue with
conn as data, but the pointer comparison fails (data is conn_handle).
The existing hci_conn_valid check has a TOCTOU gap since conn can be
freed after the check passes. conn_handle also leaks on cancellation
because no destroy callback is set.

Take hci_conn_get on conn, add a destroy callback that frees
conn_handle and drops the reference, and move kfree(conn_handle) from
the sync function to the destroy callback.

Fixes: e07a06b4eb41 ("Bluetooth: Convert SCO configure_datapath to hci_sync")
Signed-off-by: Aaron Esau <aaron1esau@gmail.com>
---
 net/bluetooth/hci_conn.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index a7faa4c..6a567d6 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -278,6 +278,15 @@ error:
 	return err;
 }
 
+static void hci_enhanced_setup_sync_complete(struct hci_dev *hdev,
+					     void *data, int err)
+{
+	struct conn_handle_t *conn_handle = data;
+
+	hci_conn_put(conn_handle->conn);
+	kfree(conn_handle);
+}
+
 static int hci_enhanced_setup_sync(struct hci_dev *hdev, void *data)
 {
 	struct conn_handle_t *conn_handle = data;
@@ -286,8 +295,6 @@ static int hci_enhanced_setup_sync(struct hci_dev *hdev, void *data)
 	struct hci_cp_enhanced_setup_sync_conn cp;
 	const struct sco_param *param;
 
-	kfree(conn_handle);
-
 	if (!hci_conn_valid(hdev, conn))
 		return -ECANCELED;
 
@@ -467,12 +474,15 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
 		if (!conn_handle)
 			return false;
 
-		conn_handle->conn = conn;
+		conn_handle->conn = hci_conn_get(conn);
 		conn_handle->handle = handle;
 		result = hci_cmd_sync_queue(conn->hdev, hci_enhanced_setup_sync,
-					    conn_handle, NULL);
-		if (result < 0)
+					    conn_handle,
+					    hci_enhanced_setup_sync_complete);
+		if (result < 0) {
+			hci_conn_put(conn);
 			kfree(conn_handle);
+		}
 
 		return result == 0;
 	}
-- 
2.52.0