[PATCH] Bluetooth: hci_conn: hold conn for LE timeout work

[Chengfeng Ye]

le_conn_timeout is embedded in struct hci_conn, but queuing the work
does not hold a reference to the connection. hci_conn_del() uses
cancel_delayed_work() because synchronous cancellation would deadlock
when le_conn_timeout() itself calls hci_conn_del() while holding
hdev->lock.

This leaves the following interleaving possible:

  CPU 0                               CPU 1
  le_conn_timeout()
                                      hci_conn_del()
                                        cancel_delayed_work() = false
                                        hci_conn_cleanup()
                                          put_device()
                                            kfree(conn)
  hci_conn_failed(conn, ...)

The callback then dereferences the released connection. KASAN reported:

  BUG: KASAN: slab-use-after-free in hci_conn_failed+0x232/0x250
  Read of size 8 at addr ffff8881180e8e20 by task kworker/u33:1/111
  Workqueue: hci0 le_conn_timeout
  Call Trace:
    hci_conn_failed+0x232/0x250
    le_conn_timeout+0x23e/0x2c0
    process_one_work+0x61b/0xf50
    worker_thread+0x45b/0xd10

  Allocated by task 115:
    __hci_conn_add+0x1758/0x1b90
    hci_connect_le+0x523/0x780
    l2cap_chan_connect+0xfca/0x1bd0
    l2cap_sock_connect+0x310/0x530

  Freed by task 110:
    kfree+0x149/0x330
    device_release+0xc8/0x240
    kobject_put+0x14d/0x280
    hci_conn_del+0x561/0xe70
    hci_abort_conn_sync+0x3e3/0x800

Take a connection device reference before queuing le_conn_timeout. Drop
it when the callback finishes, when queuing fails, or when cancellation
removes a pending instance. If cancellation races an executing callback,
the callback retains the reference until its final access, avoiding the
use-after-free without waiting under hdev->lock.

Fixes: 980ffc0a2cec ("Bluetooth: Fix LE connection timeout deadlock")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Chengfeng Ye <nicoyip.dev@xxxxxxxxx>
---
 net/bluetooth/hci_conn.c  |  8 ++++++--
 net/bluetooth/hci_event.c | 25 ++++++++++++++++---------
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 1966cd153d97..501a2dd2543b 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -723,10 +723,13 @@ static void le_conn_timeout(struct work_struct *work)
 		hci_dev_lock(hdev);
 		hci_conn_failed(conn, HCI_ERROR_ADVERTISING_TIMEOUT);
 		hci_dev_unlock(hdev);
-		return;
+		goto done;
 	}
 
 	hci_abort_conn(conn, HCI_ERROR_REMOTE_USER_TERM);
+
+done:
+	hci_conn_put(conn);
 }
 
 struct iso_list_data {
@@ -1267,7 +1270,8 @@ void hci_conn_del(struct hci_conn *conn)
 			hdev->acl_cnt += conn->sent;
 		break;
 	case LE_LINK:
-		cancel_delayed_work(&conn->le_conn_timeout);
+		if (cancel_delayed_work(&conn->le_conn_timeout))
+			hci_conn_put(conn);
 
 		if (hdev->le_pkts) {
 			if (!hci_conn_num(hdev, LE_LINK) ||
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 741d658e9630..e1e6b5092ce8 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1566,10 +1566,13 @@ static u8 hci_cc_le_set_adv_enable(struct hci_dev *hdev, void *data,
 		hci_dev_set_flag(hdev, HCI_LE_ADV);
 
 		conn = hci_lookup_le_connect(hdev);
-		if (conn)
-			queue_delayed_work(hdev->workqueue,
-					   &conn->le_conn_timeout,
-					   conn->conn_timeout);
+		if (conn) {
+			hci_conn_get(conn);
+			if (!queue_delayed_work(hdev->workqueue,
+						&conn->le_conn_timeout,
+						conn->conn_timeout))
+				hci_conn_put(conn);
+		}
 	} else {
 		hci_dev_clear_flag(hdev, HCI_LE_ADV);
 	}
@@ -1614,10 +1617,13 @@ static u8 hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev, void *data,
 			hci_dev_set_flag(hdev, HCI_LE_ADV_0);
 
 		conn = hci_lookup_le_connect(hdev);
-		if (conn)
-			queue_delayed_work(hdev->workqueue,
-					   &conn->le_conn_timeout,
-					   conn->conn_timeout);
+		if (conn) {
+			hci_conn_get(conn);
+			if (!queue_delayed_work(hdev->workqueue,
+						&conn->le_conn_timeout,
+						conn->conn_timeout))
+				hci_conn_put(conn);
+		}
 	} else {
 		if (cp->num_of_sets) {
 			if (adv)
@@ -5771,7 +5777,8 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
 			}
 		}
 	} else {
-		cancel_delayed_work(&conn->le_conn_timeout);
+		if (cancel_delayed_work(&conn->le_conn_timeout))
+			hci_conn_put(conn);
 	}
 
 	/* The HCI_LE_Connection_Complete event is only sent once per connection.
-- 
2.43.0