Bluetooth: hci_conn: Consolidate code for aborting connections
authorLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
Tue, 27 Jun 2023 00:25:06 +0000 (17:25 -0700)
committerLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
Fri, 11 Aug 2023 18:42:15 +0000 (11:42 -0700)
This consolidates code for aborting connections using
hci_cmd_sync_queue so it is synchronized with other threads, but
because of the fact that some commands may block the cmd_sync_queue
while waiting specific events this attempt to cancel those requests by
using hci_cmd_sync_cancel.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
include/net/bluetooth/hci_core.h
net/bluetooth/hci_conn.c
net/bluetooth/hci_sync.c
net/bluetooth/mgmt.c

index 9140d4a..2dd59e3 100644 (file)
@@ -739,6 +739,7 @@ struct hci_conn {
        unsigned long   flags;
 
        enum conn_reasons conn_reason;
+       __u8            abort_reason;
 
        __u32           clock;
        __u16           clock_accuracy;
@@ -758,7 +759,6 @@ struct hci_conn {
        struct delayed_work auto_accept_work;
        struct delayed_work idle_work;
        struct delayed_work le_conn_timeout;
-       struct work_struct  le_scan_cleanup;
 
        struct device   dev;
        struct dentry   *debugfs;
index 0f89daa..fa9236d 100644 (file)
@@ -178,57 +178,6 @@ static void hci_conn_cleanup(struct hci_conn *conn)
        hci_conn_put(conn);
 }
 
-static void le_scan_cleanup(struct work_struct *work)
-{
-       struct hci_conn *conn = container_of(work, struct hci_conn,
-                                            le_scan_cleanup);
-       struct hci_dev *hdev = conn->hdev;
-       struct hci_conn *c = NULL;
-
-       BT_DBG("%s hcon %p", hdev->name, conn);
-
-       hci_dev_lock(hdev);
-
-       /* Check that the hci_conn is still around */
-       rcu_read_lock();
-       list_for_each_entry_rcu(c, &hdev->conn_hash.list, list) {
-               if (c == conn)
-                       break;
-       }
-       rcu_read_unlock();
-
-       if (c == conn) {
-               hci_connect_le_scan_cleanup(conn, 0x00);
-               hci_conn_cleanup(conn);
-       }
-
-       hci_dev_unlock(hdev);
-       hci_dev_put(hdev);
-       hci_conn_put(conn);
-}
-
-static void hci_connect_le_scan_remove(struct hci_conn *conn)
-{
-       BT_DBG("%s hcon %p", conn->hdev->name, conn);
-
-       /* We can't call hci_conn_del/hci_conn_cleanup here since that
-        * could deadlock with another hci_conn_del() call that's holding
-        * hci_dev_lock and doing cancel_delayed_work_sync(&conn->disc_work).
-        * Instead, grab temporary extra references to the hci_dev and
-        * hci_conn and perform the necessary cleanup in a separate work
-        * callback.
-        */
-
-       hci_dev_hold(conn->hdev);
-       hci_conn_get(conn);
-
-       /* Even though we hold a reference to the hdev, many other
-        * things might get cleaned up meanwhile, including the hdev's
-        * own workqueue, so we can't use that for scheduling.
-        */
-       schedule_work(&conn->le_scan_cleanup);
-}
-
 static void hci_acl_create_connection(struct hci_conn *conn)
 {
        struct hci_dev *hdev = conn->hdev;
@@ -679,13 +628,6 @@ static void hci_conn_timeout(struct work_struct *work)
        if (refcnt > 0)
                return;
 
-       /* LE connections in scanning state need special handling */
-       if (conn->state == BT_CONNECT && conn->type == LE_LINK &&
-           test_bit(HCI_CONN_SCANNING, &conn->flags)) {
-               hci_connect_le_scan_remove(conn);
-               return;
-       }
-
        hci_abort_conn(conn, hci_proto_disconn_ind(conn));
 }
 
@@ -1066,7 +1008,6 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
        INIT_DELAYED_WORK(&conn->auto_accept_work, hci_conn_auto_accept);
        INIT_DELAYED_WORK(&conn->idle_work, hci_conn_idle);
        INIT_DELAYED_WORK(&conn->le_conn_timeout, le_conn_timeout);
-       INIT_WORK(&conn->le_scan_cleanup, le_scan_cleanup);
 
        atomic_set(&conn->refcnt, 0);
 
@@ -2888,81 +2829,46 @@ u32 hci_conn_get_phy(struct hci_conn *conn)
        return phys;
 }
 
