[PATCH] Bluetooth: hci_sync: Reprogram the resolving list when a peer's IRK changes

[PATCH] Bluetooth: hci_sync: Reprogram the resolving list when a peer's IRK changes

[Brandon Williams]

hci_le_add_resolve_list_sync() skips a device whose identity address is
already in the controller's resolving list without comparing the IRK.
The programmed IRK can be stale: hci_add_irk() updates a known device's
IRK in place, and an unpaired device can stay programmed, since
hci_update_passive_scan_sync() only disables scanning when no device
needs it, so hci_update_accept_list_sync() does not run to remove it.

When a bonded device pairs again under the same identity address with a
new IRK, for example because it generates a new IRK for each bond, or
with none, the stale entry stays. The controller then cannot resolve the
device's new RPAs, or keeps expecting RPAs from it, so with the accept
list filter policy its advertisements are dropped and background
reconnection fails until the adapter is powered off and on, which clears
the resolving list.

Remove the entry when the IRK differs or is gone, ignoring errors as
hci_le_del_accept_list_sync() does, and program the new IRK if there is
one. Copy the IRK and compare it with the entry under hdev->lock first,
as either can be replaced or freed while the removal waits for the
controller.

Fixes: 0eee35bdfa3b ("Bluetooth: Update resolving list when updating whitelist")
Assisted-by: LLM sparse
Signed-off-by: Brandon Williams <turbine.bmw@gmail.com>
---

