mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: msft: Revalidate address filters before canceling
@ 2026-10-22 12:16 Cen Zhang
  0 siblings, 0 replies; only message in thread
From: Cen Zhang @ 2026-10-22 12:16 UTC (permalink / raw)
  To: marcel, luiz.dentz
  Cc: linux-bluetooth, linux-kernel, baijiaju1990, jjzuming, zzzccc427

An address filter selected by msft_cancel_address_filter_sync() must
still be a live entry in msft->address_filters. The callback instead
trusts the raw pointer queued by msft_monitor_device_evt(), and its
HCI_UP test only describes the controller's current state.

On a configured controller using Microsoft address filters, a lost
monitor-device event can queue cancellation of an ADDED filter while
HCIDEVRESET holds req_lock. If reset closes and reopens successfully,
the following ordering is possible:

    HCIDEVRESET                RX worker           Request worker
    -----------                ---------           --------------
    hci_dev_do_reset()
      lock(req_lock)
                               lost event
                               state = REMOVING
                               queue cancel ------->
                                                   wait for req_lock
      hci_dev_close_sync()
        drain RX work
        msft_do_close()
          unlink filter
          kfree(filter)
      hci_dev_open_sync()
        set HCI_UP
      unlock(req_lock) ---------------------------->
                                                   acquire req_lock
                                                   cancel callback:
                                                     HCI_UP is set
                                                     list_del(...)
                                                       use-after-free

The reset path drains receive work, but not the separate request
workqueue. Thus req_lock serializes the bodies without preserving the
queued filter, and list_del() reads freed storage.

Find the queued pointer in address_filters under filter_lock and
require AF_STATE_REMOVING before unlinking or reading its fields. An
allocation reusing that address cannot become ADDED and queue its own
cancellation until its add work runs, which is ordered after this old
cancel request. Checking both membership and REMOVING therefore also
rejects a replacement allocation.

Keep REMOVING filters listed when their pattern monitor is removed so
the cancellation callback can still find and release them. Restore
ADDED if cancellation cannot be queued, allowing synchronous removal
to reclaim that filter.

KASAN report as below:

    ==================================================================
        BUG: KASAN: slab-use-after-free in
    __list_del_entry_valid_or_report+0x1c6/0x210
    Read of size 8 at addr ffff888108985da0 by task kworker/u17:2/494

    Workqueue: hci0 hci_cmd_sync_work
    Call Trace:
     <TASK>
     dump_stack_lvl+0x93/0xd0
     print_report+0xce/0x630
     ? __list_del_entry_valid_or_report+0x1c6/0x210
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __virt_addr_valid+0x20d/0x410
     ? __list_del_entry_valid_or_report+0x1c6/0x210
     kasan_report+0xe0/0x110
     ? __list_del_entry_valid_or_report+0x1c6/0x210
     __list_del_entry_valid_or_report+0x1c6/0x210
     msft_cancel_address_filter_sync+0x14f/0x510
     ? __pfx_msft_cancel_address_filter_sync+0x10/0x10
     ? __pfx_msft_cancel_address_filter_sync+0x10/0x10
     hci_cmd_sync_work+0x1cc/0x4a0
     ? 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
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __kthread_parkme+0x175/0x220
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __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 512:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     __kasan_kmalloc+0xaa/0xb0
     __kmalloc_cache_noprof+0x251/0x610
     msft_add_address_filter+0x84/0x850
     msft_test_stage_reset_route+0x380/0x540
     msft_test_stage_write+0x11c/0x170
     full_proxy_write+0x11f/0x180
     vfs_write+0x25a/0x1010
     ksys_write+0x111/0x200
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    Freed by task 511:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     kasan_save_free_info+0x3b/0x60
     __kasan_slab_free+0x5f/0x80
     kfree+0x307/0x580
     msft_do_close+0x3b9/0x6a0
     hci_dev_close_sync+0xfef/0x1220
     hci_dev_reset+0x187/0x450
     hci_sock_ioctl+0x3f3/0x710
     sock_do_ioctl+0x120/0x2b0
     sock_ioctl+0x41b/0x670
     __x64_sys_ioctl+0x163/0x1d0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

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

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

    Memory state around the buggy address:
     ffff888108985c80: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
     ffff888108985d00: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
    >ffff888108985d80: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
                                   ^
     ffff888108985e00: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
     ffff888108985e80: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
    ==================================================================

