* [PATCH RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups
@ 2026-08-16 10:26 Pauli Virtanen
2026-08-16 11:15 ` [RESEND] " bluez.test.bot
2026-08-16 11:19 ` [PATCH RESEND] " Pauli Virtanen
0 siblings, 2 replies; 3+ messages in thread
From: Pauli Virtanen @ 2026-08-16 10:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen, marcel, luiz.dentz, linux-kernel
Add lockdep check for RCU || hdev->lock in hci_conn_hash lookups that
return hci_conn pointer, as dereferencing that without locks can be
TOCTOU issue. It used to be several callsites did not hold appropriate
locks.
The check is equivalent to removing rcu_read_lock() and doing instead
list_for_each_entry_rcu(c, &h->list, list, lockdep_is_held(&hdev->lock))
Although there should not be any remaining callsites without locks,
don't remove the rcu_read_lock() for now, and just add the warning here.
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
Notes:
resend:
- no changes
include/net/bluetooth/hci_core.h | 44 ++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4105c446ca98..c12cd6873f65 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1030,6 +1030,9 @@ static inline bool hci_conn_sc_enabled(struct hci_conn *conn)
static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c)
{
struct hci_conn_hash *h = &hdev->conn_hash;
+
+ lockdep_assert_held(&hdev->lock);
+
list_add_tail_rcu(&c->list, &h->list);
switch (c->type) {
case ACL_LINK:
@@ -1060,6 +1063,8 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
{
struct hci_conn_hash *h = &hdev->conn_hash;
+ lockdep_assert_held(&hdev->lock);
+
list_del_rcu(&c->list);
synchronize_rcu();
@@ -1088,6 +1093,15 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
}
}
+#ifdef CONFIG_PROVE_RCU
+#define HCI_CONN_HASH_LOCKDEP_CHECK(hdev) \
+ RCU_LOCKDEP_WARN(!lockdep_is_held(&(hdev)->lock) && \
+ !rcu_read_lock_held(), \
+ "suspicious hci_conn locking")
+#else
+#define HCI_CONN_HASH_LOCKDEP_CHECK(hdev) do { } while (0 && (hdev))
+#endif
+
static inline unsigned int hci_conn_num(struct hci_dev *hdev, __u8 type)
{
struct hci_conn_hash *h = &hdev->conn_hash;
@@ -1169,6 +1183,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_bis(struct hci_dev *hdev,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1191,6 +1207,8 @@ hci_conn_hash_lookup_create_pa_sync(struct hci_dev *hdev)
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1217,6 +1235,8 @@ hci_conn_hash_lookup_per_adv_bis(struct hci_dev *hdev,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1241,6 +1261,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_handle(struct hci_dev *hdev,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1260,6 +1282,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_ba(struct hci_dev *hdev,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1281,6 +1305,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_role(struct hci_dev *hdev,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1302,6 +1328,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_le(struct hci_dev *hdev,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1328,6 +1356,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_cis(struct hci_dev *hdev,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1360,6 +1390,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_cig(struct hci_dev *hdev,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1383,6 +1415,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_big(struct hci_dev *hdev,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1407,6 +1441,8 @@ hci_conn_hash_lookup_big_sync_pend(struct hci_dev *hdev,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1431,6 +1467,8 @@ hci_conn_hash_lookup_big_state(struct hci_dev *hdev, __u8 handle, __u16 state,
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1454,6 +1492,8 @@ hci_conn_hash_lookup_pa_sync_big_handle(struct hci_dev *hdev, __u8 big)
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1477,6 +1517,8 @@ hci_conn_hash_lookup_pa_sync_handle(struct hci_dev *hdev, __u16 sync_handle)
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
@@ -1546,6 +1588,8 @@ static inline struct hci_conn *hci_lookup_le_connect(struct hci_dev *hdev)
struct hci_conn_hash *h = &hdev->conn_hash;
struct hci_conn *c;
+ HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
+
rcu_read_lock();
list_for_each_entry_rcu(c, &h->list, list) {
--
2.55.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* RE: [RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups
2026-08-16 10:26 [PATCH RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups Pauli Virtanen
@ 2026-08-16 11:15 ` bluez.test.bot
2026-08-16 11:19 ` [PATCH RESEND] " Pauli Virtanen
1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2026-08-16 11:15 UTC (permalink / raw)
To: linux-bluetooth, pav
[-- Attachment #1: Type: text/plain, Size: 2389 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/project/bluetooth/list/?series=1146718
---Test result---
Test Summary:
CheckPatch PASS 0.77 seconds
VerifyFixes PASS 0.13 seconds
VerifySignedoff PASS 0.13 seconds
GitLint PASS 0.32 seconds
SubjectPrefix PASS 0.27 seconds
BuildKernel PASS 27.43 seconds
CheckAllWarning PASS 30.53 seconds
CheckSparse PASS 29.28 seconds
BuildKernel32 PASS 26.86 seconds
CheckKernelLLVM SKIP 0.00 seconds
TestRunnerSetup PASS 508.42 seconds
TestRunner_l2cap-tester PASS 66.05 seconds
TestRunner_iso-tester PASS 83.58 seconds
TestRunner_bnep-tester PASS 19.20 seconds
TestRunner_mgmt-tester FAIL 225.44 seconds
TestRunner_rfcomm-tester PASS 26.55 seconds
TestRunner_sco-tester PASS 32.08 seconds
TestRunner_ioctl-tester PASS 26.68 seconds
TestRunner_mesh-tester FAIL 25.95 seconds
TestRunner_smp-tester PASS 23.70 seconds
TestRunner_userchan-tester PASS 20.34 seconds
TestRunner_6lowpan-tester PASS 24.08 seconds
IncrementalBuild PASS 25.72 seconds
Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 501, Passed: 496 (99.0%), Failed: 1, Not Run: 4
Failed Test Cases
Read Exp Feature - Success Failed 0.252 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0
Failed Test Cases
Mesh - Send cancel - 1 Timed out 1.920 seconds
Mesh - Send cancel - 2 Timed out 1.981 seconds
https://github.com/bluez/bluetooth-next/pull/593
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups
2026-08-16 10:26 [PATCH RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups Pauli Virtanen
2026-08-16 11:15 ` [RESEND] " bluez.test.bot
@ 2026-08-16 11:19 ` Pauli Virtanen
1 sibling, 0 replies; 3+ messages in thread
From: Pauli Virtanen @ 2026-08-16 11:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, luiz.dentz, linux-kernel
su, 2026-08-16 kello 13:26 +0300, Pauli Virtanen kirjoitti:
> Add lockdep check for RCU || hdev->lock in hci_conn_hash lookups that
> return hci_conn pointer, as dereferencing that without locks can be
> TOCTOU issue. It used to be several callsites did not hold appropriate
> locks.
https://sashiko.dev/#/patchset/2be38d111362590f45776a0bc114f7890906ead9.1786875148.git.pav%40iki.fi
Sashiko review complains about HCI_CONN_HASH_LOCKDEP_CHECK() added to
wrong functions.
It applied the patch to bluetooth/master, instead of bluetooth-
next/master as intended.
Apparently the patch applies with fuzz also to bluetooth/master, but
some of the added HCI_CONN_HASH_LOCKDEP_CHECK land in wrong functions
there.
> The check is equivalent to removing rcu_read_lock() and doing instead
> list_for_each_entry_rcu(c, &h->list, list, lockdep_is_held(&hdev->lock))
> Although there should not be any remaining callsites without locks,
> don't remove the rcu_read_lock() for now, and just add the warning here.
>
> Signed-off-by: Pauli Virtanen <pav@iki.fi>
> ---
>
> Notes:
> resend:
> - no changes
>
> include/net/bluetooth/hci_core.h | 44 ++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 4105c446ca98..c12cd6873f65 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1030,6 +1030,9 @@ static inline bool hci_conn_sc_enabled(struct hci_conn *conn)
> static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c)
> {
> struct hci_conn_hash *h = &hdev->conn_hash;
> +
> + lockdep_assert_held(&hdev->lock);
> +
> list_add_tail_rcu(&c->list, &h->list);
> switch (c->type) {
> case ACL_LINK:
> @@ -1060,6 +1063,8 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
> {
> struct hci_conn_hash *h = &hdev->conn_hash;
>
> + lockdep_assert_held(&hdev->lock);
> +
> list_del_rcu(&c->list);
> synchronize_rcu();
>
> @@ -1088,6 +1093,15 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
> }
> }
>
> +#ifdef CONFIG_PROVE_RCU
> +#define HCI_CONN_HASH_LOCKDEP_CHECK(hdev) \
> + RCU_LOCKDEP_WARN(!lockdep_is_held(&(hdev)->lock) && \
> + !rcu_read_lock_held(), \
> + "suspicious hci_conn locking")
> +#else
> +#define HCI_CONN_HASH_LOCKDEP_CHECK(hdev) do { } while (0 && (hdev))
> +#endif
> +
> static inline unsigned int hci_conn_num(struct hci_dev *hdev, __u8 type)
> {
> struct hci_conn_hash *h = &hdev->conn_hash;
> @@ -1169,6 +1183,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_bis(struct hci_dev *hdev,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1191,6 +1207,8 @@ hci_conn_hash_lookup_create_pa_sync(struct hci_dev *hdev)
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1217,6 +1235,8 @@ hci_conn_hash_lookup_per_adv_bis(struct hci_dev *hdev,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1241,6 +1261,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_handle(struct hci_dev *hdev,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1260,6 +1282,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_ba(struct hci_dev *hdev,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1281,6 +1305,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_role(struct hci_dev *hdev,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1302,6 +1328,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_le(struct hci_dev *hdev,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1328,6 +1356,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_cis(struct hci_dev *hdev,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1360,6 +1390,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_cig(struct hci_dev *hdev,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1383,6 +1415,8 @@ static inline struct hci_conn *hci_conn_hash_lookup_big(struct hci_dev *hdev,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1407,6 +1441,8 @@ hci_conn_hash_lookup_big_sync_pend(struct hci_dev *hdev,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1431,6 +1467,8 @@ hci_conn_hash_lookup_big_state(struct hci_dev *hdev, __u8 handle, __u16 state,
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1454,6 +1492,8 @@ hci_conn_hash_lookup_pa_sync_big_handle(struct hci_dev *hdev, __u8 big)
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1477,6 +1517,8 @@ hci_conn_hash_lookup_pa_sync_handle(struct hci_dev *hdev, __u16 sync_handle)
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
> @@ -1546,6 +1588,8 @@ static inline struct hci_conn *hci_lookup_le_connect(struct hci_dev *hdev)
> struct hci_conn_hash *h = &hdev->conn_hash;
> struct hci_conn *c;
>
> + HCI_CONN_HASH_LOCKDEP_CHECK(hdev);
> +
> rcu_read_lock();
>
> list_for_each_entry_rcu(c, &h->list, list) {
--
Pauli Virtanen
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-16 11:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-16 10:26 [PATCH RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups Pauli Virtanen
2026-08-16 11:15 ` [RESEND] " bluez.test.bot
2026-08-16 11:19 ` [PATCH RESEND] " Pauli Virtanen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox