diff mbox series

[v3,4/6] Bluetooth: Perform hci_conn_drop in hci_conn_unlink

Message ID 20230502145737.140856-5-lrh2000@pku.edu.cn
State New
Headers show
Series Bluetooth: Fix potential double free caused by hci_conn_unlink | expand

Commit Message

Ruihan Li May 2, 2023, 2:57 p.m. UTC 
Since hci_conn_link invokes both hci_conn_get and hci_conn_hold,
hci_conn_unlink should perform both hci_conn_put and hci_conn_drop as
well. However, currently it performs only hci_conn_put.

This patch makes hci_conn_unlink call hci_conn_drop as well, which
simplifies the logic in hci_conn_del a bit and may benefit future users
of hci_conn_unlink. But it is noted that this change additionally
implies that hci_conn_unlink can queue disc_work on conn itself, with
the following call stack:

	hci_conn_unlink(conn)  [conn->parent == NULL]
		-> hci_conn_unlink(child)  [child->parent == conn]
			-> hci_conn_drop(child->parent)
				-> queue_delayed_work(&conn->disc_work)

Queued disc_work after hci_conn_del can be spurious, so during the
process of hci_conn_del, it is necessary to make the call to
cancel_delayed_work(&conn->disc_work) after invoking hci_conn_unlink.

Signed-off-by: Ruihan Li <lrh2000@pku.edu.cn>
---
 net/bluetooth/hci_conn.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index eef148291..e76ebb50d 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1105,6 +1105,7 @@  static void hci_conn_unlink(struct hci_conn *conn)
 	list_del_rcu(&conn->link->list);
 	synchronize_rcu();
 
+	hci_conn_drop(conn->parent);
 	hci_conn_put(conn->parent);
 	conn->parent = NULL;
 
@@ -1118,7 +1119,6 @@  void hci_conn_del(struct hci_conn *conn)
 
 	BT_DBG("%s hcon %p handle %d", hdev->name, conn, conn->handle);
 
-	cancel_delayed_work_sync(&conn->disc_work);
 	cancel_delayed_work_sync(&conn->auto_accept_work);
 	cancel_delayed_work_sync(&conn->idle_work);
 
@@ -1134,12 +1134,7 @@  void hci_conn_del(struct hci_conn *conn)
 		else
 			hdev->acl_cnt += conn->sent;
 	} else {
-		struct hci_conn *acl = conn->parent;
-
-		if (acl) {
-			hci_conn_unlink(conn);
-			hci_conn_drop(acl);
-		}
+		hci_conn_unlink(conn);
 
 		/* Unacked ISO frames */
 		if (conn->type == ISO_LINK) {
@@ -1152,6 +1147,11 @@  void hci_conn_del(struct hci_conn *conn)
 		}
 	}
 
+	/* hci_conn_unlink may trigger additional disc_work, so
+	 * ensure to perform cancelling after that.
+	 */
+	cancel_delayed_work_sync(&conn->disc_work);
+
 	if (conn->amp_mgr)
 		amp_mgr_put(conn->amp_mgr);