-int hci_abort_conn(struct hci_conn *conn, u8 reason)
+static int abort_conn_sync(struct hci_dev *hdev, void *data)
 {
-       int r = 0;
+       struct hci_conn *conn;
+       u16 handle = PTR_ERR(data);
 
-       if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags))
+       conn = hci_conn_hash_lookup_handle(hdev, handle);
+       if (!conn)
                return 0;
 
-       switch (conn->state) {
-       case BT_CONNECTED:
-       case BT_CONFIG:
-               if (conn->type == AMP_LINK) {
-                       struct hci_cp_disconn_phy_link cp;
+       return hci_abort_conn_sync(hdev, conn, conn->abort_reason);
+}
 
-                       cp.phy_handle = HCI_PHY_HANDLE(conn->handle);
-                       cp.reason = reason;
-                       r = hci_send_cmd(conn->hdev, HCI_OP_DISCONN_PHY_LINK,
-                                        sizeof(cp), &cp);
-               } else {
-                       struct hci_cp_disconnect dc;
+int hci_abort_conn(struct hci_conn *conn, u8 reason)
+{
+       struct hci_dev *hdev = conn->hdev;
 
-                       dc.handle = cpu_to_le16(conn->handle);
-                       dc.reason = reason;
-                       r = hci_send_cmd(conn->hdev, HCI_OP_DISCONNECT,
-                                        sizeof(dc), &dc);
-               }
+       /* If abort_reason has already been set it means the connection is
+        * already being aborted so don't attempt to overwrite it.
+        */
+       if (conn->abort_reason)
+               return 0;
 
-               conn->state = BT_DISCONN;
+       bt_dev_dbg(hdev, "handle 0x%2.2x reason 0x%2.2x", conn->handle, reason);
 
-               break;
-       case BT_CONNECT:
-               if (conn->type == LE_LINK) {
-                       if (test_bit(HCI_CONN_SCANNING, &conn->flags))
-                               break;
-                       r = hci_send_cmd(conn->hdev,
-                                        HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
-               } else if (conn->type == ACL_LINK) {
-                       if (conn->hdev->hci_ver < BLUETOOTH_VER_1_2)
-                               break;
-                       r = hci_send_cmd(conn->hdev,
-                                        HCI_OP_CREATE_CONN_CANCEL,
-                                        6, &conn->dst);
-               }
-               break;
-       case BT_CONNECT2:
-               if (conn->type == ACL_LINK) {
-                       struct hci_cp_reject_conn_req rej;
-
-                       bacpy(&rej.bdaddr, &conn->dst);
-                       rej.reason = reason;
-
-                       r = hci_send_cmd(conn->hdev,
-                                        HCI_OP_REJECT_CONN_REQ,
-                                        sizeof(rej), &rej);
-               } else if (conn->type == SCO_LINK || conn->type == ESCO_LINK) {
-                       struct hci_cp_reject_sync_conn_req rej;
-
-                       bacpy(&rej.bdaddr, &conn->dst);
-
-                       /* SCO rejection has its own limited set of
-                        * allowed error values (0x0D-0x0F) which isn't
-                        * compatible with most values passed to this
-                        * function. To be safe hard-code one of the
-                        * values that's suitable for SCO.
-                        */
-                       rej.reason = HCI_ERROR_REJ_LIMITED_RESOURCES;
+       conn->abort_reason = reason;
 
-                       r = hci_send_cmd(conn->hdev,
-                                        HCI_OP_REJECT_SYNC_CONN_REQ,
-                                        sizeof(rej), &rej);
+       /* If the connection is pending check the command opcode since that
+        * might be blocking on hci_cmd_sync_work while waiting its respective
+        * event so we need to hci_cmd_sync_cancel to cancel it.
+        */
+       if (conn->state == BT_CONNECT && hdev->req_status == HCI_REQ_PEND) {
+               switch (hci_skb_event(hdev->sent_cmd)) {
+               case HCI_EV_LE_CONN_COMPLETE:
+               case HCI_EV_LE_ENHANCED_CONN_COMPLETE:
+               case HCI_EVT_LE_CIS_ESTABLISHED:
+                       hci_cmd_sync_cancel(hdev, -ECANCELED);
+                       break;
                }
-               break;
-       default:
-               conn->state = BT_CLOSED;
-               break;
        }
 
-       return r;
+       return hci_cmd_sync_queue(hdev, abort_conn_sync, ERR_PTR(conn->handle),
+                                 NULL);
 }
index 73b49d0..5f7a901 100644 (file)
@@ -5274,22 +5274,27 @@ static int hci_disconnect_sync(struct hci_dev *hdev, struct hci_conn *conn,
 }
 
 static int hci_le_connect_cancel_sync(struct hci_dev *hdev,
-                                     struct hci_conn *conn)
+                                     struct hci_conn *conn, u8 reason)
 {
+       /* Return reason if scanning since the connection shall probably be
+        * cleanup directly.
+        */
        if (test_bit(HCI_CONN_SCANNING, &conn->flags))
-               return 0;
+               return reason;
 
-       if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags))
+       if (conn->role == HCI_ROLE_SLAVE ||
+           test_and_set_bit(HCI_CONN_CANCEL, &conn->flags))
                return 0;
 
        return __hci_cmd_sync_status(hdev, HCI_OP_LE_CREATE_CONN_CANCEL,
                                     0, NULL, HCI_CMD_TIMEOUT);
 }
 
