From patchwork Wed Jul 26 21:25:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 706695 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DBC58C001E0 for ; Wed, 26 Jul 2023 21:26:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231533AbjGZV0F (ORCPT ); Wed, 26 Jul 2023 17:26:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45324 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231500AbjGZV0C (ORCPT ); Wed, 26 Jul 2023 17:26:02 -0400 Received: from meesny.iki.fi (meesny.iki.fi [195.140.195.201]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EFCE5269E for ; Wed, 26 Jul 2023 14:25:47 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: pav) by meesny.iki.fi (Postfix) with ESMTPSA id 4RB6Q11Gkgzyhc; Thu, 27 Jul 2023 00:25:45 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406745; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=cStnu3uRKmISA/8+0oThhWnWzX9S6j2dgsNQupODTH0=; b=Cxs1Ml+x+UbjKtpPG1TkGExgmubCzwPcQKRcf3WPnunzmY3WpEqfvN3OoRP2w70LoKbXxF UfjX3/WFK4CFk4EgUZcu0eS4p/5jqkArhgAS/hqCHrF4MJPNA6sIqLS4HetccrnatQ/HMC mOgVtv7aUg5fU0gV8BnE3V7ywZZuvWU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406745; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=cStnu3uRKmISA/8+0oThhWnWzX9S6j2dgsNQupODTH0=; b=q5ctp2/J7By+6X1Jp0uAins5jCxv7WSSHnWdQcHAhZtHElqaHy1nKC2bPnJ20ONY/T1OWn zUGyQqSFhVJWNU2Bq/1zq0afBNMwvhXI2fLiyl9lUKzdCoVT4urFSIwXpC3498x1BHH6oH vfdc/jzM2fADAv8DhXsh4MfS+FJ1WZo= ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=meesny; d=iki.fi; t=1690406745; a=rsa-sha256; cv=none; b=EosX0/9lrf+3m2IgynUDxBnIMNKHULhWhBGI+3ISeEHLzsQJsGYvtH2/emagO2hYvLvxfX qq8SM1BoOCK0hjWPpPQP5I5jDdP1vfjNiikl/SyuK6EIzjHRpirNHQqkI3mh/wLzgpwuxq YWHyewmJc1AugRBIZ+QdP8HNj+eH2Yg= From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 1/6] Bluetooth: hci_conn: hci_conn_cleanup is not needed, combine with del Date: Thu, 27 Jul 2023 00:25:21 +0300 Message-ID: <30c5e6a7dc62ce209a2b9916fe8b2579d1b27756.1690399379.git.pav@iki.fi> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org hci_conn_cleanup is no longer needed, so move the code back to hci_conn_del to keep the hci_conn teardown in a single place. This undoes commit b958f9a3e877 ("Bluetooth: Fix reference counting for LE-scan based connections"), but keeps the current order of cleanup operations. Signed-off-by: Pauli Virtanen --- net/bluetooth/hci_conn.c | 78 +++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 45 deletions(-) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index cccc2b8b60a8..a71a54a5c8d8 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -139,45 +139,6 @@ static void hci_connect_le_scan_cleanup(struct hci_conn *conn, u8 status) hci_update_passive_scan(hdev); } -static void hci_conn_cleanup(struct hci_conn *conn) -{ - struct hci_dev *hdev = conn->hdev; - - if (test_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags)) - hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type); - - if (test_and_clear_bit(HCI_CONN_FLUSH_KEY, &conn->flags)) - hci_remove_link_key(hdev, &conn->dst); - - hci_chan_list_flush(conn); - - hci_conn_hash_del(hdev, conn); - - if (conn->cleanup) - conn->cleanup(conn); - - if (conn->type == SCO_LINK || conn->type == ESCO_LINK) { - switch (conn->setting & SCO_AIRMODE_MASK) { - case SCO_AIRMODE_CVSD: - case SCO_AIRMODE_TRANSP: - if (hdev->notify) - hdev->notify(hdev, HCI_NOTIFY_DISABLE_SCO); - break; - } - } else { - if (hdev->notify) - hdev->notify(hdev, HCI_NOTIFY_CONN_DEL); - } - - hci_conn_del_sysfs(conn); - - debugfs_remove_recursive(conn->debugfs); - - hci_dev_put(hdev); - - hci_conn_put(conn); -} - static void hci_acl_create_connection(struct hci_conn *conn) { struct hci_dev *hdev = conn->hdev; @@ -1127,12 +1088,39 @@ void hci_conn_del(struct hci_conn *conn) skb_queue_purge(&conn->data_q); - /* Remove the connection from the list and cleanup its remaining - * state. This is a separate function since for some cases like - * BT_CONNECT_SCAN we *only* want the cleanup part without the - * rest of hci_conn_del. - */ - hci_conn_cleanup(conn); + if (test_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags)) + hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type); + + if (test_and_clear_bit(HCI_CONN_FLUSH_KEY, &conn->flags)) + hci_remove_link_key(hdev, &conn->dst); + + hci_chan_list_flush(conn); + + hci_conn_hash_del(hdev, conn); + + if (conn->cleanup) + conn->cleanup(conn); + + if (conn->type == SCO_LINK || conn->type == ESCO_LINK) { + switch (conn->setting & SCO_AIRMODE_MASK) { + case SCO_AIRMODE_CVSD: + case SCO_AIRMODE_TRANSP: + if (hdev->notify) + hdev->notify(hdev, HCI_NOTIFY_DISABLE_SCO); + break; + } + } else { + if (hdev->notify) + hdev->notify(hdev, HCI_NOTIFY_CONN_DEL); + } + + hci_conn_del_sysfs(conn); + + debugfs_remove_recursive(conn->debugfs); + + hci_dev_put(hdev); + + hci_conn_put(conn); } struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src, uint8_t src_type) From patchwork Wed Jul 26 21:25:22 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 706696 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 05A5BC001DC for ; Wed, 26 Jul 2023 21:26:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231536AbjGZV0D (ORCPT ); Wed, 26 Jul 2023 17:26:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45318 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231485AbjGZV0C (ORCPT ); Wed, 26 Jul 2023 17:26:02 -0400 Received: from meesny.iki.fi (meesny.iki.fi [IPv6:2001:67c:2b0:1c1::201]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 85A202D64 for ; Wed, 26 Jul 2023 14:25:47 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: pav) by meesny.iki.fi (Postfix) with ESMTPSA id 4RB6Q15NvBz107J; Thu, 27 Jul 2023 00:25:45 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406745; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=C1am/h+ATMBUgghrZPssfKQla6Kn6yuarwxQv/n87UQ=; b=TrLP1CbBqUc1DgZizM18niZgl6Y6J2g7sUKtGqCe/NwiQipY8Y7uUmNZ0O2mELRDBBzyQj mZPB5eZli3qrnCR7NKGA0X47/GCdPNEWMFsIolDp1WewrzgS2lppoGhkXQuQTK8+ivxhHX PAQYEn6fDHElkhUdpo7sFRdW+f7QT+s= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406745; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=C1am/h+ATMBUgghrZPssfKQla6Kn6yuarwxQv/n87UQ=; b=x4tGfuAjYecbMm0g0GKlWbVvPk937reMea6M4KJx9Kgp3aUkyM9cOIyiFpFmDYZ4xf9Kuw TPUpB1BElXv3bt+UfUtaE3keeOPvFjLcBIl7byeZczqWDDCkFX8vcid5BIPK27zpq7ojeT Y+trnRZvz3bjID4D2TMQAUCgJ+947Fg= ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=meesny; d=iki.fi; t=1690406745; a=rsa-sha256; cv=none; b=qVEh7rWsNWkm40lTk68xtO0z4IkMHM4fTuCbL+lnLPeDFAhjd0BnzjhvAHR2sSBYI7HbJM ZpGhX107mFjhHuJ6/IYksuYaF3I4QgfXPTo+fgSMLqTHlJeDa9/rUD1BGfuDbAEiLH0WL9 3F0WvQ8ULbBCdLY99+2QOB85JUYWaLM= From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 2/6] Bluetooth: hci_conn: add hci_conn_is_alive Date: Thu, 27 Jul 2023 00:25:22 +0300 Message-ID: <053df33e4e3b35982b257238edcde451780fc832.1690399379.git.pav@iki.fi> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org There's currently no good way to know if a given hci_conn we hold hci_conn_get reference for has been deleted in the meanwhile. To enable checking whether a connection is still alive, add HCI_CONN_DELETED flag to indicate whether hci_conn_del has run. The flag is meaningful also with RCU lock only, but with different semantics. If hci_conn_is_alive(conn) returns true inside rcu_read_lock, conn was in conn_hash from the point of view of the current task when the flag was read. Then its deletion will not complete at least before rcu_read_unlock. Signed-off-by: Pauli Virtanen --- Notes: This is now suggesting the same thing once again, because the alternatives seem limited, and it's currently hard to write code in hci_sync safe against memory bugs. One alternative here is to remove the connection from conn_hash only when hci_conn_get/put refcount goes to zero. This simplifies resuming interrupted conn_hash iteration, as hci_conn_get then ensures the conn remains a valid iteration cursor. However, some deletion indicator flag is still needed, if we want to do something else with the conn. The suggestion to use conn->refcnt seems a bit hard to make it work, since that is used for different purpose (marking what connections userspace keeps alive), and it seems we'd need to change all callsites for hci_conn_del. E.g. in Disconnect Complete event we must remove the conn from sysfs and invalidate its handle immediately as the handle can be reused. Similarly for failed LE Create CIS while hci_conn_abort is running. It maybe could also be possible to not allow events to run while hci_sync is running, except when it is waiting for an event. This seems actually to be the concurrency model what is assumed in the hci_sync code (but it is not what is actually happening). However, it seems the deletion flag would be needed also here, as the conn might be gone while we are waiting for events. include/net/bluetooth/hci_core.h | 15 +++++++++++++++ net/bluetooth/hci_conn.c | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 8200a6689b39..34e4ad7c32e7 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -978,6 +978,7 @@ enum { HCI_CONN_CREATE_CIS, HCI_CONN_BIG_SYNC, HCI_CONN_BIG_SYNC_FAILED, + HCI_CONN_DELETED, }; static inline bool hci_conn_ssp_enabled(struct hci_conn *conn) @@ -997,6 +998,7 @@ static inline bool hci_conn_sc_enabled(struct hci_conn *conn) static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c) { struct hci_conn_hash *h = &hdev->conn_hash; + WARN_ON(test_bit(HCI_CONN_DELETED, &c->flags)); list_add_tail_rcu(&c->list, &h->list); switch (c->type) { case ACL_LINK: @@ -1024,6 +1026,7 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c) { struct hci_conn_hash *h = &hdev->conn_hash; + WARN_ON(!test_bit(HCI_CONN_DELETED, &c->flags)); list_del_rcu(&c->list); synchronize_rcu(); @@ -1049,6 +1052,18 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c) } } +/* With hdev->lock: whether hci_conn is in conn_hash. + * With RCU: if true, the hci_conn is valid conn_hash iteration cursor and + * hci_conn_hash_del has not completed. (Note that if hci_conn was obtained in + * this critical section it is always valid, but this may return false!) + */ +static inline bool hci_conn_is_alive(struct hci_dev *hdev, struct hci_conn *c) +{ + RCU_LOCKDEP_WARN(!lockdep_is_held(&hdev->lock) && !rcu_read_lock_held(), + "suspicious locking"); + return !test_bit(HCI_CONN_DELETED, &c->flags); +} + static inline unsigned int hci_conn_num(struct hci_dev *hdev, __u8 type) { struct hci_conn_hash *h = &hdev->conn_hash; diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index a71a54a5c8d8..ee304618bf0a 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1052,9 +1052,13 @@ static void hci_conn_unlink(struct hci_conn *conn) void hci_conn_del(struct hci_conn *conn) { struct hci_dev *hdev = conn->hdev; + bool deleted; BT_DBG("%s hcon %p handle %d", hdev->name, conn, conn->handle); + deleted = test_and_set_bit(HCI_CONN_DELETED, &conn->flags); + WARN_ON(deleted); + hci_conn_unlink(conn); cancel_delayed_work_sync(&conn->disc_work); From patchwork Wed Jul 26 21:25:23 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 707626 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9488FC001DC for ; Wed, 26 Jul 2023 21:26:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231485AbjGZV0G (ORCPT ); Wed, 26 Jul 2023 17:26:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45318 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231345AbjGZV0D (ORCPT ); Wed, 26 Jul 2023 17:26:03 -0400 Received: from meesny.iki.fi (meesny.iki.fi [195.140.195.201]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2998326A8 for ; Wed, 26 Jul 2023 14:25:47 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: pav) by meesny.iki.fi (Postfix) with ESMTPSA id 4RB6Q161v0z107j; Thu, 27 Jul 2023 00:25:45 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406745; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2hPuiy3xlYhwpql/vmJpKc986dGEM1JauIIbxMsiXH8=; b=iNQSvc2B4ZbViTPjNdWYH0WPRfzD/dDo24qdIVZNFezRnZa4KxnfO2pXXELhEVvBR+4suN s6dwuxeL63kvs1C/yKeEcSmZm7cnd/jW3PhUzEmTWiPgqgy+wcGlheYxb1LuT60rwz2BmM Pu9nbOZ7drrGVqF7mJa2KLfPW5bzQHY= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406745; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2hPuiy3xlYhwpql/vmJpKc986dGEM1JauIIbxMsiXH8=; b=c5zXbYQ3tSt8uk1c0wJxqh/5OCbhpAa1TDJwFhYGWvc4zGy6N1VlpRr5Xye7viBpMpMffX 4Wv6QEjPiC3hu1NPqUK3rV73ZtBUeCOBFRmpuTiGEz7XDYW8iyA6PZ7H3dViM/12jqFzyK ybA73niXV0BpXazi9FZ1qbS9LhheoW0= ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=meesny; d=iki.fi; t=1690406745; a=rsa-sha256; cv=none; b=Pazk0Y4BXQZLYzzMP0dmYy/EberXneDdcMVWF0t6mD0iGpCwNQNtw1/8zwYQo0F1MXvGfL IAKcsWuUTErJ+ScmcMRcuc36nXKJcBpMvWHj5JXhxOtd9bJ66lIPuQg3w//N2Ec7a6P5R9 7gOrP/9gUy0S7kInvSxz1Z+zBw4rFC4= From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 3/6] Bluetooth: hci_sync: add hci_conn_sync_queue and hci_cmd_sync_dev_(un)lock Date: Thu, 27 Jul 2023 00:25:23 +0300 Message-ID: <15e7863c06bd87cd991ab963132fa9d12ef7e11a.1690399379.git.pav@iki.fi> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Operations on a given hci_conn in hci_sync are hard to write safely, because the conn might be deleted or modified unexpectedly either concurrently or when waiting for the events. Holding hci_dev_lock in the sync callbacks is also inconvenient since it has to be manually released before entering the event waits. Add convenience utilities to make things easier: Add hci_cmd_sync_dev_lock/unlock, for easier writing of hci_sync callbacks where hci_dev_lock should be held. The lock is however still released and reacquired around request waits. Callbacks using them can then assume that state changes protected by hci_dev_lock can only occur when waiting for events. (This is currently assumed in many of the callbacks.) Add hci_conn_sync_queue/submit, whose callback on entry holds hci_dev_lock and the hci_conn has not been deleted. If it was deleted while the sync command was queued, the destroy routine has err -ENODEV. Similarly, synchronous commands called in the callback fail with ENODEV if the conn was deleted during the wait. Signed-off-by: Pauli Virtanen --- include/net/bluetooth/hci_core.h | 7 ++++ include/net/bluetooth/hci_sync.h | 3 ++ net/bluetooth/hci_conn.c | 60 ++++++++++++++++++++++++++++++++ net/bluetooth/hci_sync.c | 31 +++++++++++++++++ 4 files changed, 101 insertions(+) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 34e4ad7c32e7..8e218300ef4e 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -519,6 +519,9 @@ struct hci_dev { struct work_struct cmd_sync_cancel_work; struct work_struct reenable_adv_work; + bool cmd_sync_locked; + struct hci_conn *cmd_sync_conn; + __u16 discov_timeout; struct delayed_work discov_off; @@ -1400,6 +1403,10 @@ void hci_conn_del(struct hci_conn *conn); void hci_conn_hash_flush(struct hci_dev *hdev); void hci_conn_check_pending(struct hci_dev *hdev); +void hci_conn_sync_set_conn(struct hci_dev *hdev, struct hci_conn *conn); +int hci_conn_sync_queue(struct hci_conn *conn, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy); + struct hci_chan *hci_chan_create(struct hci_conn *conn); void hci_chan_del(struct hci_chan *chan); void hci_chan_list_flush(struct hci_conn *conn); diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h index b516a0f4a55b..a9a94950d523 100644 --- a/include/net/bluetooth/hci_sync.h +++ b/include/net/bluetooth/hci_sync.h @@ -46,6 +46,9 @@ int hci_cmd_sync_submit(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, void *data, hci_cmd_sync_work_destroy_t destroy); +void hci_cmd_sync_dev_lock(struct hci_dev *hdev); +void hci_cmd_sync_dev_unlock(struct hci_dev *hdev); + int hci_update_eir_sync(struct hci_dev *hdev); int hci_update_class_sync(struct hci_dev *hdev); diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index ee304618bf0a..208eb5eea451 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -2889,3 +2889,63 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason) return hci_cmd_sync_queue(hdev, abort_conn_sync, ERR_PTR(conn->handle), NULL); } + +struct hci_conn_sync_work_entry { + struct hci_conn *conn; + hci_cmd_sync_work_func_t func; + void *data; + hci_cmd_sync_work_destroy_t destroy; +}; + +static int hci_conn_sync_work_run(struct hci_dev *hdev, void *data) +{ + struct hci_conn_sync_work_entry *entry = data; + int err; + + hci_cmd_sync_dev_lock(hdev); + hdev->cmd_sync_conn = entry->conn; + + if (hci_conn_is_alive(hdev, entry->conn)) + err = entry->func(hdev, entry->data); + else + err = -ENODEV; + + hdev->cmd_sync_conn = NULL; + hci_cmd_sync_dev_unlock(hdev); + + return err; +} + +static void hci_conn_sync_work_destroy(struct hci_dev *hdev, void *data, + int err) +{ + struct hci_conn_sync_work_entry *entry = data; + + if (entry->destroy) + entry->destroy(hdev, entry->data, err); + hci_conn_put(entry->conn); + kfree(entry); +} + +int hci_conn_sync_queue(struct hci_conn *conn, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy) +{ + struct hci_conn_sync_work_entry *entry; + int err; + + entry = kmalloc(sizeof(*entry), GFP_KERNEL); + if (!entry) + return -ENOMEM; + + entry->func = func; + entry->data = data; + entry->destroy = destroy; + entry->conn = hci_conn_get(conn); + + err = hci_cmd_sync_queue(conn->hdev, hci_conn_sync_work_run, entry, + hci_conn_sync_work_destroy); + if (err) + kfree(entry); + + return err; +} diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 3348a1b0e3f7..6a295a9e1f5d 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -164,10 +164,16 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen, if (err < 0) return ERR_PTR(err); + if (hdev->cmd_sync_locked) + hci_dev_unlock(hdev); + err = wait_event_interruptible_timeout(hdev->req_wait_q, hdev->req_status != HCI_REQ_PEND, timeout); + if (hdev->cmd_sync_locked) + hci_dev_lock(hdev); + if (err == -ERESTARTSYS) return ERR_PTR(-EINTR); @@ -185,6 +191,11 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen, break; } + if (hdev->cmd_sync_conn) { + if (!hci_conn_is_alive(hdev, hdev->cmd_sync_conn)) + err = -ENODEV; + } + hdev->req_status = 0; hdev->req_result = 0; skb = hdev->req_skb; @@ -740,6 +751,26 @@ int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, } EXPORT_SYMBOL(hci_cmd_sync_queue); +void hci_cmd_sync_dev_lock(struct hci_dev *hdev) +{ + lockdep_assert_held(&hdev->req_lock); + + hci_dev_lock(hdev); + + WARN_ON_ONCE(hdev->cmd_sync_locked); + hdev->cmd_sync_locked = true; +} + +void hci_cmd_sync_dev_unlock(struct hci_dev *hdev) +{ + lockdep_assert_held(&hdev->req_lock); + + WARN_ON_ONCE(!hdev->cmd_sync_locked); + hdev->cmd_sync_locked = false; + + hci_dev_unlock(hdev); +} + int hci_update_eir_sync(struct hci_dev *hdev) { struct hci_cp_write_eir cp; From patchwork Wed Jul 26 21:25:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 706694 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B181FC04A94 for ; Wed, 26 Jul 2023 21:26:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229778AbjGZV0I (ORCPT ); Wed, 26 Jul 2023 17:26:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45320 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231545AbjGZV0D (ORCPT ); Wed, 26 Jul 2023 17:26:03 -0400 Received: from meesny.iki.fi (meesny.iki.fi [195.140.195.201]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 339B426AC for ; Wed, 26 Jul 2023 14:25:47 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: pav) by meesny.iki.fi (Postfix) with ESMTPSA id 4RB6Q16qHjz107y; Thu, 27 Jul 2023 00:25:45 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406746; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=H4V1uWl2v29EFzh2Lvit/4KNTqlM0Wa8H2yX7NKORRM=; b=qDNONah0H9kCE6/Pscyne1JCu3EsO34+Gc1kSsT+W0lYN7dAJguvdOR3fnTbYZfRr+Oj1z kLmsIA6D2p1xsvpUJLpqDG0zy+DkJ9gKojb/0bGHb3tEXKt4hIlKfiPSpdqQnraSRdf1iu HR5uFpplqpNTZJDakx7CLJSez5WT6JI= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406746; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=H4V1uWl2v29EFzh2Lvit/4KNTqlM0Wa8H2yX7NKORRM=; b=uq0+fuzSwpVrlkQs5SsaLb7EuhJ5u4ATgQ6U9AEfXkbShgWsLCkW+X8Gg+ffWbn43M12Au N6CcLQb/NcXFwOOefRaP+M77S7gA0ZgsiSJcpnrfAyPQYyMTL7ipCqrau5slSXTtA7bVGk BtJ2s+1kWGAidJmxzh+YraYMvfu42vk= ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=meesny; d=iki.fi; t=1690406746; a=rsa-sha256; cv=none; b=k4nI/Pebyex6kD5QB3682gnD/MyhsCBgxowKDEEIV9wD1Tw99SzAx1IT1QJ6cdzsIIU3+p gtS4c8FibwUCrmEju5LeLyMeZBACfIvLoeuB0H8szCT6yAEKKzfiF7deI1Ry79WwUnOrQ6 WChSPRAwpvsPZ+j0p6x4EmZu7J7uMZM= From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 4/6] Bluetooth: hci_sync: fix locking in hci_conn_abort and hci_disconnect_all_sync Date: Thu, 27 Jul 2023 00:25:24 +0300 Message-ID: <70a377ca228992facffcc57d77e9acdbde8e46a7.1690399379.git.pav@iki.fi> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org hci_conn_abort may run concurrently with operations that e.g. change conn handle or delete it. Similarly, hci_disconnect_all_sync iterates conn_hash without locks/RCU which is not OK. To make it correct vs locking, use hci_conn_sync_queue to hold hdev->lock and check hci_conn aliveness after waiting for events. Make iteration in hci_disconnect_all_sync safe. Since we don't have a flag for indicating whether a connection was aborted, just make a copy of the conn_hash and iterate the copy. Signed-off-by: Pauli Virtanen --- Notes: The iterate-over-copy is ugly, could add another hci_conn flag. Or change things so that hci_conn_get keeps the hci_conn in conn_hash so we can always continue the iteration safely as long as a reference was held. net/bluetooth/hci_conn.c | 10 ++------ net/bluetooth/hci_sync.c | 52 ++++++++++++++++++++++++++++------------ 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 208eb5eea451..ba339a0eb851 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -2845,12 +2845,7 @@ u32 hci_conn_get_phy(struct hci_conn *conn) static int abort_conn_sync(struct hci_dev *hdev, void *data) { - struct hci_conn *conn; - u16 handle = PTR_ERR(data); - - conn = hci_conn_hash_lookup_handle(hdev, handle); - if (!conn) - return 0; + struct hci_conn *conn = data; return hci_abort_conn_sync(hdev, conn, conn->abort_reason); } @@ -2886,8 +2881,7 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason) } } - return hci_cmd_sync_queue(hdev, abort_conn_sync, ERR_PTR(conn->handle), - NULL); + return hci_conn_sync_queue(conn, abort_conn_sync, conn, NULL); } struct hci_conn_sync_work_entry { diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 6a295a9e1f5d..101548fe81da 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -5407,6 +5407,8 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason) { int err; + lockdep_assert_held(&hdev->lock); + switch (conn->state) { case BT_CONNECTED: case BT_CONFIG: @@ -5418,21 +5420,15 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason) * or in case of LE it was still scanning so it can be cleanup * safely. */ - if (err) { - hci_dev_lock(hdev); + if (err && hci_conn_is_alive(hdev, conn)) hci_conn_failed(conn, err); - hci_dev_unlock(hdev); - } return err; case BT_CONNECT2: return hci_reject_conn_sync(hdev, conn, reason); case BT_OPEN: /* Cleanup bises that failed to be established */ - if (test_and_clear_bit(HCI_CONN_BIG_SYNC_FAILED, &conn->flags)) { - hci_dev_lock(hdev); + if (test_and_clear_bit(HCI_CONN_BIG_SYNC_FAILED, &conn->flags)) hci_conn_failed(conn, reason); - hci_dev_unlock(hdev); - } break; default: conn->state = BT_CLOSED; @@ -5444,16 +5440,42 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason) static int hci_disconnect_all_sync(struct hci_dev *hdev, u8 reason) { - struct hci_conn *conn, *tmp; - int err; + struct hci_conn *conn; + struct hci_conn **conns; + int err = 0; + size_t i, n; - list_for_each_entry_safe(conn, tmp, &hdev->conn_hash.list, list) { - err = hci_abort_conn_sync(hdev, conn, reason); - if (err) - return err; + hci_cmd_sync_dev_lock(hdev); + + /* Make a copy of conn_hash, because hci_abort_conn_sync may release the + * lock and wait for events, during which the list may be mutated + * arbitrarily. + */ + n = 0; + list_for_each_entry(conn, &hdev->conn_hash.list, list) + ++n; + + conns = kvcalloc(n, sizeof(*conns), GFP_KERNEL); + if (!conns) { + err = -ENOMEM; + goto unlock; } - return 0; + i = 0; + list_for_each_entry(conn, &hdev->conn_hash.list, list) + conns[i++] = hci_conn_get(conn); + + for (i = 0; i < n; ++i) { + if (!err) + err = hci_abort_conn_sync(hdev, conns[i], reason); + hci_conn_put(conns[i]); + } + + kvfree(conns); + +unlock: + hci_cmd_sync_dev_unlock(hdev); + return err; } /* This function perform power off HCI command sequence as follows: From patchwork Wed Jul 26 21:25:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 707625 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E47F5C001DF for ; Wed, 26 Jul 2023 21:26:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231464AbjGZV0K (ORCPT ); Wed, 26 Jul 2023 17:26:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45422 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231550AbjGZV0I (ORCPT ); Wed, 26 Jul 2023 17:26:08 -0400 Received: from meesny.iki.fi (meesny.iki.fi [IPv6:2001:67c:2b0:1c1::201]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9CD552691 for ; Wed, 26 Jul 2023 14:26:03 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: pav) by meesny.iki.fi (Postfix) with ESMTPSA id 4RB6Q20W7xz1097; Thu, 27 Jul 2023 00:25:46 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406746; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wY79wLmp/mbU+avRSt0aj3qMsw6uaa1fo1QqQOAfhG4=; b=pY1mp0fjF1Xcol9Py+W3bVDl+MCGXwWJUAzINIZKSCDamX/fqOifGJaoicozA7MDuHrgRF lSCvBvIR/tJy1fNpvrfhFQxGp5tlISVWFfcYMPsDwmq49Z0MVZgUydxbkTR/2RDMB3iRR3 adPugtQdiK85MA3FuWV+xtj6ya2nDw0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406746; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wY79wLmp/mbU+avRSt0aj3qMsw6uaa1fo1QqQOAfhG4=; b=KNooRWsdtR23rd36tJ9Di8qTFIC6Bd4Jm0ouZnGYh/VY1MLlZ04+UEnIvhyoziWV7Hmck4 78lPN55vWpjwklI51aZbTZlAZgX4C20/NiCY7TBwQnFNCiMxI1+L/Z5E7SWgn0OGzeFOtb i9fOf20TtFTQcdlLwFzJ/yIV+9t9JaY= ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=meesny; d=iki.fi; t=1690406746; a=rsa-sha256; cv=none; b=W/V7dIXFraEOTGx86/Ff0TkybY5VcDo4vvnoP79MN35NZUneg9okazKh+aQB+Sk3HytwJD nTXbBYEwRsNiMm+bPFjuIKbKjUZjOqvl5kIJfAC7vAZkX0omTwmGm3T/ZAaCn3tePAsE/T IzLnIce9NypSxAnMIzMSwjjkuq8296Y= From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 5/6] Bluetooth: hci_sync: delete CIS in BT_OPEN/CONNECT/BOUND when aborting Date: Thu, 27 Jul 2023 00:25:25 +0300 Message-ID: <6d9672c9a1e97b87e823e05ff07576013683979d.1690399379.git.pav@iki.fi> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Dropped CIS that are in state BT_OPEN/BT_BOUND, and in state BT_CONNECT with HCI_CONN_CREATE_CIS unset, should be cleaned up immediately. Closing CIS ISO sockets should result to the hci_conn be deleted, so that potentially pending CIG removal can run. hci_abort_conn cannot refer to them by handle, since their handle is still unset if Set CIG Parameters has not yet completed. This fixes CIS not being terminated if the socket is shut down immediately after connection, so that the hci_abort_conn runs before Set CIG Parameters completes. See new BlueZ test "ISO Connect Close - Success" Signed-off-by: Pauli Virtanen --- net/bluetooth/hci_sync.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 101548fe81da..3926213c29e6 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -5339,6 +5339,10 @@ static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn, if (test_bit(HCI_CONN_CREATE_CIS, &conn->flags)) return hci_disconnect_sync(hdev, conn, reason); + /* CIS with no Create CIS sent have nothing to cancel */ + if (bacmp(&conn->dst, BDADDR_ANY)) + return HCI_ERROR_LOCAL_HOST_TERM; + /* There is no way to cancel a BIS without terminating the BIG * which is done later on connection cleanup. */ @@ -5426,9 +5430,17 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason) case BT_CONNECT2: return hci_reject_conn_sync(hdev, conn, reason); case BT_OPEN: - /* Cleanup bises that failed to be established */ - if (test_and_clear_bit(HCI_CONN_BIG_SYNC_FAILED, &conn->flags)) + /* Cleanup failed CIS, and BIS that failed to be established */ + if (bacmp(&conn->dst, BDADDR_ANY) || + test_and_clear_bit(HCI_CONN_BIG_SYNC_FAILED, &conn->flags)) + hci_conn_failed(conn, reason); + break; + case BT_BOUND: + /* Bound CIS should be cleaned up */ + if (conn->type == ISO_LINK && bacmp(&conn->dst, BDADDR_ANY)) hci_conn_failed(conn, reason); + else + conn->state = BT_CLOSED; break; default: conn->state = BT_CLOSED; From patchwork Wed Jul 26 21:25:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 706693 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 08DB8C001E0 for ; Wed, 26 Jul 2023 21:26:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231621AbjGZV0K (ORCPT ); Wed, 26 Jul 2023 17:26:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45434 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231569AbjGZV0J (ORCPT ); Wed, 26 Jul 2023 17:26:09 -0400 Received: from meesny.iki.fi (meesny.iki.fi [IPv6:2001:67c:2b0:1c1::201]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9CE60269E for ; Wed, 26 Jul 2023 14:26:03 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: pav) by meesny.iki.fi (Postfix) with ESMTPSA id 4RB6Q21MRYz109C; Thu, 27 Jul 2023 00:25:46 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406746; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QF4dKVf7TmhOZ/dmj3ft7QgJjtuX9ExKfqILvYm6EEI=; b=tMSnUdNhfinhzfxpyt1UM8GkkW3emkSWXn/TNUIq5PxSQFwXtOwQD9uRwppiT6dZZIpSHw TjHp5cHk2WY//V2fA6yA63y65Dcw2KiJkse+2inUpW4H9qdZzKV+ZmDtyV7xQG3C3vgn3G C4nvSoiGbT7k9vzR9vlrOqBHZZpRonQ= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1690406746; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QF4dKVf7TmhOZ/dmj3ft7QgJjtuX9ExKfqILvYm6EEI=; b=Qy5Lt0WW6VvfQvKSTJBSdOGks/ylXk/Xla2utYL22UHYMSW9j0RerSuK6GV3QVYeHDIpTR OICistTKdtjtTLWDRKvZdoZYxpA0V9WQAJBH9PRHLf9Q1+Kaju1lyKiOAIGZD4ItAsY0Nz acr4h40NYPbMKfCZJhvRGwRdQOFPlIE= ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=meesny; d=iki.fi; t=1690406746; a=rsa-sha256; cv=none; b=phyUNmSJeQHSeXfYlfTyNB0T1nk4SuZzYlj9n5jeW5ZqLx/9s7OYexDbwQBexHSW3q23wT sCNl/eniy7hQGm46GwjgXJ3LF0I+ApsM2vS/DzPhml6rXJAcjQ+lBX4NP0lkim3ol0Zdyr BJlXUKq3zaUeeJOU90bDs/EYOoijaqM= From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 6/6] Bluetooth: ISO: handle bound CIS cleanup via hci_conn Date: Thu, 27 Jul 2023 00:25:26 +0300 Message-ID: X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Calling hci_conn_del in __iso_sock_close is invalid. It needs hdev->lock, but it cannot be acquired there due to lock ordering. Fix this by doing cleanup via hci_conn_drop. Return hci_conn with refcount 1 from hci_bind_cis and hci_connect_cis, so that the iso_conn always holds one reference. This also fixes refcounting when error handling. Since hci_conn_abort shall handle termination of connections in any state properly, we can handle BT_CONNECT socket state in the same way as BT_CONNECTED. Signed-off-by: Pauli Virtanen --- net/bluetooth/hci_conn.c | 5 +++++ net/bluetooth/iso.c | 14 +------------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index ba339a0eb851..33fbdc8e0748 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1901,6 +1901,8 @@ struct hci_conn *hci_bind_cis(struct hci_dev *hdev, bdaddr_t *dst, return ERR_PTR(-EINVAL); } + hci_conn_hold(cis); + cis->iso_qos = *qos; cis->state = BT_BOUND; @@ -2254,6 +2256,9 @@ struct hci_conn *hci_connect_cis(struct hci_dev *hdev, bdaddr_t *dst, return ERR_PTR(-ENOLINK); } + /* Link takes the refcount */ + hci_conn_drop(cis); + cis->state = BT_CONNECT; hci_le_create_cis_pending(hdev); diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c index cbe3299b4a41..358954bfbb32 100644 --- a/net/bluetooth/iso.c +++ b/net/bluetooth/iso.c @@ -628,6 +628,7 @@ static void __iso_sock_close(struct sock *sk) iso_sock_cleanup_listen(sk); break; + case BT_CONNECT: case BT_CONNECTED: case BT_CONFIG: if (iso_pi(sk)->conn->hcon) { @@ -643,19 +644,6 @@ static void __iso_sock_close(struct sock *sk) break; case BT_CONNECT2: - iso_chan_del(sk, ECONNRESET); - break; - case BT_CONNECT: - /* In case of DEFER_SETUP the hcon would be bound to CIG which - * needs to be removed so just call hci_conn_del so the cleanup - * callback do what is needed. - */ - if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags) && - iso_pi(sk)->conn->hcon) { - hci_conn_del(iso_pi(sk)->conn->hcon); - iso_pi(sk)->conn->hcon = NULL; - } - iso_chan_del(sk, ECONNRESET); break; case BT_DISCONN: