Bluetooth: mgmt: hold reference for hci_conn in mgmt_pending_cmds

Dereferencing RCU-protected pointers outside critical sections is
invalid and may lead to UAF.  Use of hci_conn in hci_sync callbacks also
needs to hold refcount to avoid UAF.

Take appropriate locks for hci_conn lookups, and take refcount for
hci_conn pointers stored in mgmt_pending_cmd so that the pointer stays
valid.

When accessing conn->state, ensure hdev->lock is held to avoid data
race.

Fixes: 7b445e220db9 ("Bluetooth: MGMT: Fix holding hci_conn reference while command is queued")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f3437a3fe4a15bd541046c68381e1814bd8d8a10..23a1aded0ca8fa77188a73614ea8fb480f9defd6 100644 (file)
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -7404,6 +7404,9 @@ static void get_conn_info_complete(struct hci_dev *hdev, void *data, int err)
                rp.max_tx_power = HCI_TX_POWER_INVALID;
        }
 
+       if (conn)
+               hci_conn_put(conn);
+
        mgmt_cmd_complete(cmd->sk, cmd->hdev->id, MGMT_OP_GET_CONN_INFO, status,
                          &rp, sizeof(rp));
 
@@ -7418,6 +7421,8 @@ static int get_conn_info_sync(struct hci_dev *hdev, void *data)
        int err;
        __le16   handle;
 
+       hci_dev_lock(hdev);
+
        /* Make sure we are still connected */
        if (cp->addr.type == BDADDR_BREDR)
                conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
@@ -7425,12 +7430,16 @@ static int get_conn_info_sync(struct hci_dev *hdev, void *data)
        else
                conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
 
-       if (!conn || conn->state != BT_CONNECTED)
+       if (!conn || conn->state != BT_CONNECTED) {
+               hci_dev_unlock(hdev);
                return MGMT_STATUS_NOT_CONNECTED;
+       }
 
-       cmd->user_data = conn;
+       cmd->user_data = hci_conn_get(conn);
        handle = cpu_to_le16(conn->handle);
 
+       hci_dev_unlock(hdev);
+
        /* Refresh RSSI each time */
        err = hci_read_rssi_sync(hdev, handle);
 
@@ -7564,6 +7573,9 @@ static void get_clock_info_complete(struct hci_dev *hdev, void *data, int err)
        }
 
 complete:
+       if (conn)
+               hci_conn_put(conn);
+
        mgmt_cmd_complete(cmd->sk, cmd->hdev->id, cmd->opcode, status, &rp,
                          sizeof(rp));
 
@@ -7580,15 +7592,21 @@ static int get_clock_info_sync(struct hci_dev *hdev, void *data)
        memset(&hci_cp, 0, sizeof(hci_cp));
        hci_read_clock_sync(hdev, &hci_cp);
 
+       hci_dev_lock(hdev);
+
        /* Make sure connection still exists */
        conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->addr.bdaddr);
-       if (!conn || conn->state != BT_CONNECTED)
+       if (!conn || conn->state != BT_CONNECTED) {
+               hci_dev_unlock(hdev);
                return MGMT_STATUS_NOT_CONNECTED;
+       }
 
-       cmd->user_data = conn;
+       cmd->user_data = hci_conn_get(conn);
        hci_cp.handle = cpu_to_le16(conn->handle);
        hci_cp.which = 0x01; /* Piconet clock */
 
+       hci_dev_unlock(hdev);
+
        return hci_read_clock_sync(hdev, &hci_cp);
 }