[RFC PATCH 00/24] Bluetooth: add locks to hci_conn accesses

[RFC PATCH 00/24] Bluetooth: add locks to hci_conn accesses

[Pauli Virtanen]

(RFC since this needs to be tested much better.)

Each hdev has two ordered workqueues that run in parallel, in addition
to user tasks and some timers in global workqueues.

Both workqueues may delete hci_conn* and modify their state. The current
situation is there are races and UAF due to this.  In older kernels, it
used to be much of the work was done from a single ordered
hdev->workqueue, so one could be more lax with locking.  I don't think
what used to be safe earlier is necessarily so now, so some simple rules
are probably needed.

Set some rules for hci_conn* locking and try follow them:

- lookups: hdev->lock or rcu_read_lock() shall be held by caller

- field access: hdev->lock shall be held, unless lockless operation is
  explained in comments, in which case rcu_read_lock() is enough

- hci_conn pointers remain valid after exiting critical section only if
  hci_conn_get() refcount remains held

- before field access, if hci_conn* was sustained only by refcount,
  hci_conn_valid() shall be checked before dereferencing

***

Add lockdep asserts to lookup functions and hci_conn_valid() to catch
some bad callsites.

In hci_sync, the critical sections cannot extend across HCI event waits.
There, add helpers hci_dev_lock/unlock_sync(hdev) that release/acquire
hdev->lock before/after the wait.

Following the rules above then means checking hci_conn* validity after
each call to a waiting subroutine.

This series also contains some fixes to ABA issues: if hci_conn pointer
is used across critical sections, one should hold a refcount, and not
use hci_conn_valid() on potentially wild pointer even though it doesn't
dereference.

Pauli Virtanen (24):
  Bluetooth: ISO: free rx_skb if not consumed
  Bluetooth: ISO: don't leak skb in ISO_CONT RX
  Bluetooth: hci_sync: make hci_cmd_sync_run* indicate if item was added
  Bluetooth: hci_sync: hci_cmd_sync_queue_once() return -EEXIST if
    exists
  Bluetooth: hci_conn: avoid ABA error in abort_conn_sync
  Bluetooth: hci_sync: avoid ABA/UAF in hci_sync callbacks
  Bluetooth: hci_event: extend conn_hash lookup RCU critical sections
  Bluetooth: hci_sync: extend conn_hash lookup RCU critical sections
  Bluetooth: mgmt: extend conn_hash lookup RCU critical sections
  Bluetooth: hci_conn: extend conn_hash lookup RCU critical sections
  Bluetooth: hci_core: add lockdep check to hci_conn_hash lookups
  Bluetooth: hci_core: add lockdep check to hci_conn_valid()
  Bluetooth: hci_sync: fix hdev locking in hci_le_create_conn_sync
  Bluetooth: hci_core: hold hdev lock in packet TX scheduler
  Bluetooth: lookup hci_conn on RX path on protocol side
  Bluetooth: L2CAP: fix hci_conn_valid() usage
  Bluetooth: hci_sync: add helper for hdev locking across waits
  Bluetooth: hci_sync: hold lock in hci_acl_create_conn_sync
  Bluetooth: hci_sync: hold lock in hci_le_create_conn_sync
  Bluetooth: hci_sync: add hdev lock lockdep asserts in subroutines
  Bluetooth: fix locking for hci_abort_conn_sync()
  Bluetooth: hci_sync: lock properly in hci_le_pa/big_create_sync
  Bluetooth: hci_sync: fix locking in hci_disconnect_sync
  Bluetooth: hci_conn: fix ABA and locking in hci_enhanced_setup_sync

 include/net/bluetooth/hci_core.h |  66 ++++++-
 include/net/bluetooth/hci_sync.h |   4 +
 net/bluetooth/hci_conn.c         |  83 ++++++--
 net/bluetooth/hci_core.c         | 114 +++++------
 net/bluetooth/hci_event.c        |  33 ++--
 net/bluetooth/hci_sync.c         | 315 ++++++++++++++++++++++++-------
 net/bluetooth/iso.c              |  34 +++-
 net/bluetooth/l2cap_core.c       |  28 ++-
 net/bluetooth/mgmt.c             |  38 +++-
 net/bluetooth/sco.c              |  35 +++-
 10 files changed, 551 insertions(+), 199 deletions(-)

-- 
2.51.0

[RFC PATCH 01/24] Bluetooth: ISO: free rx_skb if not consumed

[Pauli Virtanen]

If iso_conn is freed when RX is incomplete, free any leftover skb piece.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/iso.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 5c68c0ea7d97..d63d4d6f874c 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -111,6 +111,8 @@ static void iso_conn_free(struct kref *ref)
 	/* Ensure no more work items will run since hci_conn has been dropped */
 	disable_delayed_work_sync(&conn->timeout_work);
 
+	kfree_skb(conn->rx_skb);
+
 	kfree(conn);
 }
 
-- 
2.51.0

[RFC PATCH 02/24] Bluetooth: ISO: don't leak skb in ISO_CONT RX

[Pauli Virtanen]

For ISO_CONT RX, the data from skb is copied to conn->rx_skb, but the
skb is leaked.

Free skb after copying its data.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/iso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index d63d4d6f874c..f5a9a13317df 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -2420,7 +2420,7 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
 		skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb->len),
 					  skb->len);
 		conn->rx_len -= skb->len;
-		return;
+		break;
 
 	case ISO_END:
 		skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb->len),
-- 
2.51.0

[RFC PATCH 03/24] Bluetooth: hci_sync: make hci_cmd_sync_run* indicate if item was added

[Pauli Virtanen]

hci_cmd_sync_run_once() needs to indicate whether a queue item was
added, so caller can know if callbacks will be called.  Change the
function to return -EEXIST if queue item already exists.

hci_cmd_sync_run() may run the work immediately. If so, it shall behave
as if item was queued successfully. Change it to call also destroy() and
return 0.

Modify all callsites vs. the changes.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_conn.c |  4 +++-
 net/bluetooth/hci_sync.c | 13 ++++++++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 111f0e37b672..4a9067b2e87f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -2930,6 +2930,7 @@ static int abort_conn_sync(struct hci_dev *hdev, void *data)
 int hci_abort_conn(struct hci_conn *conn, u8 reason)
 {
 	struct hci_dev *hdev = conn->hdev;
+	int err;
 
 	/* If abort_reason has already been set it means the connection is
 	 * already being aborted so don't attempt to overwrite it.
@@ -2966,7 +2967,8 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
 	 * as a result to MGMT_OP_DISCONNECT/MGMT_OP_UNPAIR which does
 	 * already queue its callback on cmd_sync_work.
 	 */
-	return hci_cmd_sync_run_once(hdev, abort_conn_sync, conn, NULL);
+	err = hci_cmd_sync_run_once(hdev, abort_conn_sync, conn, NULL);
+	return (err == -EEXIST) ? 0 : err;
 }
 
 void hci_setup_tx_timestamp(struct sk_buff *skb, size_t key_offset,
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 7a7d49890858..78af430be381 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -801,8 +801,15 @@ int hci_cmd_sync_run(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
 		return -ENETDOWN;
 
 	/* If on cmd_sync_work then run immediately otherwise queue */
-	if (current_work() == &hdev->cmd_sync_work)
-		return func(hdev, data);
+	if (current_work() == &hdev->cmd_sync_work) {
+		int err;
+
+		err = func(hdev, data);
+		if (destroy)
+			destroy(hdev, data, err);
+
+		return 0;
+	}
 
 	return hci_cmd_sync_submit(hdev, func, data, destroy);
 }
@@ -818,7 +825,7 @@ int hci_cmd_sync_run_once(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
 			  void *data, hci_cmd_sync_work_destroy_t destroy)
 {
 	if (hci_cmd_sync_lookup_entry(hdev, func, data, destroy))
-		return 0;
+		return -EEXIST;
 
 	return hci_cmd_sync_run(hdev, func, data, destroy);
 }
-- 
2.51.0

[RFC PATCH 04/24] Bluetooth: hci_sync: hci_cmd_sync_queue_once() return -EEXIST if exists

[Pauli Virtanen]

hci_cmd_sync_queue_once() needs to indicate whether a queue item was
added, so caller can know if callbacks will be called.

Change the function to return -EEXIST if queue item already exists, and
modify all callsites to handle that.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_sync.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 78af430be381..64e26fbf1543 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -780,7 +780,7 @@ int hci_cmd_sync_queue_once(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
 			    void *data, hci_cmd_sync_work_destroy_t destroy)
 {
 	if (hci_cmd_sync_lookup_entry(hdev, func, data, destroy))
-		return 0;
+		return -EEXIST;
 
 	return hci_cmd_sync_queue(hdev, func, data, destroy);
 }
@@ -3255,6 +3255,8 @@ static int update_passive_scan_sync(struct hci_dev *hdev, void *data)
 
 int hci_update_passive_scan(struct hci_dev *hdev)
 {
+	int err;
+
 	/* Only queue if it would have any effect */
 	if (!test_bit(HCI_UP, &hdev->flags) ||
 	    test_bit(HCI_INIT, &hdev->flags) ||
@@ -3264,8 +3266,9 @@ int hci_update_passive_scan(struct hci_dev *hdev)
 	    hci_dev_test_flag(hdev, HCI_UNREGISTER))
 		return 0;
 
-	return hci_cmd_sync_queue_once(hdev, update_passive_scan_sync, NULL,
-				       NULL);
+	err = hci_cmd_sync_queue_once(hdev, update_passive_scan_sync, NULL,
+				      NULL);
+	return (err == -EEXIST) ? 0 : err;
 }
 
 int hci_write_sc_support_sync(struct hci_dev *hdev, u8 val)
@@ -6910,8 +6913,11 @@ static int hci_acl_create_conn_sync(struct hci_dev *hdev, void *data)
 
 int hci_connect_acl_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
-	return hci_cmd_sync_queue_once(hdev, hci_acl_create_conn_sync, conn,
-				       NULL);
+	int err;
+
+	err = hci_cmd_sync_queue_once(hdev, hci_acl_create_conn_sync, conn,
+				      NULL);
+	return (err == -EEXIST) ? 0 : err;
 }
 
 static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
@@ -6947,8 +6953,11 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
 
 int hci_connect_le_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
-	return hci_cmd_sync_queue_once(hdev, hci_le_create_conn_sync, conn,
-				       create_le_conn_complete);
+	int err;
+
+	err = hci_cmd_sync_queue_once(hdev, hci_le_create_conn_sync, conn,
+				      create_le_conn_complete);
+	return (err == -EEXIST) ? 0 : err;
 }
 
 int hci_cancel_connect_sync(struct hci_dev *hdev, struct hci_conn *conn)
@@ -7106,8 +7115,11 @@ static int hci_le_pa_create_sync(struct hci_dev *hdev, void *data)
 
 int hci_connect_pa_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
-	return hci_cmd_sync_queue_once(hdev, hci_le_pa_create_sync, conn,
-				       create_pa_complete);
+	int err;
+
+	err = hci_cmd_sync_queue_once(hdev, hci_le_pa_create_sync, conn,
+				      create_pa_complete);
+	return (err == -EEXIST) ? 0 : err;
 }
 
 static void create_big_complete(struct hci_dev *hdev, void *data, int err)
@@ -7169,6 +7181,9 @@ static int hci_le_big_create_sync(struct hci_dev *hdev, void *data)
 
 int hci_connect_big_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
-	return hci_cmd_sync_queue_once(hdev, hci_le_big_create_sync, conn,
-				       create_big_complete);
+	int err;
+
+	err = hci_cmd_sync_queue_once(hdev, hci_le_big_create_sync, conn,
+				      create_big_complete);
+	return (err == -EEXIST) ? 0 : err;
 }
-- 
2.51.0

[RFC PATCH 05/24] Bluetooth: hci_conn: avoid ABA error in abort_conn_sync

[Pauli Virtanen]

hci_conn_valid() shall not be used on potentially freed conns, otherwise
that is ABA problem. Avoid that by holding reference.

Fixes: 881559af5f5c5 ("Bluetooth: hci_sync: Attempt to dequeue connection attempt")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_conn.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 4a9067b2e87f..1d914d95cb6d 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -2927,6 +2927,13 @@ static int abort_conn_sync(struct hci_dev *hdev, void *data)
 	return hci_abort_conn_sync(hdev, conn, conn->abort_reason);
 }
 
+static void abort_conn_destroy(struct hci_dev *hdev, void *data, int err)
+{
+	struct hci_conn *conn = data;
+
+	hci_conn_put(conn);
+}
+
 int hci_abort_conn(struct hci_conn *conn, u8 reason)
 {
 	struct hci_dev *hdev = conn->hdev;
@@ -2967,7 +2974,10 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
 	 * as a result to MGMT_OP_DISCONNECT/MGMT_OP_UNPAIR which does
 	 * already queue its callback on cmd_sync_work.
 	 */
-	err = hci_cmd_sync_run_once(hdev, abort_conn_sync, conn, NULL);
+	err = hci_cmd_sync_run_once(hdev, abort_conn_sync, hci_conn_get(conn),
+				    abort_conn_destroy);
+	if (err)
+		hci_conn_put(conn);
 	return (err == -EEXIST) ? 0 : err;
 }
 
-- 
2.51.0

[RFC PATCH 06/24] Bluetooth: hci_sync: avoid ABA/UAF in hci_sync callbacks

[Pauli Virtanen]

Callbacks that take hci_conn* in their data shall hold refcount,
otherwise the conn may be already freed.

Fixes ABA error in hci_conn_valid() and possible UAF in callbacks that
run without hdev->lock.

Fix one instance of hci_conn_valid() called without lock, which is
invalid.

Fixes: 881559af5f5c5 ("Bluetooth: hci_sync: Attempt to dequeue connection attempt")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_sync.c | 55 +++++++++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 64e26fbf1543..ee2d41eead17 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -6911,12 +6911,23 @@ static int hci_acl_create_conn_sync(struct hci_dev *hdev, void *data)
 					conn->conn_timeout, NULL);
 }
 
+static void hci_acl_create_conn_sync_complete(struct hci_dev *hdev, void *data,
+					      int err)
+{
+	struct hci_conn *conn = data;
+
+	hci_conn_put(conn);
+}
+
 int hci_connect_acl_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
 	int err;
 
-	err = hci_cmd_sync_queue_once(hdev, hci_acl_create_conn_sync, conn,
-				      NULL);
+	err = hci_cmd_sync_queue_once(hdev, hci_acl_create_conn_sync,
+				      hci_conn_get(conn),
+				      hci_acl_create_conn_sync_complete);
+	if (err)
+		hci_conn_put(conn);
 	return (err == -EEXIST) ? 0 : err;
 }
 
@@ -6927,36 +6938,41 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
 	bt_dev_dbg(hdev, "err %d", err);
 
 	if (err == -ECANCELED)
-		return;
+		goto done;
 
 	hci_dev_lock(hdev);
 
 	if (!hci_conn_valid(hdev, conn))
-		goto done;
+		goto unlock;
 
 	if (!err) {
 		hci_connect_le_scan_cleanup(conn, 0x00);
-		goto done;
+		goto unlock;
 	}
 
 	/* Check if connection is still pending */
 	if (conn != hci_lookup_le_connect(hdev))
-		goto done;
+		goto unlock;
 
 	/* Flush to make sure we send create conn cancel command if needed */
 	flush_delayed_work(&conn->le_conn_timeout);
 	hci_conn_failed(conn, bt_status(err));
 
-done:
+unlock:
 	hci_dev_unlock(hdev);
+done:
+	hci_conn_put(conn);
 }
 
 int hci_connect_le_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
 	int err;
 
-	err = hci_cmd_sync_queue_once(hdev, hci_le_create_conn_sync, conn,
+	err = hci_cmd_sync_queue_once(hdev, hci_le_create_conn_sync,
+				      hci_conn_get(conn),
 				      create_le_conn_complete);
+	if (err)
+		hci_conn_put(conn);
 	return (err == -EEXIST) ? 0 : err;
 }
 
@@ -7004,7 +7020,7 @@ static void create_pa_complete(struct hci_dev *hdev, void *data, int err)
 	bt_dev_dbg(hdev, "err %d", err);
 
 	if (err == -ECANCELED)
-		return;
+		goto done;
 
 	hci_dev_lock(hdev);
 
@@ -7028,6 +7044,8 @@ static void create_pa_complete(struct hci_dev *hdev, void *data, int err)
 
 unlock:
 	hci_dev_unlock(hdev);
+done:
+	hci_conn_put(conn);
 }
 
 static int hci_le_pa_create_sync(struct hci_dev *hdev, void *data)
@@ -7117,8 +7135,11 @@ int hci_connect_pa_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
 	int err;
 
-	err = hci_cmd_sync_queue_once(hdev, hci_le_pa_create_sync, conn,
+	err = hci_cmd_sync_queue_once(hdev, hci_le_pa_create_sync,
+				      hci_conn_get(conn),
 				      create_pa_complete);
+	if (err)
+		hci_conn_put(conn);
 	return (err == -EEXIST) ? 0 : err;
 }
 
@@ -7129,10 +7150,17 @@ static void create_big_complete(struct hci_dev *hdev, void *data, int err)
 	bt_dev_dbg(hdev, "err %d", err);
 
 	if (err == -ECANCELED)
-		return;
+		goto done;
+
+	hci_dev_lock(hdev);
 
 	if (hci_conn_valid(hdev, conn))
 		clear_bit(HCI_CONN_CREATE_BIG_SYNC, &conn->flags);
+
+	hci_dev_unlock(hdev);
+
+done:
+	hci_conn_put(conn);
 }
 
 static int hci_le_big_create_sync(struct hci_dev *hdev, void *data)
@@ -7183,7 +7211,10 @@ int hci_connect_big_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
 	int err;
 
-	err = hci_cmd_sync_queue_once(hdev, hci_le_big_create_sync, conn,
+	err = hci_cmd_sync_queue_once(hdev, hci_le_big_create_sync,
+				      hci_conn_get(conn),
 				      create_big_complete);
+	if (err)
+		hci_conn_put(conn);
 	return (err == -EEXIST) ? 0 : err;
 }
-- 
2.51.0

[RFC PATCH 07/24] Bluetooth: hci_event: extend conn_hash lookup RCU critical sections

[Pauli Virtanen]

When doing hci_conn_hash lookups with only RCU, extend critical section
to cover also the handling of the return value.

This avoids any concurrent hci_conn_del from deleting the conn before we
are done dereferencing it.

Fixes: 95118dd4edfec ("Bluetooth: hci_event: Use of a function table to handle LE subevents")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_event.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d790b0d4eb9a..56f3f69404c6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -6616,25 +6616,31 @@ static void hci_le_remote_conn_param_req_evt(struct hci_dev *hdev, void *data,
 	latency = le16_to_cpu(ev->latency);
 	timeout = le16_to_cpu(ev->timeout);
 
+	hci_dev_lock(hdev);
+
 	hcon = hci_conn_hash_lookup_handle(hdev, handle);
-	if (!hcon || hcon->state != BT_CONNECTED)
-		return send_conn_param_neg_reply(hdev, handle,
-						 HCI_ERROR_UNKNOWN_CONN_ID);
+	if (!hcon || hcon->state != BT_CONNECTED) {
+		send_conn_param_neg_reply(hdev, handle,
+					  HCI_ERROR_UNKNOWN_CONN_ID);
+		goto unlock;
+	}
 
-	if (max > hcon->le_conn_max_interval)
-		return send_conn_param_neg_reply(hdev, handle,
-						 HCI_ERROR_INVALID_LL_PARAMS);
+	if (max > hcon->le_conn_max_interval) {
+		send_conn_param_neg_reply(hdev, handle,
+					  HCI_ERROR_INVALID_LL_PARAMS);
+		goto unlock;
+	}
 
-	if (hci_check_conn_params(min, max, latency, timeout))
-		return send_conn_param_neg_reply(hdev, handle,
-						 HCI_ERROR_INVALID_LL_PARAMS);
+	if (hci_check_conn_params(min, max, latency, timeout)) {
+		send_conn_param_neg_reply(hdev, handle,
+					  HCI_ERROR_INVALID_LL_PARAMS);
+		goto unlock;
+	}
 
 	if (hcon->role == HCI_ROLE_MASTER) {
 		struct hci_conn_params *params;
 		u8 store_hint;
 
-		hci_dev_lock(hdev);
-
 		params = hci_conn_params_lookup(hdev, &hcon->dst,
 						hcon->dst_type);
 		if (params) {
@@ -6647,8 +6653,6 @@ static void hci_le_remote_conn_param_req_evt(struct hci_dev *hdev, void *data,
 			store_hint = 0x00;
 		}
 
-		hci_dev_unlock(hdev);
-
 		mgmt_new_conn_param(hdev, &hcon->dst, hcon->dst_type,
 				    store_hint, min, max, latency, timeout);
 	}
@@ -6662,6 +6666,9 @@ static void hci_le_remote_conn_param_req_evt(struct hci_dev *hdev, void *data,
 	cp.max_ce_len = 0;
 
 	hci_send_cmd(hdev, HCI_OP_LE_CONN_PARAM_REQ_REPLY, sizeof(cp), &cp);
+
+unlock:
+	hci_dev_unlock(hdev);
 }
 
 static void hci_le_direct_adv_report_evt(struct hci_dev *hdev, void *data,
-- 
2.51.0

[RFC PATCH 08/24] Bluetooth: hci_sync: extend conn_hash lookup RCU critical sections

[Pauli Virtanen]

When doing hci_conn_hash lookups with only RCU, extend critical section
to cover also the handling of the return value.

This avoids any concurrent hci_conn_del from deleting the conn before we
are done dereferencing it.

Fixes: 58ddd115fe063 ("Bluetooth: hci_conn: Fix not setting conn_timeout for Broadcast Receiver")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_sync.c | 59 +++++++++++++++++++++++++++++++++-------
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index ee2d41eead17..408fe5e08c30 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -1036,6 +1036,20 @@ static bool adv_use_rpa(struct hci_dev *hdev, uint32_t flags)
 	return true;
 }
 
+static bool hci_check_le_connect(struct hci_dev *hdev)
+{
+	bool found;
+
+	rcu_read_lock();
+	found = hci_lookup_le_connect(hdev);
+	rcu_read_unlock();
+
+	/* The return value may be wrong under race conditions, as the
+	 * conn may be already deleted
+	 */
+	return found;
+}
+
 static int hci_set_random_addr_sync(struct hci_dev *hdev, bdaddr_t *rpa)
 {
 	/* If a random_addr has been set we're advertising or initiating an LE
@@ -1050,7 +1064,7 @@ static int hci_set_random_addr_sync(struct hci_dev *hdev, bdaddr_t *rpa)
 	 */
 	if (bacmp(&hdev->random_addr, BDADDR_ANY) &&
 	    (hci_dev_test_flag(hdev, HCI_LE_ADV) ||
-	    hci_lookup_le_connect(hdev))) {
+	    hci_check_le_connect(hdev))) {
 		bt_dev_dbg(hdev, "Deferring random address update");
 		hci_dev_set_flag(hdev, HCI_RPA_EXPIRED);
 		return 0;
@@ -2636,7 +2650,7 @@ static int hci_pause_addr_resolution(struct hci_dev *hdev)
 	 * when initiating an LE connection.
 	 */
 	if (hci_dev_test_flag(hdev, HCI_LE_SCAN) ||
-	    hci_lookup_le_connect(hdev)) {
+	    hci_check_le_connect(hdev)) {
 		bt_dev_err(hdev, "Command not allowed when scan/LE connect");
 		return -EPERM;
 	}
@@ -2778,6 +2792,8 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
 	if (hci_dev_test_flag(hdev, HCI_PA_SYNC)) {
 		struct hci_conn *conn;
 
+		rcu_read_lock();
+
 		conn = hci_conn_hash_lookup_create_pa_sync(hdev);
 		if (conn) {
 			struct conn_params pa;
@@ -2787,6 +2803,8 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
 			bacpy(&pa.addr, &conn->dst);
 			pa.addr_type = conn->dst_type;
 
+			rcu_read_unlock();
+
 			/* Clear first since there could be addresses left
 			 * behind.
 			 */
@@ -2796,6 +2814,8 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
 			err = hci_le_add_accept_list_sync(hdev, &pa,
 							  &num_entries);
 			goto done;
+		} else {
+			rcu_read_unlock();
 		}
 	}
 
@@ -2806,10 +2826,13 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
 	 * the controller.
 	 */
 	list_for_each_entry_safe(b, t, &hdev->le_accept_list, list) {
-		if (hci_conn_hash_lookup_le(hdev, &b->bdaddr, b->bdaddr_type))
-			continue;
+		rcu_read_lock();
+
+		if (hci_conn_hash_lookup_le(hdev, &b->bdaddr, b->bdaddr_type)) {
+			rcu_read_unlock();
+			continue;
+		}
 
-		/* Pointers not dereferenced, no locks needed */
 		pend_conn = hci_pend_le_action_lookup(&hdev->pend_le_conns,
 						      &b->bdaddr,
 						      b->bdaddr_type);
@@ -2817,6 +2840,8 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
 							&b->bdaddr,
 							b->bdaddr_type);
 
+		rcu_read_unlock();
+
 		/* If the device is not likely to connect or report,
 		 * remove it from the acceptlist.
 		 */
@@ -2943,6 +2968,8 @@ static int hci_le_set_ext_scan_param_sync(struct hci_dev *hdev, u8 type,
 		if (sent) {
 			struct hci_conn *conn;
 
+			rcu_read_lock();
+
 			conn = hci_conn_hash_lookup_ba(hdev, PA_LINK,
 						       &sent->bdaddr);
 			if (conn) {
@@ -2967,8 +2994,12 @@ static int hci_le_set_ext_scan_param_sync(struct hci_dev *hdev, u8 type,
 					phy++;
 				}
 
+				rcu_read_unlock();
+
 				if (num_phy)
 					goto done;
+			} else {
+				rcu_read_unlock();
 			}
 		}
 	}
@@ -3224,7 +3255,7 @@ int hci_update_passive_scan_sync(struct hci_dev *hdev)
 		 * since some controllers are not able to scan and connect at
 		 * the same time.
 		 */
-		if (hci_lookup_le_connect(hdev))
+		if (hci_check_le_connect(hdev))
 			return 0;
 
 		bt_dev_dbg(hdev, "start background scanning");
@@ -3448,12 +3479,20 @@ static bool disconnected_accept_list_entries(struct hci_dev *hdev)
 	list_for_each_entry(b, &hdev->accept_list, list) {
 		struct hci_conn *conn;
 
-		conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &b->bdaddr);
-		if (!conn)
-			return true;
+		rcu_read_lock();
 
-		if (conn->state != BT_CONNECTED && conn->state != BT_CONFIG)
+		conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &b->bdaddr);
+		if (!conn) {
+			rcu_read_unlock();
 			return true;
+		}
+
+		if (conn->state != BT_CONNECTED && conn->state != BT_CONFIG) {
+			rcu_read_unlock();
+			return true;
+		}
+
+		rcu_read_unlock();
 	}
 
 	return false;
-- 
2.51.0

[RFC PATCH 09/24] Bluetooth: mgmt: extend conn_hash lookup RCU critical sections

[Pauli Virtanen]

When doing hci_conn_hash lookups with only RCU, extend critical section
to cover also the handling of the return value.

This avoids any concurrent hci_conn_del from deleting the conn before we
are done dereferencing it.

Use hci_dev_lock instead of just RCU when we are reading conn fields, as
it's probably more correct.

Fixes: 227a0cdf4a028 ("Bluetooth: MGMT: Fix not generating command complete for MGMT_OP_DISCONNECT")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/mgmt.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ee7068fb9fb5..c18ce1aeb34f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3214,6 +3214,8 @@ static int disconnect_sync(struct hci_dev *hdev, void *data)
 	struct mgmt_cp_disconnect *cp = cmd->param;
 	struct hci_conn *conn;
 
+	hci_dev_lock(hdev);
+
 	if (cp->addr.type == BDADDR_BREDR)
 		conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
 					       &cp->addr.bdaddr);