Fixes: 9e14606d8f38 ("Bluetooth: msft: Extended monitor tracking by address filter")
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
Sashiko review follow-up: MSFT handle-map patch
  - Address the stale address-filter cancel issue raised by Sashiko:
    https://sashiko.dev/#/patchset/86251
  - Revalidate the address filter after reset.

Related handle-map fix already merged in bluetooth-next:
56e20c3d8ad340bbfc90221c4aae1c870485fb57

diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -323,6 +323,10 @@
 		if (address_filter->pattern_handle != handle)
 			continue;
 
+		/* A queued cancel still owns this linked filter. */
+		if (address_filter->state == AF_STATE_REMOVING)
+			continue;
+
 		list_del(&address_filter->list);
 
 		/* Keep the address filter and let
@@ -333,13 +337,6 @@
 			address_filter->state = AF_STATE_REMOVING;
 			continue;
 		}
-
-		/* Keep the address filter and let
-		 * msft_cancel_address_filter_sync() remove and free the address
-		 * filter
-		 */
-		if (address_filter->state == AF_STATE_REMOVING)
-			continue;
 
 		list_add_tail(&address_filter->list, &head);
 	}
@@ -716,7 +713,8 @@
 
 static int msft_cancel_address_filter_sync(struct hci_dev *hdev, void *data)
 {
-	struct msft_monitor_addr_filter_data *address_filter = data;
+	struct msft_monitor_addr_filter_data *address_filter = NULL;
+	struct msft_monitor_addr_filter_data *entry;
 	struct msft_cp_le_cancel_monitor_advertisement cp;
 	struct msft_data *msft = hdev->msft_data;
 	struct sk_buff *skb;
@@ -731,9 +729,23 @@
 	if (!test_bit(HCI_UP, &hdev->flags))
 		return 0;
 
+	/* Reset may free data and restore HCI_UP. Find the pointer
+	 * without dereferencing it. A replacement cannot finish its queued
+	 * add before this cancel on req_workqueue.
+	 */
 	mutex_lock(&msft->filter_lock);
-	list_del(&address_filter->list);
+	list_for_each_entry(entry, &msft->address_filters, list) {
+		if (entry != data || entry->state != AF_STATE_REMOVING)
+			continue;
+
+		address_filter = entry;
+		list_del(&address_filter->list);
+		break;
+	}
 	mutex_unlock(&msft->filter_lock);
+
+	if (!address_filter)
+		return 0;
 
 	cp.sub_opcode = MSFT_OP_LE_CANCEL_MONITOR_ADVERTISEMENT;
 	cp.handle = address_filter->msft_handle;
@@ -1063,10 +1075,10 @@
 	} else {
 		if (address_filter && address_filter->state == AF_STATE_ADDED) {
 			address_filter->state = AF_STATE_REMOVING;
-			hci_cmd_sync_queue(hdev,
-					   msft_cancel_address_filter_sync,
-					   address_filter,
-					   NULL);
+			if (hci_cmd_sync_queue(
+				    hdev, msft_cancel_address_filter_sync,
+				    address_filter, NULL) < 0)
+				address_filter->state = AF_STATE_ADDED;
 		}
 		msft_device_lost(hdev, &ev->bdaddr, addr_type, mgmt_handle);
 	}

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2026-10-22 12:16 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-22 12:16 [PATCH] Bluetooth: msft: Revalidate address filters before canceling Cen Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®