Notes:
    Found on a Surface Pro 11 (X1E80100, WCN7850) with a Surface Slim Pen 2,
    which distributes a new IRK each time it is paired. After the pen was
    paired again, the HCI trace showed no LE Add Device To Resolving List
    (LE Set Privacy Mode for its address still succeeded), and its
    advertisements were not reported until the adapter was powered off and
    on.
    
    This applies on top of 2a464ab1c5ac ("Bluetooth: hci_sync: Protect
    resolving-list lookups"): the lookup stays under hdev->lock and the
    comparison is done there too. The IRK is copied under rcu_read_lock(),
    as 4b76fa362110 does in hci_le_set_privacy_mode_sync(); before this
    patch irk->val was read after the RCU read section had ended.
    
    Choices a reviewer may ask about:
    - The entry is removed only when the IRK differs, not on every call:
      this runs for each device in the accept list on every passive scan
      update, so removing and adding it unconditionally would cost two
      more commands per device each time.
    - Errors from the Remove are ignored: hci_cc_le_del_from_resolv_list()
      only drops the entry from hdev->le_resolv_list on success, so after a
      failure the old entry is still listed and the next update tries
      again.
    - Fixes names 0eee35bdfa3b, which started adding devices to the
      resolving list along with the accept list and skipped one that was
      already listed. That code moved to hci_sync.c in e8907f76544f
      ("Bluetooth: hci_sync: Make use of hci_cmd_sync_queue set 3"), so
      this patch applies from there on.
    
    Testing:
    - Two new mgmt-tester tests, sent to this list as "[PATCH BlueZ]
      mgmt-tester: Add LL Privacy tests for a device's changed IRK", where
      the device gets a new IRK or loses its IRK while in the resolving
      list. On bluetooth-next 47d4d9f67e92 under QEMU (arm64, TCG) with
      KASAN, PROVE_LOCKING, PROVE_RCU and DEBUG_ATOMIC_SLEEP: without this
      patch both time out (10/10 runs); with it the LL Privacy tests pass
      290/290 over 10 runs, the full mgmt-tester run passes 516 of 517
      ("Set powered on - Blocked by rfkill" skips itself, as the test kernel
      has no /dev/rfkill), and there are no KASAN, lockdep or RCU reports.
    - Hardware, backported to 7.3 (WCN7850), replayed over the management
      socket with bluetoothd stopped: Load IRKs with a made-up IRK, Add
      Device and Set Device Flags; then the real IRK, no IRK, and the real
      IRK again, each followed by Set Device Flags. The controller got Add
      (made-up IRK); Remove, Add (real IRK) and Set Privacy Mode; Remove
      only; Add (real IRK) and Set Privacy Mode. The pen then reconnected
      with its advertisement reported as Resolved Random.
    - sparse and W=1 are clean for hci_sync.o; checkpatch --strict is clean.
    
    This is not a security issue: the effect is that a re-paired device
    cannot reconnect in the background.
    
    Tool use: the patch, its changelog and the BlueZ tests were written
    with an AI coding assistant (Claude Code, Claude Opus 5.5) in a longer
    session that I directed: working out from btmon traces why the pen did
    not reconnect after pairing again, writing the fix and the tests, and
    running the emulator and hardware tests above. A separate AI review of
    the patch found the IRK lifetime across the blocking command, the
    no-IRK case and the error handling, which are fixed here.
    I ran into this issue with my own hardware and ran the hardware test
    above myself. I have read the patch and the tests, and I understand why
    the IRK is compared under hdev->lock and why the stale entry is removed
    before the new one is added.

 net/bluetooth/hci_sync.c | 41 ++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 9d71ce96bd8e..44d992956c8c 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -2514,8 +2514,9 @@ static int hci_le_add_resolve_list_sync(struct hci_dev *hdev,
 {
 	struct hci_cp_le_add_to_resolv_list cp;
 	struct smp_irk *irk;
+	struct bdaddr_list_with_irk *entry;
 	struct hci_conn_params *p;
-	bool exists;
+	bool has_irk, programmed, stale;
 
 	if (!ll_privacy_capable(hdev))
 		return 0;
@@ -2533,21 +2534,45 @@ static int hci_le_add_resolve_list_sync(struct hci_dev *hdev,
 	} else if (!(params->flags & HCI_CONN_FLAG_ADDRESS_RESOLUTION))
 		return 0;
 
+	/* Copy the IRK and compare it with the programmed one now, as both
+	 * can be replaced or freed while waiting for the controller below.
+	 */
+	hci_dev_lock(hdev);
+
+	rcu_read_lock();
 	irk = hci_find_irk_by_addr(hdev, &params->addr, params->addr_type);
-	if (!irk)
-		return 0;
+	has_irk = irk;
+	if (has_irk)
+		memcpy(cp.peer_irk, irk->val, sizeof(cp.peer_irk));
+	rcu_read_unlock();
 
 	/* Check if the IK has _not_ been programmed yet. */
-	hci_dev_lock(hdev);
-	exists = hci_bdaddr_list_lookup_with_irk(
-		&hdev->le_resolv_list, &params->addr, params->addr_type);
+	entry = hci_bdaddr_list_lookup_with_irk(&hdev->le_resolv_list,
+						&params->addr,
+						params->addr_type);
+	programmed = entry;
+	stale = entry && (!has_irk || memcmp(entry->peer_irk, cp.peer_irk,
+					     sizeof(cp.peer_irk)));
+
 	hci_dev_unlock(hdev);
-	if (exists)
+
+	if (programmed && !stale)
+		return 0;
+
+	/* The device paired again with a new IRK, or without one, under the
+	 * same identity address. Remove the entry, or the controller keeps
+	 * resolving with the old IRK and the device cannot reconnect. Ignore
+	 * errors as hci_le_del_accept_list_sync() does.
+	 */
+	if (stale)
+		hci_le_del_resolve_list_sync(hdev, &params->addr,
+					     params->addr_type);
+
+	if (!has_irk)
 		return 0;
 
 	cp.bdaddr_type = params->addr_type;
 	bacpy(&cp.bdaddr, &params->addr);
-	memcpy(cp.peer_irk, irk->val, 16);
 
 	/* Default privacy mode is always Network */
 	params->privacy_mode = HCI_NETWORK_PRIVACY;

base-commit: 47d4d9f67e92d56ef3965f2727f9c98a5a611368
-- 
2.56.0