[PATCH v2 0/8] Bluetooth: avoid concurrent deletion of hci_conn

[Pauli Virtanen <pav@iki.fi>]

This contains the simpler fixes from
https://lore.kernel.org/linux-bluetooth/cover.1758481869.git.pav@iki.fi/

Changes:
  v2:
  - Fix also hci_past_sync()
  - Drop "Bluetooth: L2CAP: fix hci_conn_valid() usage".
    This does not fix the race completely, as conn->hchan also
    can race, so leave for later.

hdev has two workqueues that run concurrently, and both may delete
hci_conn. hci_conn* pointers then require either (i) hdev/rcu lock
covering lookup and usage, (ii) hci_conn_get reference held, or (iii)
other more complex invariant.

If none applies, it's likely there are corner cases that hit UAF,
especially if controller misbehaves.

Trying to protect access with "other invariants" is quite error prone,
and I don't think there are such in most of this series.

Correct code in several places to follow the patterns (1)

    take lock
    conn = hci_conn_hash_lookup(...)
    if (conn)
	do_something(conn)
    release lock

or (2)

    take lock
    conn = hci_conn_hash_lookup(...)
    if (conn)
	conn = hci_conn_get(conn)
    release lock
    do_something_carefully(conn)
    hci_conn_put(conn)

Generally do_something_carefully should do (3)

    take lock
    if (hci_conn_valid(hdev, conn))
	do_something(conn)
    release lock

hci_conn_valid() shouldn't be called unless refcount on conn is known to
be held, as the pointer may otherwise already be freed, and even though
hci_conn_valid() doesn't dereference the comparison of freed pointer it
does is strictly speaking undefined behavior (kmalloc is not guaranteed
to not reuse addresses).

Some of the existing code is missing locks for (3), those need to be
addressed in separate series.

Pauli Virtanen (8):
  Bluetooth: hci_event: extend hdev lock in
    hci_le_remote_conn_param_req_evt
  Bluetooth: hci_conn: take hdev lock in set_cig_params_sync
  Bluetooth: mgmt: take lock and hold reference when handling hci_conn
  Bluetooth: hci_sync: extend conn_hash lookup RCU critical sections
  Bluetooth: hci_sync: make hci_cmd_sync_run* return -EEXIST if not run
  Bluetooth: hci_sync: hci_cmd_sync_queue_once() return -EEXIST if
    exists
  Bluetooth: hci_conn: hold reference in abort_conn_sync
  Bluetooth: hci_sync: hold references in hci_sync callbacks

 net/bluetooth/hci_conn.c  |  22 +++++-
 net/bluetooth/hci_event.c |  33 ++++----
 net/bluetooth/hci_sync.c  | 157 ++++++++++++++++++++++++++++++--------
 net/bluetooth/mgmt.c      |  42 +++++++++-
 4 files changed, 204 insertions(+), 50 deletions(-)

-- 
2.51.1