[PATCH] Bluetooth: hci_core: Fix queuing disc_work while workqueue is drained

[PATCH] Bluetooth: hci_core: Fix queuing disc_work while workqueue is drained

[Jeremy Dean Clifton]

hci_conn_drop() queues conn->disc_work on hdev->workqueue unconditionally
when the last reference to a connection is dropped. It can run from the
socket connect path while hci_dev_close_sync() is draining
hdev->workqueue (e.g. HCIDEVDOWN racing with an SCO connect). Since that
queue_delayed_work() is not chained from a work item already running on
the queue, __queue_work() sees the queue marked __WQ_DRAINING, warns and
drops the work:

  workqueue: cannot queue hci_conn_timeout on wq hci4
  WARNING: kernel/workqueue.c:2352 at __queue_work+0xdb7/0x1370
  Call Trace:
   __queue_delayed_work kernel/workqueue.c:2617
   queue_delayed_work_on kernel/workqueue.c:2669
   hci_conn_drop include/net/bluetooth/hci_core.h:1729 [inline]
   hci_connect_sco net/bluetooth/hci_conn.c:1881
   sco_connect net/bluetooth/sco.c:375 [inline]
   sco_sock_connect net/bluetooth/sco.c:764
   __sys_connect_file net/socket.c:2166
   __sys_connect net/socket.c:2183
   __x64_sys_connect net/socket.c:2186

hci_dev_close_sync() already sets HCI_CMD_DRAIN_WORKQUEUE and calls
synchronize_rcu() before draining, and keeps the flag set until the
device is fully closed. Skip queuing disc_work while the flag is set,
checking it under rcu_read_lock().

hci_conn_hash_flush(), called from hci_dev_close_sync(),
deletes the remaining connections.

Fixes: 525daaea459f ("Bluetooth: hci_sync: Set HCI_CMD_DRAIN_WORKQUEUE during device close")
Reported-by: syzbot+5897b3c764f5989ea942@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5897b3c764f5989ea942
Tested-by: syzbot+5897b3c764f5989ea942@syzkaller.appspotmail.com
Suggested-by: Hillf Danton <hdanton@sina.com>
Assisted-by: LLM
Signed-off-by: Jeremy Dean Clifton <deaner92@yahoo.com>
---

Notes:
    Testing:
    
    syzbot ran its reproducer for this bug on upstream 90201351a862:
      - without this patch, the warning reproduces:
        https://syzkaller.appspot.com/x/report.txt?x=17940305580000
      - with this patch, the reproducer does not trigger any issue:
        https://syzkaller.appspot.com/x/bisect.txt?x=1714618e580000
    
    The evidence is the pair of runs on
    90201351a862, where the same kernel crashed without this patch and
    did not crash with it. syzbot's own retest on
    58161ead29e5 (Oct 17) still reproduces the warning without this
    patch.
    
    This patch is based on bluetooth-next (185c11232d5f). I built
    net/bluetooth and hci_vhci for arm64 with CONFIG_PROVE_LOCKING=y
    with no new warnings, and the same change also builds on x86-64
    against v7.3. I could not run the reproducer locally; all runtime
    testing above is syzbot's.
    
    Hillf Danton tested an equivalent hci_conn_drop() check in February,
    together with a hci_dev_close_sync() change that is now upstream via
    525daaea459f:
    https://lore.kernel.org/all/694c9ee9.050a0220.35954c.0030.GAE@google.com/T/
    
    Tool use: An LLM helped me analyse the syzbot
    report, compare hci_conn_drop() with the existing
    HCI_CMD_DRAIN_WORKQUEUE checks, draft the changelog and prepare the
    syzbot test requests.
    
    I made the code change myself and checked the
    locking: the RCU read section pairs with synchronize_rcu() in
    hci_dev_close_sync(), and hci_conn_hash_flush() later deletes the
    remaining connections, so not queuing disc_work loses nothing.

 include/net/bluetooth/hci_core.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8f32c860d492..c8cc25d35195 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1775,8 +1775,11 @@ static inline void hci_conn_drop(struct hci_conn *conn)
 		}
 
 		cancel_delayed_work(&conn->disc_work);
-		queue_delayed_work(conn->hdev->workqueue,
-				   &conn->disc_work, timeo);
+		rcu_read_lock();
+		if (!hci_dev_test_flag(conn->hdev, HCI_CMD_DRAIN_WORKQUEUE))
+			queue_delayed_work(conn->hdev->workqueue,
+					   &conn->disc_work, timeo);
+		rcu_read_unlock();
 	}
 }
 
-- 
2.43.0

RE: Bluetooth: hci_core: Fix queuing disc_work while workqueue is drained

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2026 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/series/1189822/

---Test result---

Test Summary:
CheckPatch                    PASS      0.57 seconds
VerifyFixes                   PASS      0.07 seconds
VerifySignedoff               PASS      0.07 seconds
GitLint                       FAIL      0.22 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      28.55 seconds
CheckAllWarning               PASS      32.74 seconds
CheckSparse                   PASS      35.40 seconds
BuildKernel32                 PASS      28.75 seconds
CheckKernelLLVM               PASS      32.49 seconds
TestRunnerSetup               PASS      686.98 seconds
TestRunner_l2cap-tester       PASS      12.52 seconds
TestRunner_iso-tester         PASS      10.62 seconds
TestRunner_bnep-tester        PASS      3.08 seconds
TestRunner_mgmt-tester        PASS      88.22 seconds
TestRunner_rfcomm-tester      PASS      4.46 seconds
TestRunner_sco-tester         PASS      5.55 seconds
TestRunner_ioctl-tester       PASS      5.24 seconds
TestRunner_mesh-tester        PASS      5.46 seconds
TestRunner_smp-tester         PASS      4.27 seconds
TestRunner_userchan-tester    PASS      3.17 seconds
TestRunner_6lowpan-tester     PASS      4.34 seconds
IncrementalBuild              PASS      27.30 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: hci_core: Fix queuing disc_work while workqueue is drained

41: B2 Line has trailing whitespace: "    "
45: B2 Line has trailing whitespace: "    "
51: B2 Line has trailing whitespace: "    "
57: B2 Line has trailing whitespace: "    "
61: B2 Line has trailing whitespace: "    "
66: B2 Line has trailing whitespace: "    "


https://github.com/bluez/bluetooth-next/pull/969

---
Regards,
Linux Bluetooth