[PATCH] Bluetooth: vhci: Flush suspend work after removing debugfs

[PATCH] Bluetooth: vhci: Flush suspend work after removing debugfs

[Cen Zhang]

The VHCI controller must remain alive until suspend_work finishes.
vhci_release() flushes the work before removing the force_suspend
debugfs entry, so an active writer can queue work after that flush.

A force_suspend write changing the suspend state can overlap the last
close of /dev/vhci as follows:

    /dev/vhci close          Debugfs writer        Suspend worker
    ---------------          --------------        --------------
    vhci_release()           force_suspend_write()
      flush_work() returns     [not queued yet]
                               schedule_work() -->
                                                   hci_suspend_dev()
                                                     check powered
                                                     check !HCI_UNREGISTER
                                                     hci_suspend_sync()
                                                     unlock(req_lock)
      vhci_debugfs_remove()
        wait for writer
      hci_unregister_dev()
      hci_free_dev()
        drop last reference
        free hdev
                                                   hci_clear_wake_reason()
                                                     lock freed mutex
                                                     use-after-free

The controller locks serialize state access but do not retain the
allocation. Unregister drains HCI-owned work, but does not cover VHCI's
suspend_work on the system workqueue.

Flush suspend_work again after removing the debugfs entries and before
unregistering the controller. Removal waits for active writers and
prevents new callbacks, so the second flush drains all remaining work
while the controller and driver data are still alive. Retain the initial
flush to preserve the existing teardown order.

Runtime testing of this change was not performed.

KASAN report as below:

    ==================================================================
    BUG: KASAN: use-after-free in __mutex_lock+0x1e68/0x2590
    Read of size 8 at addr ffff8881011b80b0 by task kworker/0:0/9

    Workqueue: events vhci_suspend_work
    Call Trace:
     <TASK>
     dump_stack_lvl+0x93/0xd0
     print_report+0xce/0x630
     ? __mutex_lock+0x1e68/0x2590
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __virt_addr_valid+0x20d/0x410
     ? __mutex_lock+0x1e68/0x2590
     kasan_report+0xe0/0x110
     ? __mutex_lock+0x1e68/0x2590
     __mutex_lock+0x1e68/0x2590
     ? hci_suspend_dev+0x1ff/0x650
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? mark_held_locks+0x40/0x70
     ? _raw_spin_unlock_irqrestore+0x44/0x60
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? lockdep_hardirqs_on_prepare+0xea/0x1a0
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __pfx___mutex_lock+0x10/0x10
     ? _raw_spin_unlock_irqrestore+0x44/0x60
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? pmbd_gate_site_armed+0x31/0x1c0
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __pfx_pmbd_probe_hit_cookie+0x10/0x10
     ? hci_suspend_dev+0x1ff/0x650
     hci_suspend_dev+0x1ff/0x650
     vhci_suspend_work+0xa5/0x190
     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>

    The buggy address belongs to the physical page:
        page: refcount:0 mapcount:0 mapping:0000000000000000
    index:0xffff8881011b9040 pfn:0x1011b8
    flags: 0x200000000000000(node=0|zone=2)
        raw: 0200000000000000 ffffea0004073308 ffff88818863f180
    0000000000000000
        raw: ffff8881011b9040 0000000000000000 00000000ffffffff
    0000000000000000
    page dumped because: kasan: bad access detected

    Memory state around the buggy address:
     ffff8881011b7f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
     ffff8881011b8000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
    >ffff8881011b8080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
                                         ^
     ffff8881011b8100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
     ffff8881011b8180: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
    ==================================================================

Fixes: d16e6d19ccc6 ("Bluetooth: hci_vhci: Fix calling hci_{suspend,resume}_dev")
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index 8f7db0102b25f..bedba3c140ac2 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -701,6 +701,8 @@ static int vhci_release(struct inode *inode, struct file *file)
 	if (hdev) {
 		if (!IS_ERR_OR_NULL(hdev->debugfs))
 			vhci_debugfs_remove(hdev);
+		/* Drain work queued by the last force_suspend writers. */
+		flush_work(&data->suspend_work);
 		hci_unregister_dev(hdev);
 		hci_free_dev(hdev);
 	}

Re: [PATCH] Bluetooth: vhci: Flush suspend work after removing debugfs

[Chad Aldreda]

On Sun, 25 Oct 2026, Cen Zhang wrote:
> Runtime testing of this change was not performed.

The use-after-free in your report reproduces on mainline with the two
test-only delays below, and your patch removes it.

Tested on mainline d48723711bae (v7.4-rc1-672), whose hci_vhci.c and
hci_sync.c match bluetooth-next at 983f3fcb1282 (hci_core.c differs
only in rfkill handling), on x86_64 in QEMU (4 vCPUs) with KASAN,
lockdep, DEBUG_LIST, kmemleak and hung-task detection. A test program
opens /dev/vhci, answers every HCI command with a Command Complete,
brings the controller up with HCIDEVUP, then closes /dev/vhci while
another thread keeps toggling force_suspend in debugfs. One mode keeps
answering commands until close() returns; the other stops answering
before close().

Unaided the window is too narrow: 300 cycles in the answering mode on
the unpatched kernel gave no reports. Both kernels therefore carried
two test-only delays, not for merging:

  vhci_release():
  	cancel_delayed_work_sync(&data->open_timeout);
  	flush_work(&data->suspend_work);
  +	msleep(50);

  hci_suspend_dev():
  	hci_req_sync_unlock(hdev);
  +	msleep(200);
  	hci_clear_wake_reason(hdev);

With the delays and without your patch, 30 cycles in each mode gave
834 and 621 KASAN reports (kasan_multi_shot, so one bad close gives
many). Two objects are hit. The hci_dev is accessed after free from
hci_suspend_dev(), including the __mutex_lock() site in your report; it
is page-backed, so KASAN shows no free stack, and once the page is
reused the same accesses show up as slab-out-of-bounds against
unrelated slab objects. The vhci_data (kmalloc in vhci_open(), kfree in
vhci_release()) is read by vhci_suspend_work() and read and written by
process_one_work() after free, because suspend_work is embedded in it.
hci_resume_dev() was also reached, with an hdev pointer read from the
freed vhci_data, so it dereferenced an unrelated address.

With the delays and your patch, 160 cycles (130 answering, 30 not) had
no KASAN, lockdep, DEBUG_LIST, kmemleak or hung-task reports. Without
the delays, 150 cycles with the patch (answering mode) were also
clean; since the unpatched kernel was clean without them too, that run
only shows no regression. As expected, release can now also wait for
suspend or resume work that is already running, including an HCI
command timeout inside it, as the existing first flush_work() already
can.

Not tested: suspend with force_wakeup set, with connections, or with
scanning, advertising or discovery active (the controller was brought
up with HCIDEVUP and no bluetoothd, so hci_suspend_sync() had nothing
to pause), mgmt, real controllers or btvirt, module unload (Bluetooth
was built in), other architectures, and a build of bluetooth-next
itself (compared at source level only).

The harness and this report were produced with an AI assistant (Claude
Opus 5.5), which also did the earlier runs, including the unpatched
comparison. I ran the final version on the patched kernel myself and
got the same results. The harness is available on request.

Tested-by: Chad Aldreda <ca@modelmiser.com>