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@xxxxxxxxxx> --- net/bluetooth/hci_conn.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) 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); -- 2.40.0