@@ -3221,15 +3223,15 @@ static int disconnect_sync(struct hci_dev *hdev, void *data)
 		conn = hci_conn_hash_lookup_le(hdev, &cp->addr.bdaddr,
 					       le_addr_type(cp->addr.type));
 
-	if (!conn)
-		return -ENOTCONN;
+	hci_dev_unlock(hdev);
 
 	/* Disregard any possible error since the likes of hci_abort_conn_sync
 	 * will clean up the connection no matter the error.
 	 */
-	hci_abort_conn(conn, HCI_ERROR_REMOTE_USER_TERM);
+	if (conn)
+		hci_abort_conn(conn, HCI_ERROR_REMOTE_USER_TERM);
 
-	return 0;
+	return conn ? 0 : -ENOTCONN;
 }
 
 static int disconnect(struct sock *sk, struct hci_dev *hdev, void *data,
@@ -7273,6 +7275,9 @@ static void get_conn_info_complete(struct hci_dev *hdev, void *data, int err)
 		rp.max_tx_power = HCI_TX_POWER_INVALID;
 	}
 
+	if (conn)
+		hci_conn_put(conn);
+
 	mgmt_cmd_complete(cmd->sk, cmd->hdev->id, MGMT_OP_GET_CONN_INFO, status,
 			  &rp, sizeof(rp));
 
@@ -7287,6 +7292,8 @@ static int get_conn_info_sync(struct hci_dev *hdev, void *data)
 	int err;
 	__le16   handle;
 
+	hci_dev_lock(hdev);
+
 	/* Make sure we are still connected */
 	if (cp->addr.type == BDADDR_BREDR)
 		conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK,
@@ -7294,12 +7301,16 @@ static int get_conn_info_sync(struct hci_dev *hdev, void *data)
 	else
 		conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &cp->addr.bdaddr);
 
-	if (!conn || conn->state != BT_CONNECTED)
+	if (!conn || conn->state != BT_CONNECTED) {
+		hci_dev_unlock(hdev);
 		return MGMT_STATUS_NOT_CONNECTED;
+	}
 
-	cmd->user_data = conn;
+	cmd->user_data = hci_conn_get(conn);
 	handle = cpu_to_le16(conn->handle);
 
+	hci_dev_unlock(hdev);
+
 	/* Refresh RSSI each time */
 	err = hci_read_rssi_sync(hdev, handle);
 
@@ -7428,11 +7439,16 @@ static void get_clock_info_complete(struct hci_dev *hdev, void *data, int err)
 	rp.local_clock = cpu_to_le32(hdev->clock);
 
 	if (conn) {
+		hci_dev_lock(hdev);
 		rp.piconet_clock = cpu_to_le32(conn->clock);
 		rp.accuracy = cpu_to_le16(conn->clock_accuracy);
+		hci_dev_unlock(hdev);
 	}
 
 complete:
+	if (conn)
+		hci_conn_put(conn);
+
 	mgmt_cmd_complete(cmd->sk, cmd->hdev->id, cmd->opcode, status, &rp,
 			  sizeof(rp));
 
@@ -7449,15 +7465,21 @@ static int get_clock_info_sync(struct hci_dev *hdev, void *data)
 	memset(&hci_cp, 0, sizeof(hci_cp));
 	hci_read_clock_sync(hdev, &hci_cp);
 
+	hci_dev_lock(hdev);
+
 	/* Make sure connection still exists */
 	conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &cp->addr.bdaddr);
-	if (!conn || conn->state != BT_CONNECTED)
+	if (!conn || conn->state != BT_CONNECTED) {
+		hci_dev_unlock(hdev);
 		return MGMT_STATUS_NOT_CONNECTED;
+	}
 
-	cmd->user_data = conn;
+	cmd->user_data = hci_conn_get(conn);
 	hci_cp.handle = cpu_to_le16(conn->handle);
 	hci_cp.which = 0x01; /* Piconet clock */
 
+	hci_dev_unlock(hdev);
+
 	return hci_read_clock_sync(hdev, &hci_cp);
 }
 
-- 
2.51.0

[RFC PATCH 10/24] Bluetooth: hci_conn: extend conn_hash lookup RCU critical sections

[Pauli Virtanen]

When doing hci_conn_hash lookups with only RCU, extend critical section
to cover also the handling of the return value.

This avoids any concurrent hci_conn_del from deleting the conn before we
are done dereferencing it.

Fixes: a091289218202 ("Bluetooth: hci_conn: Fix hci_le_set_cig_params")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_conn.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 1d914d95cb6d..b6c3ee60024f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1813,9 +1813,13 @@ static int set_cig_params_sync(struct hci_dev *hdev, void *data)
 	u8 aux_num_cis = 0;
 	u8 cis_id;
 
+	rcu_read_lock();
+
 	conn = hci_conn_hash_lookup_cig(hdev, cig_id);
-	if (!conn)
+	if (!conn) {
+		rcu_read_unlock();
 		return 0;
+	}
 
 	qos = &conn->iso_qos;
 	pdu->cig_id = cig_id;
@@ -1854,6 +1858,8 @@ static int set_cig_params_sync(struct hci_dev *hdev, void *data)
 	}
 	pdu->num_cis = aux_num_cis;
 
+	rcu_read_unlock();
+
 	if (!pdu->num_cis)
 		return 0;
 
-- 
2.51.0

[RFC PATCH 11/24] Bluetooth: hci_core: add lockdep check to hci_conn_hash lookups

[Pauli Virtanen]

Callers of conn_hash lookup functions that return hci_conn*, shall hold
hdev->lock/rcu_read_lock covering dereferencing and other usage of the
returned conn. Cf. BUG!!! in Documentation/RCU/whatisRCU.rst

This makes builds with CONFIG_PROVE_RCU emit lockdep splats if these
functions are called without appropriate locks.

The lookup functions actually should instead not do rcu_read_lock() but
list_for_each_entry_rcu(c, &h->list, list, lockdep_is_held(&hdev->lock))
leaving locking to the caller. However, for now just emit lockdep
warning, in case there are remaining unconverted callsites.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
    There's been several UAF from hci_conn* being deleted concurrently.
    
    It's probably a good idea to add these lockdep warnings in the
    hci_conn_hash functions, and to fix up whatever they reveal.
    
    RFC since probably should fix the issues these reveal...  I have an
    unfinished patch series that fixes up the lockdep splats these checks
    generate.
    
    Locking in hci_sync maybe needs some redesign there.
    
    General pattern is just
    
        hci_dev_lock(hdev); /* or rcu_read_lock() */
        conn = hci_conn_hash_lookup_xxx(hdev, ...);
        if (!conn) {
            hci_dev_unlock(hdev);
            return -ENOENT;
        }
        do_something(stuff);
        hci_dev_unlock(hdev);
    
    or
    
        rcu_read_lock();
        conn = hci_conn_hash_lookup_xxx(hdev, ...);
        if (conn)
            hci_conn_get(conn);
        rcu_read_unlock();
        if (!conn)
            return -ENOENT;
        do_something(conn);
        hci_conn_put(conn);

 include/net/bluetooth/hci_core.h | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 2924c2bf2a98..b0f2adfdd5b4 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1060,6 +1060,18 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
 	}
 }
 