-static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn)
+static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn,
+                                  u8 reason)
 {
        if (conn->type == LE_LINK)
-               return hci_le_connect_cancel_sync(hdev, conn);
+               return hci_le_connect_cancel_sync(hdev, conn, reason);
 
        if (hdev->hci_ver < BLUETOOTH_VER_1_2)
                return 0;
@@ -5342,9 +5347,11 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
        case BT_CONFIG:
                return hci_disconnect_sync(hdev, conn, reason);
        case BT_CONNECT:
-               err = hci_connect_cancel_sync(hdev, conn);
+               err = hci_connect_cancel_sync(hdev, conn, reason);
                /* Cleanup hci_conn object if it cannot be cancelled as it
-                * likelly means the controller and host stack are out of sync.
+                * likelly means the controller and host stack are out of sync
+                * or in case of LE it was still scanning so it can be cleanup
+                * safely.
                 */
                if (err) {
                        hci_dev_lock(hdev);
@@ -6258,7 +6265,7 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn)
 
 done:
        if (err == -ETIMEDOUT)
-               hci_le_connect_cancel_sync(hdev, conn);
+               hci_le_connect_cancel_sync(hdev, conn, 0x00);
 
        /* Re-enable advertising after the connection attempt is finished. */
        hci_resume_advertising_sync(hdev);
index 4c352ab..5707ff4 100644 (file)
@@ -3586,18 +3586,6 @@ unlock:
        return err;
 }
 
-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;
-
-       return hci_abort_conn_sync(hdev, conn, HCI_ERROR_REMOTE_USER_TERM);
-}
-
 static int cancel_pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
                              u16 len)
 {
@@ -3648,8 +3636,7 @@ static int cancel_pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
                                              le_addr_type(addr->type));
 
        if (conn->conn_reason == CONN_REASON_PAIR_DEVICE)
-               hci_cmd_sync_queue(hdev, abort_conn_sync, ERR_PTR(conn->handle),
-                                  NULL);
+               hci_abort_conn(conn, HCI_ERROR_REMOTE_USER_TERM);
 
 unlock:
        hci_dev_unlock(hdev);