[PATCH] Bluetooth: hci_sync: Protect accept list membership lookups

[PATCH] Bluetooth: hci_sync: Protect accept list membership lookups

[Cen Zhang]

Entries in hdev->le_accept_list must remain allocated while a reader
compares their address and type. hci_le_del_accept_list_sync() and
hci_le_add_accept_list_sync() call hci_bdaddr_list_lookup() without
hdev->lock, although command-complete handlers modify and free entries
under that mutex. The request worker's req_lock does not serialize raw
HCI commands or their completions.

With a nonempty accept list and scanning disabled for passive-scan
reconciliation, a successful raw LE Clear Accept List completion can
overlap a membership check. For example, removing the last pending peer
A stops scanning but leaves A in the accept list. Adding a new pending
peer B then reaches deletion of stale A:

  1. The request worker holds req_lock, enters
     hci_le_del_accept_list_sync() and obtains entry A during
     hci_bdaddr_list_lookup(), without hdev->lock.
  2. A CAP_NET_RAW sender submits LE Clear Accept List through a raw HCI
     socket. The event worker handles its successful completion in
     hci_cc_le_clear_accept_list(), takes hdev->lock, then unlinks and
     frees A through hci_bdaddr_list_clear() and releases hdev->lock.
  3. After that free, the request worker compares A's bdaddr_type.

The final comparison reads freed memory; KASAN reported a one-byte read
22 bytes into the bdaddr_list allocation. Hold hdev->lock across both
membership checks so the clear completion cannot free an entry during
the traversal. Keep the delete command's address copy under that lock
as well, because its caller can pass an address embedded in the list
entry. Release the mutex before resolving-list operations or synchronous
HCI commands, whose completion handlers also acquire it.

