[PATCH] Bluetooth: hci_core: Hold hdev->lock when freeing an adv monitor

[PATCH] Bluetooth: hci_core: Hold hdev->lock when freeing an adv monitor

[Praneeth Balasubramanian]

hci_free_adv_monitor() removes a monitor from hdev->adv_monitors_idr and
frees it, and its comment says the caller must hold hdev->lock. Readers
such as read_adv_mon_features() walk adv_monitors_idr and dereference
each monitor under hdev->lock.

Since commit 7cf5c2978f23 ("Bluetooth: hci_sync: Refactor remove Adv
Monitor"), MGMT_OP_REMOVE_ADV_MONITOR runs from the cmd_sync work, which
holds only hci_req_sync_lock. On the HCI_ADV_MONITOR_EXT_NONE path, which
is also taken while the controller is powered off,
hci_remove_adv_monitor() then calls hci_free_adv_monitor() without
hdev->lock. A concurrent MGMT_OP_READ_ADV_MONITOR_FEATURES can load a
monitor pointer from the IDR and read monitor->handle after the monitor
has been freed.

The MSFT path already frees the monitor under hdev->lock, in
msft_le_cancel_monitor_advertisement_cb(). Take hdev->lock around
hci_free_adv_monitor() in hci_remove_adv_monitor() too. None of its
callers hold hdev->lock.

The missing lock was found with an LLM-based tool that checks callers
against the locking rules stated in kernel comments.

Fixes: 7cf5c2978f23 ("Bluetooth: hci_sync: Refactor remove Adv Monitor")
Cc: stable@vger.kernel.org
Signed-off-by: Praneeth Balasubramanian <praneeth@ucsb.edu>
---

Notes:
    This was found by an LLM-based static analysis (GPT-5.6) that checks
    kernel code against the rules stated in its comments, as part of research
    at UCSB SecLab. The patch and changelog were drafted with Claude Opus 5.5.
    I reviewed the patch and take responsibility for it.
    
    MGMT commands need CAP_NET_ADMIN in the initial user namespace, so I am
    sending this as a regular fix rather than a security report.
    
    Testing: builds without new warnings with W=1 (x86_64, KASAN and lockdep
    config). Not runtime-tested.
    
    The patch applies to mainline 9ba46df34488 and to next-20261109.

 net/bluetooth/hci_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 74d9865e08c2..b66d3c295e5e 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1966,7 +1966,10 @@ static int hci_remove_adv_monitor(struct hci_dev *hdev,
 	if (status == -ENOENT)
 		bt_dev_warn(hdev, "Removing monitor with no matching handle %d",
 			    monitor->handle);
+
+	hci_dev_lock(hdev);
 	hci_free_adv_monitor(hdev, monitor);
+	hci_dev_unlock(hdev);
 
 	return status;
 }
-- 
2.25.1

RE: Bluetooth: hci_core: Hold hdev->lock when freeing an adv monitor

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2712 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/1207695/

---Test result---

Test Summary:
CheckPatch                    FAIL      0.78 seconds
VerifyFixes                   PASS      0.13 seconds
VerifySignedoff               PASS      0.13 seconds
GitLint                       FAIL      0.35 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      31.24 seconds
CheckAllWarning               PASS      35.23 seconds
CheckSparse                   PASS      37.42 seconds
BuildKernel32                 PASS      31.14 seconds
CheckKernelLLVM               PASS      33.13 seconds
TestRunnerSetup               PASS      745.20 seconds
TestRunner_l2cap-tester       PASS      12.77 seconds
TestRunner_iso-tester         PASS      10.84 seconds
TestRunner_bnep-tester        PASS      3.22 seconds
TestRunner_mgmt-tester        PASS      88.50 seconds
TestRunner_rfcomm-tester      PASS      4.57 seconds
TestRunner_sco-tester         PASS      5.75 seconds
TestRunner_ioctl-tester       PASS      4.78 seconds
TestRunner_mesh-tester        PASS      5.62 seconds
TestRunner_smp-tester         PASS      4.44 seconds
TestRunner_userchan-tester    PASS      3.29 seconds
TestRunner_6lowpan-tester     PASS      4.54 seconds
IncrementalBuild              PASS      29.55 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: hci_core: Hold hdev->lock when freeing an adv monitor
WARNING: From:/Signed-off-by: email address mismatch: 'From: Praneeth Balasubramanian <praneethb2002@gmail.com>' != 'Signed-off-by: Praneeth Balasubramanian <praneeth@ucsb.edu>'

total: 0 errors, 1 warnings, 0 checks, 10 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/home/runner/work/bluetooth-next/bluetooth-next/src/patch/14970524.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: hci_core: Hold hdev->lock when freeing an adv monitor

34: B2 Line has trailing whitespace: "    "
37: B2 Line has trailing whitespace: "    "
40: B2 Line has trailing whitespace: "    "


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

---
Regards,
Linux Bluetooth