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

[Pauli Virtanen <pav@iki.fi>]

(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