+/* TODO: later on, remove all rcu_read_lock() in lookups and use instead
+ * list_for_each_entry_rcu(c, &h->list, list, lockdep_is_held(&hdev->lock))
+ */
+#ifdef CONFIG_PROVE_RCU
+#define HCI_CONN_HASH_LOCKDEP_CHECK(hdev)				\
+	RCU_LOCKDEP_WARN(!lockdep_is_held(&(hdev)->lock) &&		\
+			 !rcu_read_lock_held(),				\
+			 "wrong 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;
@@ -1141,6 +1153,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) {
@@ -1163,6 +1177,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) {
@@ -1189,6 +1205,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) {
@@ -1213,6 +1231,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) {
@@ -1232,6 +1252,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) {
@@ -1253,6 +1275,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) {
@@ -1274,6 +1298,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) {
@@ -1300,6 +1326,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) {
@@ -1332,6 +1360,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) {
@@ -1355,6 +1385,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) {
@@ -1379,6 +1411,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) {
@@ -1403,6 +1437,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) {
@@ -1426,6 +1462,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) {
@@ -1449,6 +1487,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) {
@@ -1518,6 +1558,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.51.0

[RFC PATCH 12/24] Bluetooth: hci_core: add lockdep check to hci_conn_valid()

[Pauli Virtanen]

Callers of hci_conn_valid() shall hold hdev->lock/rcu_read_lock covering
dereferencing and other usage of the returned conn.

Typically hci_conn_valid(conn) is used to check whether conn is still
alive, after which it is dereferenced.  This is potential UAF if there
is no proper locking, as conn may be deleted after hci_conn_valid()
check was done. If hci_conn_get() refcount is held, there is no UAF but
the call to hci_conn_valid() is useless as it doesn't guarantee
anything.

Add lockdep splat to hci_conn_valid() to catch callers with wrong
locking.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 include/net/bluetooth/hci_core.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b0f2adfdd5b4..68a5c3214cfe 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1114,6 +1114,8 @@ static inline bool hci_conn_valid(struct hci_dev *hdev, struct hci_conn *conn)
 	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.51.0

[RFC PATCH 13/24] Bluetooth: hci_sync: fix hdev locking in hci_le_create_conn_sync

[Pauli Virtanen]

Fix call to hci_conn_del() to hold hdev->lock and check the conn was not
concurrently deleted.

Fixes: 8e8b92ee60de5 ("Bluetooth: hci_sync: Add hci_le_create_conn_sync")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
    hci_le_create_conn_sync() probably would need to do more locking/checks
    etc.  to guard against the conn being deleted or modified while it is
    running. That likely is not UAF, but only misbehavior.
    
    Only the wrong delete locking bug is fixed in this patch.

 net/bluetooth/hci_sync.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 408fe5e08c30..8fe2f5b73040 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -6584,7 +6584,10 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
 		if (hci_dev_test_flag(hdev, HCI_LE_SCAN) &&
 		    hdev->le_scan_type == LE_SCAN_ACTIVE &&
 		    !hci_dev_test_flag(hdev, HCI_LE_SIMULTANEOUS_ROLES)) {
-			hci_conn_del(conn);
+			hci_dev_lock(hdev);
+			if (hci_conn_valid(hdev, conn))
+				hci_conn_del(conn);
+			hci_dev_unlock(hdev);
 			return -EBUSY;
 		}
 
-- 
2.51.0

[RFC PATCH 14/24] Bluetooth: hci_core: hold hdev lock in packet TX scheduler

[Pauli Virtanen]

hci_conn* shall be dereferenced only if holding appropriate lock.

Have packet schedulers hold hdev lock while sending packets for conns.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_core.c | 50 ++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3418d7b964a1..f106586cc5a3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3428,9 +3428,8 @@ static struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type,
 	/* We don't have to lock device here. Connections are always
 	 * added and removed with TX task disabled. */
 
-	rcu_read_lock();
-
-	list_for_each_entry_rcu(c, &h->list, list) {
+	list_for_each_entry_rcu(c, &h->list, list,
+				lockdep_is_held(&hdev->lock)) {
 		if (c->type != type ||
 		    skb_queue_empty(&c->data_q))
 			continue;
@@ -3453,8 +3452,6 @@ static struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type,
 			break;
 	}
 
-	rcu_read_unlock();
-
 	hci_quote_sent(conn, num, quote);
 
 	BT_DBG("conn %p quote %d", conn, *quote);
@@ -3468,18 +3465,15 @@ static void hci_link_tx_to(struct hci_dev *hdev, __u8 type)
 
 	bt_dev_err(hdev, "link tx timeout");
 
-	hci_dev_lock(hdev);
-
 	/* Kill stalled connections */
-	list_for_each_entry(c, &h->list, list) {
+	list_for_each_entry_rcu(c, &h->list, list,
+				lockdep_is_held(&hdev->lock)) {
 		if (c->type == type && c->sent) {
 			bt_dev_err(hdev, "killing stalled connection %pMR",
 				   &c->dst);
 			hci_disconnect(c, HCI_ERROR_REMOTE_USER_TERM);
 		}
 	}
-
-	hci_dev_unlock(hdev);
 }
 
 static struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type,
@@ -3493,9 +3487,8 @@ static struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type,
 
 	BT_DBG("%s", hdev->name);
 
-	rcu_read_lock();
-
-	list_for_each_entry_rcu(conn, &h->list, list) {
+	list_for_each_entry_rcu(conn, &h->list, list,
+				lockdep_is_held(&hdev->lock)) {
 		struct hci_chan *tmp;
 
 		if (conn->type != type)
@@ -3534,8 +3527,6 @@ static struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type,
 			break;
 	}
 
-	rcu_read_unlock();
-
 	if (!chan)
 		return NULL;
 
@@ -3632,7 +3623,7 @@ static void __check_timeout(struct hci_dev *hdev, unsigned int cnt, u8 type)
 }
 
 /* Schedule SCO */
-static void hci_sched_sco(struct hci_dev *hdev, __u8 type)
+static void __hci_sched_sco(struct hci_dev *hdev, __u8 type)
 {
 	struct hci_conn *conn;
 	struct sk_buff *skb;
@@ -3673,6 +3664,13 @@ static void hci_sched_sco(struct hci_dev *hdev, __u8 type)
 		queue_work(hdev->workqueue, &hdev->tx_work);
 }
 
+static void hci_sched_sco(struct hci_dev *hdev, __u8 type)
+{
+	hci_dev_lock(hdev);
+	__hci_sched_sco(hdev, type);
+	hci_dev_unlock(hdev);
+}
+
 static void hci_sched_acl_pkt(struct hci_dev *hdev)
 {
 	unsigned int cnt = hdev->acl_cnt;
@@ -3680,6 +3678,8 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev)
 	struct sk_buff *skb;
 	int quote;
 
+	hci_dev_lock(hdev);
+
 	__check_timeout(hdev, cnt, ACL_LINK);
 
 	while (hdev->acl_cnt &&
@@ -3706,11 +3706,13 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev)
 			chan->conn->sent++;
 
 			/* Send pending SCO packets right away */
-			hci_sched_sco(hdev, SCO_LINK);
-			hci_sched_sco(hdev, ESCO_LINK);
+			__hci_sched_sco(hdev, SCO_LINK);
+			__hci_sched_sco(hdev, ESCO_LINK);
 		}
 	}
 
+	hci_dev_unlock(hdev);
+
 	if (cnt != hdev->acl_cnt)
 		hci_prio_recalculate(hdev, ACL_LINK);
 }
@@ -3739,6 +3741,8 @@ static void hci_sched_le(struct hci_dev *hdev)
 
 	cnt = hdev->le_pkts ? &hdev->le_cnt : &hdev->acl_cnt;
 
+	hci_dev_lock(hdev);
+
 	__check_timeout(hdev, *cnt, LE_LINK);
 
 	tmp = *cnt;
@@ -3762,11 +3766,13 @@ static void hci_sched_le(struct hci_dev *hdev)
 			chan->conn->sent++;
 
 			/* Send pending SCO packets right away */
-			hci_sched_sco(hdev, SCO_LINK);
-			hci_sched_sco(hdev, ESCO_LINK);
+			__hci_sched_sco(hdev, SCO_LINK);
+			__hci_sched_sco(hdev, ESCO_LINK);
 		}
 	}
 
+	hci_dev_unlock(hdev);
+
 	if (*cnt != tmp)
 		hci_prio_recalculate(hdev, LE_LINK);
 }
@@ -3785,6 +3791,8 @@ static void hci_sched_iso(struct hci_dev *hdev, __u8 type)
 
 	cnt = &hdev->iso_cnt;
 
+	hci_dev_lock(hdev);
+
 	__check_timeout(hdev, *cnt, type);
 
 	while (*cnt && (conn = hci_low_sent(hdev, type, &quote))) {
@@ -3800,6 +3808,8 @@ static void hci_sched_iso(struct hci_dev *hdev, __u8 type)
 			(*cnt)--;
 		}
 	}
+
+	hci_dev_unlock(hdev);
 }
 
 static void hci_tx_work(struct work_struct *work)
-- 
2.51.0

[RFC PATCH 15/24] Bluetooth: lookup hci_conn on RX path on protocol side

[Pauli Virtanen]

The lock/lookup/unlock pattern in the packet RX path is unsafe. The lock
needs to be held as long as the hci_conn is used.

The critical section varies somewhat between protocols, so pass handle
to the protocol recv handler, and do conn lookup there.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 include/net/bluetooth/hci_core.h | 20 +++++++---
 net/bluetooth/hci_core.c         | 64 ++++++++------------------------
 net/bluetooth/iso.c              | 30 ++++++++++++---
 net/bluetooth/l2cap_core.c       | 24 +++++++++---
 net/bluetooth/sco.c              | 35 ++++++++++++-----
 5 files changed, 99 insertions(+), 74 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 68a5c3214cfe..1e9b27b3b108 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -856,11 +856,12 @@ extern struct mutex hci_cb_list_lock;
 /* ----- HCI interface to upper protocols ----- */
 int l2cap_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr);
 int l2cap_disconn_ind(struct hci_conn *hcon);
-void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags);
+int l2cap_recv_acldata(struct hci_dev *hdev, __u16 handle, struct sk_buff *skb,
+		       u16 flags);
 
 #if IS_ENABLED(CONFIG_BT_BREDR)
 int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags);
-void sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb);
+int sco_recv_scodata(struct hci_dev *hdev, __u16 handle, struct sk_buff *skb);
 #else
 static inline int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
 				  __u8 *flags)
@@ -868,23 +869,30 @@ static inline int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
 	return 0;
 }
 
-static inline void sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb)
+static inline int sco_recv_scodata(struct hci_dev *hdev, __u16 handle,
+				   struct sk_buff *skb)
 {
+	kfree_skb(skb);
+	return 0;
 }
 #endif
 
 #if IS_ENABLED(CONFIG_BT_LE)
 int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags);
-void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags);
+int iso_recv(struct hci_dev *hdev, __u16 handle, struct sk_buff *skb,
+	     u16 flags);
 #else
 static inline int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
 				  __u8 *flags)
 {
 	return 0;
 }
-static inline void iso_recv(struct hci_conn *hcon, struct sk_buff *skb,
-			    u16 flags)
+
+static inline int iso_recv(struct hci_dev *hdev, __u16 handle,
+			   struct sk_buff *skb, u16 flags)
 {
+	kfree_skb(skb);
+	return 0;
 }
 #endif
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index f106586cc5a3..35483f614a1c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3842,13 +3842,14 @@ static void hci_tx_work(struct work_struct *work)
 static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_acl_hdr *hdr;
-	struct hci_conn *conn;
 	__u16 handle, flags;
+	int err;
 
 	hdr = skb_pull_data(skb, sizeof(*hdr));
 	if (!hdr) {
 		bt_dev_err(hdev, "ACL packet too small");
-		goto drop;
+		kfree_skb(skb);
+		return;
 	}
 
 	handle = __le16_to_cpu(hdr->handle);
@@ -3860,36 +3861,24 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 
 	hdev->stat.acl_rx++;
 
-	hci_dev_lock(hdev);
-	conn = hci_conn_hash_lookup_handle(hdev, handle);
-	hci_dev_unlock(hdev);
-
-	if (conn) {
-		hci_conn_enter_active_mode(conn, BT_POWER_FORCE_ACTIVE_OFF);
-
-		/* Send to upper protocol */
-		l2cap_recv_acldata(conn, skb, flags);
-		return;
-	} else {
+	err = l2cap_recv_acldata(hdev, handle, skb, flags);
+	if (err == -ENOENT)
 		bt_dev_err(hdev, "ACL packet for unknown connection handle %d",
 			   handle);
-	}
-
-drop:
-	kfree_skb(skb);
 }
 
 /* SCO data packet */
 static void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_sco_hdr *hdr;
-	struct hci_conn *conn;
 	__u16 handle, flags;
+	int err;
 
 	hdr = skb_pull_data(skb, sizeof(*hdr));
 	if (!hdr) {
 		bt_dev_err(hdev, "SCO packet too small");
-		goto drop;
+		kfree_skb(skb);
+		return;
 	}
 
 	handle = __le16_to_cpu(hdr->handle);
@@ -3901,34 +3890,25 @@ static void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 
 	hdev->stat.sco_rx++;
 
-	hci_dev_lock(hdev);
-	conn = hci_conn_hash_lookup_handle(hdev, handle);
-	hci_dev_unlock(hdev);
+	hci_skb_pkt_status(skb) = flags & 0x03;
 
-	if (conn) {
-		/* Send to upper protocol */
-		hci_skb_pkt_status(skb) = flags & 0x03;
-		sco_recv_scodata(conn, skb);
-		return;
-	} else {
+	err = sco_recv_scodata(hdev, handle, skb);
+	if (err == -ENOENT)
 		bt_dev_err_ratelimited(hdev, "SCO packet for unknown connection handle %d",
 				       handle);
-	}
-
-drop:
-	kfree_skb(skb);
 }
 
 static void hci_isodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_iso_hdr *hdr;
-	struct hci_conn *conn;
 	__u16 handle, flags;
+	int err;
 
 	hdr = skb_pull_data(skb, sizeof(*hdr));
 	if (!hdr) {
 		bt_dev_err(hdev, "ISO packet too small");
-		goto drop;
+		kfree_skb(skb);
+		return;
 	}
 
 	handle = __le16_to_cpu(hdr->handle);
@@ -3938,22 +3918,10 @@ static void hci_isodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 	bt_dev_dbg(hdev, "len %d handle 0x%4.4x flags 0x%4.4x", skb->len,
 		   handle, flags);
 
-	hci_dev_lock(hdev);
-	conn = hci_conn_hash_lookup_handle(hdev, handle);
-	hci_dev_unlock(hdev);
-
-	if (!conn) {
+	err = iso_recv(hdev, handle, skb, flags);
+	if (err == -ENOENT)
 		bt_dev_err(hdev, "ISO packet for unknown connection handle %d",
 			   handle);
-		goto drop;
-	}
-
-	/* Send to upper protocol */
-	iso_recv(conn, skb, flags);
-	return;
-
-drop:
-	kfree_skb(skb);
 }
 
 static bool hci_req_is_complete(struct hci_dev *hdev)
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index f5a9a13317df..541e71aef6bd 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -2301,14 +2301,31 @@ static void iso_disconn_cfm(struct hci_conn *hcon, __u8 reason)
 	iso_conn_del(hcon, bt_to_errno(reason));
 }
 
