Bluetooth: hci_sync: extend conn_hash lookup critical sections

Using RCU-protected pointers outside the critical sections without
refcount is incorrect and may result to UAF.

Extend critical section to cover both hci_conn_hash lookup and use of
the returned conn.

Add surrounding rcu_read_lock() also when return value is not used, in
preparation for RCU lockdep requirement to hci_lookup_le_connect().

This avoids concurrent deletion of the conn before we are done
dereferencing it.

Also, make sure to hold hdev->lock when accessing hdev->accept_list.

Fixes: 6d0417e4e1cf ("Bluetooth: hci_conn: Fix not setting conn_timeout for Broadcast Receiver")
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/hci_sync.c b/net/bluetooth/hci_sync.c
index c896d4edd0133e3c1ff3304cd71c984372a22732..7a60e34c91b6c46d7c3a7a35ad26f6fb1676b8f8 100644 (file)
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -1054,14 +1054,19 @@ static int hci_set_random_addr_sync(struct hci_dev *hdev, bdaddr_t *rpa)
         * In this kind of scenario skip the update and let the random
         * address be updated at the next cycle.
         */
+       rcu_read_lock();
+
        if (bacmp(&hdev->random_addr, BDADDR_ANY) &&
            (hci_dev_test_flag(hdev, HCI_LE_ADV) ||
            hci_lookup_le_connect(hdev))) {
                bt_dev_dbg(hdev, "Deferring random address update");
                hci_dev_set_flag(hdev, HCI_RPA_EXPIRED);
+               rcu_read_unlock();
                return 0;
        }
 
+       rcu_read_unlock();
+
        return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_RANDOM_ADDR,
                                     6, rpa, HCI_CMD_TIMEOUT);
 }
@@ -2647,12 +2652,17 @@ static int hci_pause_addr_resolution(struct hci_dev *hdev)
        /* Cannot disable addr resolution if scanning is enabled or
         * when initiating an LE connection.
         */
+       rcu_read_lock();
+
        if (hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
            hci_lookup_le_connect(hdev)) {
+               rcu_read_unlock();
                bt_dev_err(hdev, "Command not allowed when scan/LE connect");
                return -EPERM;
        }
 
+       rcu_read_unlock();
+
        /* Cannot disable addr resolution if advertising is enabled. */
        err = hci_pause_advertising_sync(hdev);
        if (err) {
@@ -2790,6 +2800,8 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
        if (hci_dev_test_flag(hdev, HCI_PA_SYNC)) {
                struct hci_conn *conn;
 
+               rcu_read_lock();
+
                conn = hci_conn_hash_lookup_create_pa_sync(hdev);
                if (conn) {
                        struct conn_params pa;
@@ -2799,6 +2811,8 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
                        bacpy(&pa.addr, &conn->dst);
                        pa.addr_type = conn->dst_type;
 
+                       rcu_read_unlock();
+
                        /* Clear first since there could be addresses left
                         * behind.
                         */
@@ -2808,6 +2822,8 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
                        err = hci_le_add_accept_list_sync(hdev, &pa,
                                                          &num_entries);
                        goto done;
+               } else {
+                       rcu_read_unlock();
                }
        }
 
@@ -2818,10 +2834,13 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
         * the controller.
         */
        list_for_each_entry_safe(b, t, &hdev->le_accept_list, list) {
-               if (hci_conn_hash_lookup_le(hdev, &b->bdaddr, b->bdaddr_type))
+               rcu_read_lock();
+
+               if (hci_conn_hash_lookup_le(hdev, &b->bdaddr, b->bdaddr_type)) {
+                       rcu_read_unlock();
                        continue;
+               }
 
-               /* Pointers not dereferenced, no locks needed */
                pend_conn = hci_pend_le_action_lookup(&hdev->pend_le_conns,
                                                      &b->bdaddr,
                                                      b->bdaddr_type);
@@ -2829,6 +2848,8 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
                                                        &b->bdaddr,
                                                        b->bdaddr_type);
 
+               rcu_read_unlock();
+
                /* If the device is not likely to connect or report,
                 * remove it from the acceptlist.
                 */
@@ -2955,6 +2976,8 @@ static int hci_le_set_ext_scan_param_sync(struct hci_dev *hdev, u8 type,
                if (sent) {
                        struct hci_conn *conn;
 
+                       rcu_read_lock();
+
                        conn = hci_conn_hash_lookup_ba(hdev, PA_LINK,
                                                       &sent->bdaddr);
                        if (conn) {
@@ -2979,8 +3002,12 @@ static int hci_le_set_ext_scan_param_sync(struct hci_dev *hdev, u8 type,
                                        phy++;
                                }
 
+                               rcu_read_unlock();
+
                                if (num_phy)
                                        goto done;
+                       } else {
+                               rcu_read_unlock();
                        }
                }
        }
@@ -3231,12 +3258,16 @@ int hci_update_passive_scan_sync(struct hci_dev *hdev)
                /* If there is at least one pending LE connection, we should
                 * keep the background scan running.
                 */
+               bool exists;
 
                /* If controller is connecting, we should not start scanning
                 * since some controllers are not able to scan and connect at
                 * the same time.
                 */
-               if (hci_lookup_le_connect(hdev))
+               rcu_read_lock();
+               exists = hci_lookup_le_connect(hdev);
+               rcu_read_unlock();
+               if (exists)
                        return 0;
 
                bt_dev_dbg(hdev, "start background scanning");
@@ -3454,6 +3485,7 @@ int hci_write_fast_connectable_sync(struct hci_dev *hdev, bool enable)
 }
 
 static bool disconnected_accept_list_entries(struct hci_dev *hdev)
+       __must_hold(&hdev->lock)
 {
        struct bdaddr_list *b;
 
@@ -3494,12 +3526,16 @@ int hci_update_scan_sync(struct hci_dev *hdev)
        if (hdev->scanning_paused)
                return 0;
 
+       hci_dev_lock(hdev);
+
        if (hci_dev_test_flag(hdev, HCI_CONNECTABLE) ||
            disconnected_accept_list_entries(hdev))
                scan = SCAN_PAGE;
        else
                scan = SCAN_DISABLED;
 
+       hci_dev_unlock(hdev);
+
        if (hci_dev_test_flag(hdev, HCI_DISCOVERABLE))
                scan |= SCAN_INQUIRY;