@@ -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;
@@ -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);
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 <pav@iki.fi> --- 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(+)