-void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
+int iso_recv(struct hci_dev *hdev, __u16 handle, struct sk_buff *skb,
+	     u16 flags)
 {
-	struct iso_conn *conn = hcon->iso_data;
+	struct hci_conn *hcon;
+	struct iso_conn *conn;
 	struct skb_shared_hwtstamps *hwts;
 	__u16 pb, ts, len, sn;
 
-	if (!conn)
-		goto drop;
+	hci_dev_lock(hdev);
+
+	hcon = hci_conn_hash_lookup_handle(hdev, handle);
+	if (!hcon) {
+		hci_dev_unlock(hdev);
+		kfree_skb(skb);
+		return -ENOENT;
+	}
+
+	conn = iso_conn_hold_unless_zero(hcon->iso_data);
+	if (!conn) {
+		hci_dev_unlock(hdev);
+		kfree_skb(skb);
+		return -ENOENT;
+	}
+
+	hci_dev_unlock(hdev);
 
 	pb     = hci_iso_flags_pb(flags);
 	ts     = hci_iso_flags_ts(flags);
@@ -2364,7 +2381,7 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
 			hci_skb_pkt_status(skb) = flags & 0x03;
 			hci_skb_pkt_seqnum(skb) = sn;
 			iso_recv_frame(conn, skb);
-			return;
+			goto done;
 		}
 
 		if (pb == ISO_SINGLE) {
@@ -2442,6 +2459,9 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
 
 drop:
 	kfree_skb(skb);
+done:
+	iso_conn_put(conn);
+	return 0;
 }
 
 static struct hci_cb iso_cb = {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 805c752ac0a9..9370b7bd226a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7509,13 +7509,23 @@ struct l2cap_conn *l2cap_conn_hold_unless_zero(struct l2cap_conn *c)
 	return c;
 }
 
-void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
+int l2cap_recv_acldata(struct hci_dev *hdev, uint16_t handle,
+		       struct sk_buff *skb, u16 flags)
 {
+	struct hci_conn *hcon;
 	struct l2cap_conn *conn;
 	int len;
 
-	/* Lock hdev to access l2cap_data to avoid race with l2cap_conn_del */
-	hci_dev_lock(hcon->hdev);
+	hci_dev_lock(hdev);
+
+	hcon = hci_conn_hash_lookup_handle(hdev, handle);
+	if (!hcon) {
+		hci_dev_unlock(hdev);
+		kfree_skb(skb);
+		return -ENOENT;
+	}
+
+	hci_conn_enter_active_mode(hcon, BT_POWER_FORCE_ACTIVE_OFF);
 
 	conn = hcon->l2cap_data;
 
@@ -7524,13 +7534,14 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
 
 	conn = l2cap_conn_hold_unless_zero(conn);
 
-	hci_dev_unlock(hcon->hdev);
-
 	if (!conn) {
+		hci_dev_unlock(hdev);
 		kfree_skb(skb);
-		return;
+		return -ENOENT;
 	}
 
+	hci_dev_unlock(hdev);
+
 	BT_DBG("conn %p len %u flags 0x%x", conn, skb->len, flags);
 
 	mutex_lock(&conn->lock);
@@ -7642,6 +7653,7 @@ void l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
 unlock:
 	mutex_unlock(&conn->lock);
 	l2cap_conn_put(conn);
+	return 0;
 }
 
 static struct hci_cb l2cap_cb = {
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d382d980fd9a..b5746e4caf20 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -1451,22 +1451,39 @@ static void sco_disconn_cfm(struct hci_conn *hcon, __u8 reason)
 	sco_conn_del(hcon, bt_to_errno(reason));
 }
 
