[PATCH RESEND] Bluetooth: hci_core: add lockdep check to hci_conn lookups
Pauli Virtanen <[email protected]>
| Newsgroups | org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <2be38d111362590f45776a0bc114f7890906ead9.1786875148.git.pav@iki.fi> |
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 <[email protected]> --- 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