[PATCH RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups

[PATCH RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups

[Pauli Virtanen]

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

Re: [PATCH RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups

[Pauli Virtanen]

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

Re: [PATCH RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Sun, 16 Aug 2026 13:26:21 +0300 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups
    https://git.kernel.org/bluetooth/bluetooth-next/c/f4fe51177b82

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html