[PATCH] Bluetooth: mgmt: Fix UAF in mesh_send_sync caused by race in mesh queue

[PATCH] Bluetooth: mgmt: Fix UAF in mesh_send_sync caused by race in mesh queue

[Igor Putko]

A race condition between mesh_send() and mesh_next() can lead to the
same struct mgmt_mesh_tx object being queued twice into the cmd_sync
work list.

Specifically, mesh_send_done_sync() and mesh_send_start_complete() clear
the HCI_MESH_SENDING flag before mesh_next() is executed or without
holding hci_dev_lock. If a concurrent mesh_send() arrives during this
window, it observes HCI_MESH_SENDING as cleared, adds the new mesh_tx
to hdev->mesh_pending, and queues it via hci_cmd_sync_queue().
Immediately thereafter, mesh_next() executes without holding
hci_dev_lock, inspects hdev->mesh_pending with mgmt_mesh_next(), and
queues the very same mesh_tx a second time.

When the first queued entry finishes, its completion callback invokes
mesh_send_complete(), which frees the mesh_tx structure. When the worker
subsequently picks up the duplicate second entry, mesh_send_sync()
attempts to access fields in the already freed mesh_tx, triggering a
KASAN slab-use-after-free read.

Fix this by:
1. Synchronizing mesh_next() under hci_dev_lock and checking if
   HCI_MESH_SENDING is already set before queueing.
2. Holding hci_dev_lock when modifying HCI_MESH_SENDING and completing
   mesh transactions in mesh_send_done_sync() and
   mesh_send_start_complete().
3. Unconditionally removing mesh_tx if hci_cmd_sync_queue() fails in
   mesh_send().

Fixes: b338d91703fa ("Bluetooth: Implement support for Mesh")
Reported-by: syzbot+f20755f37da70df33a16@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?id=d2762f31ce58dfcb900c2db500bf361f165c07bc
Signed-off-by: Igor Putko <igorpetindev@gmail.com>
---
 net/bluetooth/mgmt.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 251a896babd5..0dda38af00ec 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1094,13 +1094,15 @@ static int mesh_send_done_sync(struct hci_dev *hdev, void *data)
 {
 	struct mgmt_mesh_tx *mesh_tx;
 
-	hci_dev_clear_flag(hdev, HCI_MESH_SENDING);
 	if (list_empty(&hdev->adv_instances))
 		hci_disable_advertising_sync(hdev);
-	mesh_tx = mgmt_mesh_next(hdev, NULL);
 
+	hci_dev_lock(hdev);
+	mesh_tx = mgmt_mesh_next(hdev, NULL);
 	if (mesh_tx)
 		mesh_send_complete(hdev, mesh_tx, false);
+	hci_dev_clear_flag(hdev, HCI_MESH_SENDING);
+	hci_dev_unlock(hdev);
 
 	return 0;
 }
@@ -1109,18 +1111,30 @@ static int mesh_send_sync(struct hci_dev *hdev, void *data);
 static void mesh_send_start_complete(struct hci_dev *hdev, void *data, int err);
 static void mesh_next(struct hci_dev *hdev, void *data, int err)
 {
-	struct mgmt_mesh_tx *mesh_tx = mgmt_mesh_next(hdev, NULL);
+	struct mgmt_mesh_tx *mesh_tx;
+	int ret;
 
-	if (!mesh_tx)
+	hci_dev_lock(hdev);
+
+	if (hci_dev_test_flag(hdev, HCI_MESH_SENDING)) {
+		hci_dev_unlock(hdev);
 		return;
+	}
 
-	err = hci_cmd_sync_queue(hdev, mesh_send_sync, mesh_tx,
-				 mesh_send_start_complete);
+	mesh_tx = mgmt_mesh_next(hdev, NULL);
+	if (!mesh_tx) {
+		hci_dev_unlock(hdev);
+		return;
+	}
 
-	if (err < 0)
+	ret = hci_cmd_sync_queue(hdev, mesh_send_sync, mesh_tx,
+				 mesh_send_start_complete);
+	if (ret < 0)
 		mesh_send_complete(hdev, mesh_tx, false);
 	else
 		hci_dev_set_flag(hdev, HCI_MESH_SENDING);
+
+	hci_dev_unlock(hdev);
 }
 
 static void mesh_send_done(struct work_struct *work)
@@ -2309,9 +2323,12 @@ static void mesh_send_start_complete(struct hci_dev *hdev, void *data, int err)
 	/* Report any errors here, but don't report completion */
 
 	if (mgmt_err) {
+		hci_dev_lock(hdev);
 		hci_dev_clear_flag(hdev, HCI_MESH_SENDING);
 		/* Send Complete Error Code for handle */
 		mesh_send_complete(hdev, mesh_tx, false);
+		hci_dev_unlock(hdev);
+
 		if (err != -ECANCELED)
 			mesh_next(hdev, NULL, 0);
 		return;
@@ -2541,10 +2558,8 @@ static int mesh_send(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_MESH_SEND,
 				      MGMT_STATUS_FAILED);
 
-		if (mesh_tx) {
-			if (sending)
-				mgmt_mesh_remove(mesh_tx);
-		}
+		if (mesh_tx)
+			mgmt_mesh_remove(mesh_tx);
 	} else {
 		hci_dev_set_flag(hdev, HCI_MESH_SENDING);
 
-- 
2.47.3