-void sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb)
+int sco_recv_scodata(struct hci_dev *hdev, __u16 handle, struct sk_buff *skb)
 {
-	struct sco_conn *conn = hcon->sco_data;
+	struct hci_conn *hcon;
+	struct sco_conn *conn;
 
-	if (!conn)
-		goto drop;
+	hci_dev_lock(hdev);
+
+	hcon = hci_conn_hash_lookup_handle(hdev, handle);
+	if (!hcon) {
+		hci_dev_unlock(hdev);
+		kfree_skb(skb);
+		return -ENOENT;
+	}
+
+	conn = hcon->sco_data;
+	if (!conn) {
+		hci_dev_unlock(hdev);
+		kfree_skb(skb);
+		return -ENOENT;
+	}
+
+	sco_conn_hold(conn);
+	hci_dev_unlock(hdev);
 
 	BT_DBG("conn %p len %u", conn, skb->len);
 
-	if (skb->len) {
+	if (skb->len)
 		sco_recv_frame(conn, skb);
-		return;
-	}
+	else
+		kfree_skb(skb);
 
-drop:
-	kfree_skb(skb);
+	sco_conn_put(conn);
+	return 0;
 }
 
 static struct hci_cb sco_cb = {
-- 
2.51.0

[RFC PATCH 16/24] Bluetooth: L2CAP: fix hci_conn_valid() usage

[Pauli Virtanen]

Hold RCU lock while queuing to ensure conn remains valid.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/l2cap_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 9370b7bd226a..9086c86adb2c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -949,10 +949,14 @@ static void l2cap_send_acl(struct l2cap_conn *conn, struct sk_buff *skb,
 			   u8 flags)
 {
 	/* Check if the hcon still valid before attempting to send */
+	rcu_read_lock();
+
 	if (hci_conn_valid(conn->hcon->hdev, conn->hcon))
 		hci_send_acl(conn->hchan, skb, flags);
 	else
 		kfree_skb(skb);
+
+	rcu_read_unlock();
 }
 
 static void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len,
-- 
2.51.0

[RFC PATCH 17/24] Bluetooth: hci_sync: add helper for hdev locking across waits

[Pauli Virtanen]

Add hci_dev_lock/unlock_sync, for holding hdev->lock except when waiting
request completion on hdev->req_wait_q.

This makes writing hci_sync subroutines somewhat simpler, as the lock
needs to be acquired only on top level.  Routines will however still
have to recheck conditions after waits.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 include/net/bluetooth/hci_core.h |  2 ++
 include/net/bluetooth/hci_sync.h |  4 ++++
 net/bluetooth/hci_sync.c         | 26 ++++++++++++++++++++++++++
 3 files changed, 32 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 1e9b27b3b108..10960462c5dd 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -533,6 +533,8 @@ struct hci_dev {
 	struct sk_buff		*req_skb;
 	struct sk_buff		*req_rsp;
 
+	bool			req_hdev_locked;
+
 	void			*smp_data;
 	void			*smp_bredr_data;
 
diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h
index e352a4e0ef8d..eabc423b210e 100644
--- a/include/net/bluetooth/hci_sync.h
+++ b/include/net/bluetooth/hci_sync.h
@@ -188,3 +188,7 @@ int hci_le_conn_update_sync(struct hci_dev *hdev, struct hci_conn *conn,
 
 int hci_connect_pa_sync(struct hci_dev *hdev, struct hci_conn *conn);
 int hci_connect_big_sync(struct hci_dev *hdev, struct hci_conn *conn);
+
+void hci_dev_lock_sync(struct hci_dev *hdev);
+void hci_dev_unlock_sync(struct hci_dev *hdev);
+void hci_assert_lock_sync_held(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 8fe2f5b73040..5391c1bb17f0 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -20,6 +20,25 @@
 #include "aosp.h"
 #include "leds.h"
 
+void hci_dev_lock_sync(struct hci_dev *hdev)
+{
+	hci_dev_lock(hdev);
+	lockdep_assert(!hdev->req_hdev_locked);
+	hdev->req_hdev_locked = true;
+}
+
+void hci_dev_unlock_sync(struct hci_dev *hdev)
+{
+	lockdep_assert(hdev->req_hdev_locked);
+	hdev->req_hdev_locked = false;
+	hci_dev_unlock(hdev);
+}
+
+void hci_assert_lock_sync_held(struct hci_dev *hdev)
+{
+	lockdep_assert(lockdep_is_held(&hdev->lock) && hdev->req_hdev_locked);
+}
+
 static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode,
 				  struct sk_buff *skb)
 {
@@ -159,6 +178,7 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
 {
 	struct hci_request req;
 	struct sk_buff *skb;
+	bool locked = READ_ONCE(hdev->req_hdev_locked);
 	int err = 0;
 
 	bt_dev_dbg(hdev, "Opcode 0x%4.4x", opcode);
@@ -173,10 +193,16 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
 	if (err < 0)
 		return ERR_PTR(err);
 
+	if (locked)
+		hci_dev_unlock(hdev);
+
 	err = wait_event_interruptible_timeout(hdev->req_wait_q,
 					       hdev->req_status != HCI_REQ_PEND,
 					       timeout);
 
+	if (locked)
+		hci_dev_lock(hdev);
+
 	if (err == -ERESTARTSYS)
 		return ERR_PTR(-EINTR);
 
-- 
2.51.0

Re: [RFC PATCH 17/24] Bluetooth: hci_sync: add helper for hdev locking across waits

[Luiz Augusto von Dentz]

Hi Pauli,

On Sun, Sep 21, 2025 at 3:19 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Add hci_dev_lock/unlock_sync, for holding hdev->lock except when waiting
> request completion on hdev->req_wait_q.
>
> This makes writing hci_sync subroutines somewhat simpler, as the lock
> needs to be acquired only on top level.  Routines will however still
> have to recheck conditions after waits.

This one doesn't sound quite right, I'm afraid this will impact the
lifetime of hdev itself as well since this means the hdev cannot be
freed while req_hdev_locked, so it is effectively another lock, also I
think we might be better of adding a lock directly to hci_conn in that
case since most problems seems related to accessing it after waiting
for an event.

> Signed-off-by: Pauli Virtanen <pav@iki.fi>
> ---
>  include/net/bluetooth/hci_core.h |  2 ++
>  include/net/bluetooth/hci_sync.h |  4 ++++
>  net/bluetooth/hci_sync.c         | 26 ++++++++++++++++++++++++++
>  3 files changed, 32 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 1e9b27b3b108..10960462c5dd 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -533,6 +533,8 @@ struct hci_dev {
>         struct sk_buff          *req_skb;
>         struct sk_buff          *req_rsp;
>
> +       bool                    req_hdev_locked;
> +
>         void                    *smp_data;
>         void                    *smp_bredr_data;
>
> diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h
> index e352a4e0ef8d..eabc423b210e 100644
> --- a/include/net/bluetooth/hci_sync.h
> +++ b/include/net/bluetooth/hci_sync.h
> @@ -188,3 +188,7 @@ int hci_le_conn_update_sync(struct hci_dev *hdev, struct hci_conn *conn,
>
>  int hci_connect_pa_sync(struct hci_dev *hdev, struct hci_conn *conn);
>  int hci_connect_big_sync(struct hci_dev *hdev, struct hci_conn *conn);
> +
> +void hci_dev_lock_sync(struct hci_dev *hdev);
> +void hci_dev_unlock_sync(struct hci_dev *hdev);
> +void hci_assert_lock_sync_held(struct hci_dev *hdev);
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 8fe2f5b73040..5391c1bb17f0 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -20,6 +20,25 @@
>  #include "aosp.h"
>  #include "leds.h"
>
> +void hci_dev_lock_sync(struct hci_dev *hdev)
> +{
> +       hci_dev_lock(hdev);
> +       lockdep_assert(!hdev->req_hdev_locked);
> +       hdev->req_hdev_locked = true;
> +}
> +
> +void hci_dev_unlock_sync(struct hci_dev *hdev)
> +{
> +       lockdep_assert(hdev->req_hdev_locked);
> +       hdev->req_hdev_locked = false;
> +       hci_dev_unlock(hdev);
> +}
> +
> +void hci_assert_lock_sync_held(struct hci_dev *hdev)
> +{
> +       lockdep_assert(lockdep_is_held(&hdev->lock) && hdev->req_hdev_locked);
> +}
> +
>  static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode,
>                                   struct sk_buff *skb)
>  {
> @@ -159,6 +178,7 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
>  {
>         struct hci_request req;
>         struct sk_buff *skb;
> +       bool locked = READ_ONCE(hdev->req_hdev_locked);
>         int err = 0;
>
>         bt_dev_dbg(hdev, "Opcode 0x%4.4x", opcode);
> @@ -173,10 +193,16 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
>         if (err < 0)
>                 return ERR_PTR(err);
>
> +       if (locked)
> +               hci_dev_unlock(hdev);
> +
>         err = wait_event_interruptible_timeout(hdev->req_wait_q,
>                                                hdev->req_status != HCI_REQ_PEND,
>                                                timeout);
>
> +       if (locked)
> +               hci_dev_lock(hdev);
> +
>         if (err == -ERESTARTSYS)
>                 return ERR_PTR(-EINTR);
>
> --
> 2.51.0
>
>


-- 
Luiz Augusto von Dentz

Re: [RFC PATCH 17/24] Bluetooth: hci_sync: add helper for hdev locking across waits

[Pauli Virtanen]

Hi Luiz,

ma, 2025-09-22 kello 10:51 -0400, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
> 
> On Sun, Sep 21, 2025 at 3:19 PM Pauli Virtanen <pav@iki.fi> wrote:
> > 
> > Add hci_dev_lock/unlock_sync, for holding hdev->lock except when waiting
> > request completion on hdev->req_wait_q.
> > 
> > This makes writing hci_sync subroutines somewhat simpler, as the lock
> > needs to be acquired only on top level.  Routines will however still
> > have to recheck conditions after waits.
> 
> This one doesn't sound quite right, I'm afraid this will impact the
> lifetime of hdev itself as well since this means the hdev cannot be
> freed while req_hdev_locked, so it is effectively another lock

Hmm, I'm not quite seeing it currently:

	hci_dev_lock_sync(hdev);
	...
	err = __hci_cmd_sync_status(hdev, ...);
	...
	hci_dev_unlock_sync(hdev);

is equivalent to

	hci_dev_lock(hdev);
	...
	hci_dev_unlock(hdev);
	err = __hci_cmd_sync_status(hdev, ...);
	hci_dev_lock(hdev);
	...
	hci_dev_unlock(hdev);

(but without having to worry about conn->xxx arguments passed to
hci_cmd_sync_*, subroutines don't need unlock/lock everywhere, and in
code the name differentiates from normal hci_dev_lock critical section
which subroutines aren't usually expected to be unlocking.)

I.e. it unlocks before every event wait, the lock is held only during
hci_sync payload code execution which shouldn't have delays, so I'm not
seeing why it would be very different from what is already in hci_sync.

The hdev cannot be freed also before if there is a hci_sync routine
still executing, destroy_workqueue() drains the queue first.

I think since we are not holding hdev->lock when destroying the
workqueue, this shouldn't deadlock that, or affect how canceling work
items functions.

> also I think we might be better of adding a lock directly to
> hci_conn in that case since most problems seems related to accessing
> it after waiting for an event.

Doesn't lock in hci_conn lead to similar issue as with hdev->lock here:

If hci_sync task holds lock over event wait, during that wait e.g.
Disconnect event (which iiuc may arrive at unexpected points) may need
to take the lock to delete the same hci_conn -> deadlock problem.

If hci_sync doesn't hold lock over event wait, then it seems you have
to do something similar as done here.

> > Signed-off-by: Pauli Virtanen <pav@iki.fi>
> > ---
> >  include/net/bluetooth/hci_core.h |  2 ++
> >  include/net/bluetooth/hci_sync.h |  4 ++++
> >  net/bluetooth/hci_sync.c         | 26 ++++++++++++++++++++++++++
> >  3 files changed, 32 insertions(+)
> > 
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index 1e9b27b3b108..10960462c5dd 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -533,6 +533,8 @@ struct hci_dev {
> >         struct sk_buff          *req_skb;
> >         struct sk_buff          *req_rsp;
> > 
> > +       bool                    req_hdev_locked;
> > +
> >         void                    *smp_data;
> >         void                    *smp_bredr_data;
> > 
> > diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h
> > index e352a4e0ef8d..eabc423b210e 100644
> > --- a/include/net/bluetooth/hci_sync.h
> > +++ b/include/net/bluetooth/hci_sync.h
> > @@ -188,3 +188,7 @@ int hci_le_conn_update_sync(struct hci_dev *hdev, struct hci_conn *conn,
> > 
> >  int hci_connect_pa_sync(struct hci_dev *hdev, struct hci_conn *conn);
> >  int hci_connect_big_sync(struct hci_dev *hdev, struct hci_conn *conn);
> > +
> > +void hci_dev_lock_sync(struct hci_dev *hdev);
> > +void hci_dev_unlock_sync(struct hci_dev *hdev);
> > +void hci_assert_lock_sync_held(struct hci_dev *hdev);
> > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > index 8fe2f5b73040..5391c1bb17f0 100644
> > --- a/net/bluetooth/hci_sync.c
> > +++ b/net/bluetooth/hci_sync.c
> > @@ -20,6 +20,25 @@
> >  #include "aosp.h"
> >  #include "leds.h"
> > 
> > +void hci_dev_lock_sync(struct hci_dev *hdev)
> > +{
> > +       hci_dev_lock(hdev);
> > +       lockdep_assert(!hdev->req_hdev_locked);
> > +       hdev->req_hdev_locked = true;
> > +}
> > +
> > +void hci_dev_unlock_sync(struct hci_dev *hdev)
> > +{
> > +       lockdep_assert(hdev->req_hdev_locked);
> > +       hdev->req_hdev_locked = false;
> > +       hci_dev_unlock(hdev);
> > +}
> > +
> > +void hci_assert_lock_sync_held(struct hci_dev *hdev)
> > +{
> > +       lockdep_assert(lockdep_is_held(&hdev->lock) && hdev->req_hdev_locked);
> > +}
> > +
> >  static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode,
> >                                   struct sk_buff *skb)
> >  {
> > @@ -159,6 +178,7 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
> >  {
> >         struct hci_request req;
> >         struct sk_buff *skb;
> > +       bool locked = READ_ONCE(hdev->req_hdev_locked);
> >         int err = 0;
> > 
> >         bt_dev_dbg(hdev, "Opcode 0x%4.4x", opcode);
> > @@ -173,10 +193,16 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
> >         if (err < 0)
> >                 return ERR_PTR(err);
> > 
> > +       if (locked)
> > +               hci_dev_unlock(hdev);
> > +
> >         err = wait_event_interruptible_timeout(hdev->req_wait_q,
> >                                                hdev->req_status != HCI_REQ_PEND,
> >                                                timeout);
> > 
> > +       if (locked)
> > +               hci_dev_lock(hdev);
> > +
> >         if (err == -ERESTARTSYS)
> >                 return ERR_PTR(-EINTR);
> > 
> > --
> > 2.51.0
> > 
> > 
> 

Re: [RFC PATCH 17/24] Bluetooth: hci_sync: add helper for hdev locking across waits

[Luiz Augusto von Dentz]

Hi Pauli,

On Mon, Sep 22, 2025 at 12:43 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi Luiz,
>
> ma, 2025-09-22 kello 10:51 -0400, Luiz Augusto von Dentz kirjoitti:
> > Hi Pauli,
> >
> > On Sun, Sep 21, 2025 at 3:19 PM Pauli Virtanen <pav@iki.fi> wrote:
> > >
> > > Add hci_dev_lock/unlock_sync, for holding hdev->lock except when waiting
> > > request completion on hdev->req_wait_q.
> > >
> > > This makes writing hci_sync subroutines somewhat simpler, as the lock
> > > needs to be acquired only on top level.  Routines will however still
> > > have to recheck conditions after waits.
> >
> > This one doesn't sound quite right, I'm afraid this will impact the
> > lifetime of hdev itself as well since this means the hdev cannot be
> > freed while req_hdev_locked, so it is effectively another lock
>
> Hmm, I'm not quite seeing it currently:
>
>         hci_dev_lock_sync(hdev);
>         ...
>         err = __hci_cmd_sync_status(hdev, ...);
>         ...
>         hci_dev_unlock_sync(hdev);
>
> is equivalent to
>
>         hci_dev_lock(hdev);
>         ...
>         hci_dev_unlock(hdev);
>         err = __hci_cmd_sync_status(hdev, ...);
>         hci_dev_lock(hdev);
>         ...
>         hci_dev_unlock(hdev);
>
> (but without having to worry about conn->xxx arguments passed to
> hci_cmd_sync_*, subroutines don't need unlock/lock everywhere, and in
> code the name differentiates from normal hci_dev_lock critical section
> which subroutines aren't usually expected to be unlocking.)
>
> I.e. it unlocks before every event wait, the lock is held only during
> hci_sync payload code execution which shouldn't have delays, so I'm not
> seeing why it would be very different from what is already in hci_sync.
>
> The hdev cannot be freed also before if there is a hci_sync routine
> still executing, destroy_workqueue() drains the queue first.
>
> I think since we are not holding hdev->lock when destroying the
> workqueue, this shouldn't deadlock that, or affect how canceling work
> items functions.
>
> > also I think we might be better of adding a lock directly to
> > hci_conn in that case since most problems seems related to accessing
> > it after waiting for an event.
>
> Doesn't lock in hci_conn lead to similar issue as with hdev->lock here:
>
> If hci_sync task holds lock over event wait, during that wait e.g.
> Disconnect event (which iiuc may arrive at unexpected points) may need
> to take the lock to delete the same hci_conn -> deadlock problem.
>
> If hci_sync doesn't hold lock over event wait, then it seems you have
> to do something similar as done here.

I was thinking more on the likes of locking after hci_conn_valid, so
just to protect the access of fields rather than locking across waits
which as you said may lead to deadlocks, anyway I rather fix the
outstanding reports first then we can take reworks later once we
understand all the problems related to the current design.

Btw, I'm actually planning to get right of mgmt_pending for example,
that is sort of not needed because we can use
hci_cmd_sync_lookup_entry to find mgmt_pending entries.

>
> > > Signed-off-by: Pauli Virtanen <pav@iki.fi>
> > > ---
> > >  include/net/bluetooth/hci_core.h |  2 ++
> > >  include/net/bluetooth/hci_sync.h |  4 ++++
> > >  net/bluetooth/hci_sync.c         | 26 ++++++++++++++++++++++++++
> > >  3 files changed, 32 insertions(+)
> > >
> > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > > index 1e9b27b3b108..10960462c5dd 100644
> > > --- a/include/net/bluetooth/hci_core.h
> > > +++ b/include/net/bluetooth/hci_core.h
> > > @@ -533,6 +533,8 @@ struct hci_dev {
> > >         struct sk_buff          *req_skb;
> > >         struct sk_buff          *req_rsp;
> > >
> > > +       bool                    req_hdev_locked;
> > > +
> > >         void                    *smp_data;
> > >         void                    *smp_bredr_data;
> > >
> > > diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h
> > > index e352a4e0ef8d..eabc423b210e 100644
> > > --- a/include/net/bluetooth/hci_sync.h
> > > +++ b/include/net/bluetooth/hci_sync.h
> > > @@ -188,3 +188,7 @@ int hci_le_conn_update_sync(struct hci_dev *hdev, struct hci_conn *conn,
> > >
> > >  int hci_connect_pa_sync(struct hci_dev *hdev, struct hci_conn *conn);
> > >  int hci_connect_big_sync(struct hci_dev *hdev, struct hci_conn *conn);
> > > +
> > > +void hci_dev_lock_sync(struct hci_dev *hdev);
> > > +void hci_dev_unlock_sync(struct hci_dev *hdev);
> > > +void hci_assert_lock_sync_held(struct hci_dev *hdev);
> > > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > > index 8fe2f5b73040..5391c1bb17f0 100644
> > > --- a/net/bluetooth/hci_sync.c
> > > +++ b/net/bluetooth/hci_sync.c
> > > @@ -20,6 +20,25 @@
> > >  #include "aosp.h"
> > >  #include "leds.h"
> > >
> > > +void hci_dev_lock_sync(struct hci_dev *hdev)
> > > +{
> > > +       hci_dev_lock(hdev);
> > > +       lockdep_assert(!hdev->req_hdev_locked);
> > > +       hdev->req_hdev_locked = true;
> > > +}
> > > +
> > > +void hci_dev_unlock_sync(struct hci_dev *hdev)
> > > +{
> > > +       lockdep_assert(hdev->req_hdev_locked);
> > > +       hdev->req_hdev_locked = false;
> > > +       hci_dev_unlock(hdev);
> > > +}
> > > +
> > > +void hci_assert_lock_sync_held(struct hci_dev *hdev)
> > > +{
> > > +       lockdep_assert(lockdep_is_held(&hdev->lock) && hdev->req_hdev_locked);
> > > +}
> > > +
> > >  static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode,
> > >                                   struct sk_buff *skb)
> > >  {
> > > @@ -159,6 +178,7 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
> > >  {
> > >         struct hci_request req;
> > >         struct sk_buff *skb;
> > > +       bool locked = READ_ONCE(hdev->req_hdev_locked);
> > >         int err = 0;
> > >
> > >         bt_dev_dbg(hdev, "Opcode 0x%4.4x", opcode);
> > > @@ -173,10 +193,16 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
> > >         if (err < 0)
> > >                 return ERR_PTR(err);
> > >
> > > +       if (locked)
> > > +               hci_dev_unlock(hdev);
> > > +
> > >         err = wait_event_interruptible_timeout(hdev->req_wait_q,
> > >                                                hdev->req_status != HCI_REQ_PEND,
> > >                                                timeout);
> > >
> > > +       if (locked)
> > > +               hci_dev_lock(hdev);
> > > +
> > >         if (err == -ERESTARTSYS)
> > >                 return ERR_PTR(-EINTR);
> > >
> > > --
> > > 2.51.0
> > >
> > >
> >



-- 
Luiz Augusto von Dentz

[RFC PATCH 18/24] Bluetooth: hci_sync: hold lock in hci_acl_create_conn_sync

[Pauli Virtanen]

Hold hdev->lock in hci_acl_create_conn_sync() when accessing hci_conn
fields.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_sync.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 5391c1bb17f0..f41aec54659b 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -6923,10 +6923,15 @@ static int hci_acl_create_conn_sync(struct hci_dev *hdev, void *data)
 	struct hci_conn *conn = data;
 	struct inquiry_entry *ie;
 	struct hci_cp_create_conn cp;
+	u32 timeout;
 	int err;
 
-	if (!hci_conn_valid(hdev, conn))
+	hci_dev_lock_sync(hdev);
+
+	if (!hci_conn_valid(hdev, conn)) {
+		hci_dev_unlock_sync(hdev);
 		return -ECANCELED;
+	}
 
 	/* Many controllers disallow HCI Create Connection while it is doing
 	 * HCI Inquiry. So we cancel the Inquiry first before issuing HCI Create
@@ -6941,6 +6946,11 @@ static int hci_acl_create_conn_sync(struct hci_dev *hdev, void *data)
 					    NULL, HCI_CMD_TIMEOUT);
 		if (err)
 			bt_dev_warn(hdev, "Failed to cancel inquiry %d", err);
+
+		if (!hci_conn_valid(hdev, conn)) {
+			hci_dev_unlock_sync(hdev);
+			return -ECANCELED;
+		}
 	}
 
 	conn->state = BT_CONNECT;
@@ -6973,10 +6983,14 @@ static int hci_acl_create_conn_sync(struct hci_dev *hdev, void *data)
 	else
 		cp.role_switch = 0x00;
 
+	timeout = conn->conn_timeout;
+
+	hci_dev_unlock_sync(hdev);
+
 	return __hci_cmd_sync_status_sk(hdev, HCI_OP_CREATE_CONN,
 					sizeof(cp), &cp,
 					HCI_EV_CONN_COMPLETE,
-					conn->conn_timeout, NULL);
+					timeout, NULL);
 }
 
 static void hci_acl_create_conn_sync_complete(struct hci_dev *hdev, void *data,
-- 
2.51.0

[RFC PATCH 19/24] Bluetooth: hci_sync: hold lock in hci_le_create_conn_sync

[Pauli Virtanen]

Hold lock in hci_le_create_conn_sync() and after every wait operation,
check the conn is still valid.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_sync.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index f41aec54659b..3f0f9d9d8071 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -6594,8 +6594,12 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
 	int err;
 	struct hci_conn *conn = data;
 
-	if (!hci_conn_valid(hdev, conn))
+	hci_dev_lock_sync(hdev);
+
+	if (!hci_conn_valid(hdev, conn)) {
+		hci_dev_unlock_sync(hdev);
 		return -ECANCELED;
+	}
 
 	bt_dev_dbg(hdev, "conn %p", conn);
 
@@ -6610,10 +6614,8 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
 		if (hci_dev_test_flag(hdev, HCI_LE_SCAN) &&
 		    hdev->le_scan_type == LE_SCAN_ACTIVE &&
 		    !hci_dev_test_flag(hdev, HCI_LE_SIMULTANEOUS_ROLES)) {
-			hci_dev_lock(hdev);
-			if (hci_conn_valid(hdev, conn))
-				hci_conn_del(conn);
-			hci_dev_unlock(hdev);
+			hci_conn_del(conn);
+			hci_dev_unlock_sync(hdev);
 			return -EBUSY;
 		}
 
@@ -6657,6 +6659,8 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
 	 */
 	err = hci_update_random_address_sync(hdev, false, conn_use_rpa(conn),
 					     &own_addr_type);
+	if (!err && !hci_conn_valid(hdev, conn))
+		err = -ECANCELED;
 	if (err)
 		goto done;
 	/* Send command LE Extended Create Connection if supported */
@@ -6694,11 +6698,13 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
 				       conn->conn_timeout, NULL);
 
 done:
-	if (err == -ETIMEDOUT)
+	if (err == -ETIMEDOUT && hci_conn_valid(hdev, conn))
 		hci_le_connect_cancel_sync(hdev, conn, 0x00);
 
 	/* Re-enable advertising after the connection attempt is finished. */
 	hci_resume_advertising_sync(hdev);
+
+	hci_dev_unlock_sync(hdev);
 	return err;
 }
 
-- 
2.51.0

[RFC PATCH 20/24] Bluetooth: hci_sync: add hdev lock lockdep asserts in subroutines

[Pauli Virtanen]

Subroutines taking hci_conn* arguments require hci_dev_lock_sync() is
held.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_sync.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 3f0f9d9d8071..f4d984cee655 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5575,6 +5575,8 @@ static int hci_disconnect_sync(struct hci_dev *hdev, struct hci_conn *conn,
 {
 	struct hci_cp_disconnect cp;
 
+	hci_assert_lock_sync_held(hdev);
+
 	if (conn->type == BIS_LINK || conn->type == PA_LINK) {
 		/* This is a BIS connection, hci_conn_del will
 		 * do the necessary cleanup.
@@ -5608,6 +5610,8 @@ static int hci_disconnect_sync(struct hci_dev *hdev, struct hci_conn *conn,
 static int hci_le_connect_cancel_sync(struct hci_dev *hdev,
 				      struct hci_conn *conn, u8 reason)
 {
+	hci_assert_lock_sync_held(hdev);
+
 	/* Return reason if scanning since the connection shall probably be
 	 * cleanup directly.
 	 */
@@ -5625,6 +5629,8 @@ static int hci_le_connect_cancel_sync(struct hci_dev *hdev,
 static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn,
 				   u8 reason)
 {
+	hci_assert_lock_sync_held(hdev);
+
 	if (conn->type == LE_LINK)
 		return hci_le_connect_cancel_sync(hdev, conn, reason);
 
@@ -5674,6 +5680,8 @@ static int hci_reject_sco_sync(struct hci_dev *hdev, struct hci_conn *conn,
 {
 	struct hci_cp_reject_sync_conn_req cp;
 
+	hci_assert_lock_sync_held(hdev);
+
 	memset(&cp, 0, sizeof(cp));
 	bacpy(&cp.bdaddr, &conn->dst);
 	cp.reason = reason;
@@ -5693,6 +5701,8 @@ static int hci_le_reject_cis_sync(struct hci_dev *hdev, struct hci_conn *conn,
 {
 	struct hci_cp_le_reject_cis cp;
 
+	hci_assert_lock_sync_held(hdev);
+
 	memset(&cp, 0, sizeof(cp));
 	cp.handle = cpu_to_le16(conn->handle);
 	cp.reason = reason;
@@ -5706,6 +5716,8 @@ static int hci_reject_conn_sync(struct hci_dev *hdev, struct hci_conn *conn,
 {
 	struct hci_cp_reject_conn_req cp;
 
+	hci_assert_lock_sync_held(hdev);
+
 	if (conn->type == CIS_LINK)
 		return hci_le_reject_cis_sync(hdev, conn, reason);
 
@@ -5730,6 +5742,8 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
 	bool disconnect = false;
 	struct hci_conn *c;
 
+	hci_assert_lock_sync_held(hdev);
+
 	switch (conn->state) {
 	case BT_CONNECTED:
 	case BT_CONFIG:
@@ -6407,6 +6421,8 @@ static int hci_le_ext_directed_advertising_sync(struct hci_dev *hdev,
 	bdaddr_t random_addr;
 	u8 own_addr_type;
 
+	hci_assert_lock_sync_held(hdev);
+
 	err = hci_update_random_address_sync(hdev, false, conn_use_rpa(conn),
 					     &own_addr_type);
 	if (err)
@@ -6474,6 +6490,8 @@ static int hci_le_directed_advertising_sync(struct hci_dev *hdev,
 	u8 own_addr_type;
 	u8 enable;
 
+	hci_assert_lock_sync_held(hdev);
+
 	if (ext_adv_capable(hdev))
 		return hci_le_ext_directed_advertising_sync(hdev, conn);
 
@@ -6543,6 +6561,8 @@ static int hci_le_ext_create_conn_sync(struct hci_dev *hdev,
 	u8 data[sizeof(*cp) + sizeof(*p) * 3];
 	u32 plen;
 
+	hci_assert_lock_sync_held(hdev);
+
 	cp = (void *)data;
 	p = (void *)cp->data;
 
-- 
2.51.0

[RFC PATCH 21/24] Bluetooth: fix locking for hci_abort_conn_sync()

[Pauli Virtanen]

The function should hold hdev->lock except when waiting for events, to
avoid hci_conn* races.

Move locking to callers, which shall hold hci_dev_lock_sync()

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_conn.c | 13 +++++++++++--
 net/bluetooth/hci_sync.c | 42 +++++++++++++++-------------------------
 2 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b6c3ee60024f..b39fb6843fad 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -2926,11 +2926,20 @@ u32 hci_conn_get_phy(struct hci_conn *conn)
 static int abort_conn_sync(struct hci_dev *hdev, void *data)
 {
 	struct hci_conn *conn = data;
+	int err;
 
-	if (!hci_conn_valid(hdev, conn))
+	hci_dev_lock_sync(hdev);
+
+	if (!hci_conn_valid(hdev, conn)) {
+		hci_dev_unlock_sync(hdev);
 		return -ECANCELED;
+	}
 
-	return hci_abort_conn_sync(hdev, conn, conn->abort_reason);
+	err = hci_abort_conn_sync(hdev, conn, conn->abort_reason);
+
+	hci_dev_unlock_sync(hdev);
+
+	return err;
 }
 
 static void abort_conn_destroy(struct hci_dev *hdev, void *data, int err)
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index f4d984cee655..2d79667f16cd 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5738,12 +5738,12 @@ static int hci_reject_conn_sync(struct hci_dev *hdev, struct hci_conn *conn,
 int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
 {
 	int err = 0;
-	u16 handle = conn->handle;
 	bool disconnect = false;
-	struct hci_conn *c;
 
 	hci_assert_lock_sync_held(hdev);
 
+	hci_conn_get(conn);
+
 	switch (conn->state) {
 	case BT_CONNECTED:
 	case BT_CONFIG:
@@ -5763,30 +5763,23 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
 		break;
 	}
 
-	hci_dev_lock(hdev);
-
-	/* Check if the connection has been cleaned up concurrently */
-	c = hci_conn_hash_lookup_handle(hdev, handle);
-	if (!c || c != conn) {
-		err = 0;
-		goto unlock;
-	}
-
 	/* Cleanup hci_conn object if it cannot be cancelled as it
 	 * likely means the controller and host stack are out of sync
 	 * or in case of LE it was still scanning so it can be cleanup
 	 * safely.
 	 */
-	if (disconnect) {
-		conn->state = BT_CLOSED;
-		hci_disconn_cfm(conn, reason);
-		hci_conn_del(conn);
-	} else {
-		hci_conn_failed(conn, reason);
+	if (hci_conn_valid(hdev, conn)) {
+		if (disconnect) {
+			conn->state = BT_CLOSED;
+			hci_disconn_cfm(conn, reason);
+			hci_conn_del(conn);
+		} else {
+			hci_conn_failed(conn, reason);
+		}
 	}
 
-unlock:
-	hci_dev_unlock(hdev);
+	hci_conn_put(conn);
+
 	return err;
 }
 
@@ -5795,21 +5788,18 @@ static int hci_disconnect_all_sync(struct hci_dev *hdev, u8 reason)
 	struct list_head *head = &hdev->conn_hash.list;
 	struct hci_conn *conn;
 
-	rcu_read_lock();
+	hci_dev_lock_sync(hdev);
+
 	while ((conn = list_first_or_null_rcu(head, struct hci_conn, list))) {
-		/* Make sure the connection is not freed while unlocking */
-		conn = hci_conn_get(conn);
-		rcu_read_unlock();
 		/* Disregard possible errors since hci_conn_del shall have been
 		 * called even in case of errors had occurred since it would
 		 * then cause hci_conn_failed to be called which calls
 		 * hci_conn_del internally.
 		 */
 		hci_abort_conn_sync(hdev, conn, reason);
-		hci_conn_put(conn);
-		rcu_read_lock();
 	}
-	rcu_read_unlock();
+
+	hci_dev_unlock_sync(hdev);
 
 	return 0;
 }
-- 
2.51.0

[RFC PATCH 22/24] Bluetooth: hci_sync: lock properly in hci_le_pa/big_create_sync

[Pauli Virtanen]

hci_dev_lock_sync() should be held while accessing hci_conn, and its
validity needs to be checked after any waiting subroutines.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_sync.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 2d79667f16cd..0717d53c2e33 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -7153,14 +7153,22 @@ static int hci_le_pa_create_sync(struct hci_dev *hdev, void *data)
 	struct bt_iso_qos *qos = &conn->iso_qos;
 	int err;
 
-	if (!hci_conn_valid(hdev, conn))
+	hci_dev_lock_sync(hdev);
+
+	if (!hci_conn_valid(hdev, conn)) {
+		hci_dev_unlock_sync(hdev);
 		return -ECANCELED;
+	}
 
-	if (conn->sync_handle != HCI_SYNC_HANDLE_INVALID)
+	if (conn->sync_handle != HCI_SYNC_HANDLE_INVALID) {
+		hci_dev_unlock_sync(hdev);
 		return -EINVAL;
+	}
 
-	if (hci_dev_test_and_set_flag(hdev, HCI_PA_SYNC))
+	if (hci_dev_test_and_set_flag(hdev, HCI_PA_SYNC)) {
+		hci_dev_unlock_sync(hdev);
 		return -EBUSY;
+	}
 
 	/* Stop scanning if SID has not been set and active scanning is enabled
 	 * so we use passive scanning which will be scanning using the allow
@@ -7173,6 +7181,11 @@ static int hci_le_pa_create_sync(struct hci_dev *hdev, void *data)
 		hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
 	}
 
+	if (!hci_conn_valid(hdev, conn)) {
+		err = -ECANCELED;
+		goto done;
+	}
+
 	/* Mark HCI_CONN_CREATE_PA_SYNC so hci_update_passive_scan_sync can
 	 * program the address in the allow list so PA advertisements can be
 	 * received.
@@ -7181,6 +7194,11 @@ static int hci_le_pa_create_sync(struct hci_dev *hdev, void *data)
 
 	hci_update_passive_scan_sync(hdev);
 
+	if (!hci_conn_valid(hdev, conn)) {
+		err = -ECANCELED;
+		goto done;
+	}
+
 	/* SID has not been set listen for HCI_EV_LE_EXT_ADV_REPORT to update
 	 * it.
 	 */
@@ -7192,6 +7210,11 @@ static int hci_le_pa_create_sync(struct hci_dev *hdev, void *data)
 			goto done;
 	}
 
+	if (!hci_conn_valid(hdev, conn)) {
+		err = -ECANCELED;
+		goto done;
+	}
+
 	memset(&cp, 0, sizeof(cp));
 	cp.options = qos->bcast.options;
 	cp.sid = conn->sid;
@@ -7221,6 +7244,8 @@ static int hci_le_pa_create_sync(struct hci_dev *hdev, void *data)
 				      0, NULL, HCI_CMD_TIMEOUT);
 
 done:
+	hci_dev_unlock_sync(hdev);
+
 	hci_dev_clear_flag(hdev, HCI_PA_SYNC);
 
 	/* Update passive scan since HCI_PA_SYNC flag has been cleared */
@@ -7268,8 +7293,12 @@ static int hci_le_big_create_sync(struct hci_dev *hdev, void *data)
 	struct bt_iso_qos *qos = &conn->iso_qos;
 	int err;
 
-	if (!hci_conn_valid(hdev, conn))
+	hci_dev_lock_sync(hdev);
+
+	if (!hci_conn_valid(hdev, conn)) {
+		hci_dev_unlock_sync(hdev);
 		return -ECANCELED;
+	}
 
 	set_bit(HCI_CONN_CREATE_BIG_SYNC, &conn->flags);
 
@@ -7302,6 +7331,8 @@ static int hci_le_big_create_sync(struct hci_dev *hdev, void *data)
 	if (err == -ETIMEDOUT)
 		hci_le_big_terminate_sync(hdev, cp->handle);
 
+	hci_dev_unlock_sync(hdev);
+
 	return err;
 }
 
-- 
2.51.0

[RFC PATCH 23/24] Bluetooth: hci_sync: fix locking in hci_disconnect_sync

[Pauli Virtanen]

Caller of hci_disconnect_sync() shall hold hdev->lock.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_sync.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 0717d53c2e33..a07bf168594a 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5581,10 +5581,7 @@ static int hci_disconnect_sync(struct hci_dev *hdev, struct hci_conn *conn,
 		/* This is a BIS connection, hci_conn_del will
 		 * do the necessary cleanup.
 		 */
-		hci_dev_lock(hdev);
 		hci_conn_failed(conn, reason);
-		hci_dev_unlock(hdev);
-
 		return 0;
 	}
 
-- 
2.51.0

[RFC PATCH 24/24] Bluetooth: hci_conn: fix ABA and locking in hci_enhanced_setup_sync

[Pauli Virtanen]

hci_conn_valid() shall not be used on potentially freed conns, otherwise
that is ABA problem. Avoid that by holding reference.

hci_enhanced_setup_sync() also modifies hci_conn status fields, and
should hold hdev->lock during that.

Also fix wrong "return false" when "return -EINVAL" appears to have been
intended.

Fixes: 881559af5f5c5 ("Bluetooth: hci_sync: Attempt to dequeue connection attempt")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/hci_conn.c | 48 +++++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b39fb6843fad..cc1f674c743b 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -286,15 +286,22 @@ static int hci_enhanced_setup_sync(struct hci_dev *hdev, void *data)
 	struct hci_cp_enhanced_setup_sync_conn cp;
 	const struct sco_param *param;
 
-	kfree(conn_handle);
+	hci_dev_lock_sync(hdev);
 
-	if (!hci_conn_valid(hdev, conn))
+	if (!hci_conn_valid(hdev, conn)) {
+		hci_dev_unlock_sync(hdev);
 		return -ECANCELED;
+	}
 
 	bt_dev_dbg(hdev, "hcon %p", conn);
 
 	configure_datapath_sync(hdev, &conn->codec);
 
+	if (!hci_conn_valid(hdev, conn)) {
+		hci_dev_unlock_sync(hdev);
+		return -ECANCELED;
+	}
+
 	conn->state = BT_CONNECT;
 	conn->out = true;
 
@@ -311,7 +318,7 @@ static int hci_enhanced_setup_sync(struct hci_dev *hdev, void *data)
 	case BT_CODEC_MSBC:
 		if (!find_next_esco_param(conn, esco_param_msbc,
 					  ARRAY_SIZE(esco_param_msbc)))
-			return -EINVAL;
+			goto error_invalid;
 
 		param = &esco_param_msbc[conn->attempt - 1];
 		cp.tx_coding_format.id = 0x05;
@@ -337,8 +344,7 @@ static int hci_enhanced_setup_sync(struct hci_dev *hdev, void *data)
 	case BT_CODEC_TRANSPARENT:
 		if (!find_next_esco_param(conn, esco_param_msbc,
 					  ARRAY_SIZE(esco_param_msbc)))
-			return -EINVAL;
-
+			goto error_invalid;
 		param = &esco_param_msbc[conn->attempt - 1];
 		cp.tx_coding_format.id = 0x03;
 		cp.rx_coding_format.id = 0x03;
@@ -364,11 +370,11 @@ static int hci_enhanced_setup_sync(struct hci_dev *hdev, void *data)
 		if (conn->parent && lmp_esco_capable(conn->parent)) {
 			if (!find_next_esco_param(conn, esco_param_cvsd,
 						  ARRAY_SIZE(esco_param_cvsd)))
-				return -EINVAL;
+				goto error_invalid;
 			param = &esco_param_cvsd[conn->attempt - 1];
 		} else {
 			if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
-				return -EINVAL;
+				goto error_invalid;
 			param = &sco_param_cvsd[conn->attempt - 1];
 		}
 		cp.tx_coding_format.id = 2;
@@ -391,17 +397,33 @@ static int hci_enhanced_setup_sync(struct hci_dev *hdev, void *data)
 		cp.out_transport_unit_size = 16;
 		break;
 	default:
-		return -EINVAL;
+		goto error_invalid;
 	}
 
 	cp.retrans_effort = param->retrans_effort;
 	cp.pkt_type = __cpu_to_le16(param->pkt_type);
 	cp.max_latency = __cpu_to_le16(param->max_latency);
 
+	hci_dev_unlock_sync(hdev);
+
 	if (hci_send_cmd(hdev, HCI_OP_ENHANCED_SETUP_SYNC_CONN, sizeof(cp), &cp) < 0)
 		return -EIO;
 
 	return 0;
+
+error_invalid:
+	hci_dev_unlock_sync(hdev);
+	return -EINVAL;
+}
+
+static void hci_enhanced_setup_sync_destroy(struct hci_dev *hdev, void *data,
+					    int err)
+{
+	struct conn_handle_t *conn_handle = data;
+	struct hci_conn *conn = conn_handle->conn;
+
+	hci_conn_put(conn);
+	kfree(conn_handle);
 }
 
 static bool hci_setup_sync_conn(struct hci_conn *conn, __u16 handle)
@@ -467,12 +489,16 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
 		if (!conn_handle)
 			return false;
 
-		conn_handle->conn = conn;
+		conn_handle->conn = hci_conn_get(conn);
 		conn_handle->handle = handle;
+
 		result = hci_cmd_sync_queue(conn->hdev, hci_enhanced_setup_sync,
-					    conn_handle, NULL);
-		if (result < 0)
+					    conn_handle,
+					    hci_enhanced_setup_sync_destroy);
+		if (result < 0) {
+			hci_conn_put(conn);
 			kfree(conn_handle);
+		}
 
 		return result == 0;
 	}
-- 
2.51.0

Re: [RFC PATCH 00/24] Bluetooth: add locks to hci_conn accesses

[patchwork-bot+bluetooth]

Hello:

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

On Sun, 21 Sep 2025 22:14:15 +0300 you wrote:
> (RFC since this needs to be tested much better.)
> 
> Each hdev has two ordered workqueues that run in parallel, in addition
> to user tasks and some timers in global workqueues.
> 
> Both workqueues may delete hci_conn* and modify their state. The current
> situation is there are races and UAF due to this.  In older kernels, it
> used to be much of the work was done from a single ordered
> hdev->workqueue, so one could be more lax with locking.  I don't think
> what used to be safe earlier is necessarily so now, so some simple rules
> are probably needed.
> 
> [...]

Here is the summary with links:
  - [RFC,01/24] Bluetooth: ISO: free rx_skb if not consumed
    https://git.kernel.org/bluetooth/bluetooth-next/c/b18365fe359a
  - [RFC,02/24] Bluetooth: ISO: don't leak skb in ISO_CONT RX
    https://git.kernel.org/bluetooth/bluetooth-next/c/f7db34762ae5
  - [RFC,03/24] Bluetooth: hci_sync: make hci_cmd_sync_run* indicate if item was added
    (no matching commit)
  - [RFC,04/24] Bluetooth: hci_sync: hci_cmd_sync_queue_once() return -EEXIST if exists
    (no matching commit)
  - [RFC,05/24] Bluetooth: hci_conn: avoid ABA error in abort_conn_sync
    (no matching commit)
  - [RFC,06/24] Bluetooth: hci_sync: avoid ABA/UAF in hci_sync callbacks
    (no matching commit)
  - [RFC,07/24] Bluetooth: hci_event: extend conn_hash lookup RCU critical sections
    (no matching commit)
  - [RFC,08/24] Bluetooth: hci_sync: extend conn_hash lookup RCU critical sections
    (no matching commit)
  - [RFC,09/24] Bluetooth: mgmt: extend conn_hash lookup RCU critical sections
    (no matching commit)
  - [RFC,10/24] Bluetooth: hci_conn: extend conn_hash lookup RCU critical sections
    (no matching commit)
  - [RFC,11/24] Bluetooth: hci_core: add lockdep check to hci_conn_hash lookups
    (no matching commit)
  - [RFC,12/24] Bluetooth: hci_core: add lockdep check to hci_conn_valid()
    (no matching commit)
  - [RFC,13/24] Bluetooth: hci_sync: fix hdev locking in hci_le_create_conn_sync
    (no matching commit)
  - [RFC,14/24] Bluetooth: hci_core: hold hdev lock in packet TX scheduler
    (no matching commit)
  - [RFC,15/24] Bluetooth: lookup hci_conn on RX path on protocol side
    (no matching commit)
  - [RFC,16/24] Bluetooth: L2CAP: fix hci_conn_valid() usage
    (no matching commit)
  - [RFC,17/24] Bluetooth: hci_sync: add helper for hdev locking across waits
    (no matching commit)
  - [RFC,18/24] Bluetooth: hci_sync: hold lock in hci_acl_create_conn_sync
    (no matching commit)
  - [RFC,19/24] Bluetooth: hci_sync: hold lock in hci_le_create_conn_sync
    (no matching commit)
  - [RFC,20/24] Bluetooth: hci_sync: add hdev lock lockdep asserts in subroutines
    (no matching commit)
  - [RFC,21/24] Bluetooth: fix locking for hci_abort_conn_sync()
    (no matching commit)
  - [RFC,22/24] Bluetooth: hci_sync: lock properly in hci_le_pa/big_create_sync
    (no matching commit)
  - [RFC,23/24] Bluetooth: hci_sync: fix locking in hci_disconnect_sync
    (no matching commit)
  - [RFC,24/24] Bluetooth: hci_conn: fix ABA and locking in hci_enhanced_setup_sync
    (no matching commit)

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