KASAN report as below:

    BUG: KASAN: slab-use-after-free in hci_bdaddr_list_lookup+0x107/0x120
    Read of size 1 at addr ffff8881058e8c56 by task kworker/u17:0/501

    Workqueue: hci0 hci_cmd_sync_work
    Call Trace:
     <TASK>
     dump_stack_lvl+0x93/0xd0
     print_report+0xce/0x630
     ? hci_bdaddr_list_lookup+0x107/0x120
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __virt_addr_valid+0x20d/0x410
     ? hci_bdaddr_list_lookup+0x107/0x120
     kasan_report+0xe0/0x110
     ? hci_bdaddr_list_lookup+0x107/0x120
     hci_bdaddr_list_lookup+0x107/0x120
     hci_le_del_accept_list_sync.isra.0+0xa1/0x310
     ? __pfx_hci_le_del_accept_list_sync.isra.0+0x10/0x10
     ? hci_passive_scan_sync+0xd88/0x2090
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? lock_release+0xc8/0x280
     hci_passive_scan_sync+0x10a7/0x2090
     ? __pfx_hci_passive_scan_sync+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? pmbd_probe_hit_cookie+0x10/0x1c0
     ? __pfx_pmbd_probe_hit_cookie+0x10/0x10
     ? hci_update_passive_scan_sync+0x43f/0x9f0
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? lock_release+0xc8/0x280
     hci_update_passive_scan_sync+0x473/0x9f0
     hci_cmd_sync_work+0x1b3/0x450
     ? srso_alias_return_thunk+0x5/0xfbef5
     process_one_work+0x908/0x19c0
     ? __pfx_process_one_work+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? lock_is_held_type+0x8f/0x100
     ? srso_alias_return_thunk+0x5/0xfbef5
     worker_thread+0x65c/0xe40
     ? __pfx_worker_thread+0x10/0x10
     kthread+0x34f/0x460
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __pfx_kthread+0x10/0x10
     ret_from_fork+0x659/0x940
     ? __pfx_ret_from_fork+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __switch_to+0x74f/0xf80
     ? __pfx_kthread+0x10/0x10
     ret_from_fork_asm+0x1a/0x30
     </TASK>

    Allocated by task 504:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     __kasan_kmalloc+0xaa/0xb0
     __kmalloc_cache_noprof+0x251/0x610
     hci_bdaddr_list_add+0xdf/0x2c0
     hci_cc_le_add_to_accept_list+0x17a/0x230
     hci_cmd_complete_evt+0x37d/0xb60
     hci_event_packet+0x4e8/0xc70
     hci_rx_work+0x367/0xf00
     process_one_work+0x908/0x19c0
     worker_thread+0x65c/0xe40
     kthread+0x34f/0x460
     ret_from_fork+0x659/0x940
     ret_from_fork_asm+0x1a/0x30

    Freed by task 504:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     kasan_save_free_info+0x3b/0x60
     __kasan_slab_free+0x5f/0x80
     kfree+0x307/0x580
     hci_bdaddr_list_clear+0x134/0x230
     hci_cc_le_clear_accept_list+0x13d/0x240
     hci_cmd_complete_evt+0x37d/0xb60
     hci_event_packet+0x4e8/0xc70
     hci_rx_work+0x367/0xf00
     process_one_work+0x908/0x19c0
     worker_thread+0x65c/0xe40
     kthread+0x34f/0x460
     ret_from_fork+0x659/0x940
     ret_from_fork_asm+0x1a/0x30

    The buggy address belongs to the object at ffff8881058e8c40
                                                                which
                                belongs to the cache kmalloc-32 of size 32
    The buggy address is located 22 bytes inside of
                                                                freed
                                32-byte region [ffff8881058e8c40,
                                ffff8881058e8c60)

    The buggy address belongs to the physical page:
        page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0
    pfn:0x1058e8
    flags: 0x200000000000000(node=0|zone=2)
    page_type: f5(slab)
        raw: 0200000000000000 ffff888100042780 dead000000000100
    dead000000000122
        raw: 0000000000000000 0000000000400040 00000000f5000000
    0000000000000000
    page dumped because: kasan: bad access detected

    Memory state around the buggy address:
     ffff8881058e8b00: fa fb fb fb fc fc fc fc fa fb fb fb fc fc fc fc
     ffff8881058e8b80: fa fb fb fb fc fc fc fc fa fb fb fb fc fc fc fc
    >ffff8881058e8c00: fa fb fb fb fc fc fc fc fa fb fb fb fc fc fc fc
                                                     ^
     ffff8881058e8c80: fa fb fb fb fc fc fc fc fa fb fb fb fc fc fc fc
     ffff8881058e8d00: fa fb fb fb fc fc fc fc fa fb fb fb fc fc fc fc
    ==================================================================

Fixes: e8907f76544f ("Bluetooth: hci_sync: Make use of hci_cmd_sync_queue set 3")
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index efbd2e5387c0b..4e41c78a336b8 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -2473,11 +2473,16 @@ static int hci_le_del_accept_list_sync(struct hci_dev *hdev,
 	int err;
 
 	/* Check if device is on accept list before removing it */
-	if (!hci_bdaddr_list_lookup(&hdev->le_accept_list, bdaddr, bdaddr_type))
+	hci_dev_lock(hdev);
+	if (!hci_bdaddr_list_lookup(&hdev->le_accept_list, bdaddr,
+				    bdaddr_type)) {
+		hci_dev_unlock(hdev);
 		return 0;
+	}
 
 	cp.bdaddr_type = bdaddr_type;
 	bacpy(&cp.bdaddr, bdaddr);
+	hci_dev_unlock(hdev);
 
 	/* Ignore errors when removing from resolving list as that is likely
 	 * that the device was never added.
@@ -2653,9 +2658,13 @@ static int hci_le_add_accept_list_sync(struct hci_dev *hdev,
 	}
 
 	/* Check if already in accept list */
+	hci_dev_lock(hdev);
 	if (hci_bdaddr_list_lookup(&hdev->le_accept_list, &params->addr,
-				   params->addr_type))
+				   params->addr_type)) {
+		hci_dev_unlock(hdev);
 		return 0;
+	}
+	hci_dev_unlock(hdev);
 
 	*num_entries += 1;
 	cp.bdaddr_type = params->addr_type;