diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index ef6ce1c20a4f..3d9a32094347 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -699,7 +699,12 @@ struct l2cap_rx_busy { struct l2cap_pinfo { struct bt_sock bt; + + /* With owning sk_socket chan may be read without lock, other access + * should hold lock_sock. + */ struct l2cap_chan *chan; + struct list_head rx_busy; }; diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index 735167f73f31..9540617a0e6c 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1312,7 +1312,12 @@ static void l2cap_sock_kill(struct sock *sk) BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state)); + /* Take lock to synchronize against access without owning sk->sk_socket, + * eg. in l2cap_sock_cleanup_listen(). proto_ops etc. don't need lock. + */ + lock_sock(sk); l2cap_sock_put_chan(sk); + release_sock(sk); /* Kill poor orphan */ sock_set_flag(sk, SOCK_DEAD); @@ -1516,14 +1521,10 @@ static void l2cap_sock_cleanup_listen(struct sock *parent) * establish sk_lock -> conn->lock and invert the established * conn->lock -> chan->lock -> sk_lock order (lockdep deadlock). * - * Instead, briefly take the child sk lock to fetch and pin its chan. - * l2cap_conn_del() reaches the chan free only via - * l2cap_chan_del() -> l2cap_sock_teardown_cb(), which itself takes - * the child sk lock; holding it across l2cap_chan_hold_unless_zero() - * therefore guarantees the chan cannot be freed while we read and - * pin it (hold_unless_zero() additionally skips a chan already past - * its last reference). We then drop the sk lock before taking - * chan->lock, so sk and chan locks are never held together. + * Instead, briefly take the child sk lock to synchronize vs. + * l2cap_sock_kill that puts l2cap_pi(sk)->chan. We then drop the sk + * lock before taking chan->lock, so sk and chan locks are never held + * together. * * Since we cannot call l2cap_chan_close() without conn->lock, * schedule l2cap_chan_timeout to close the channel; it already @@ -1533,10 +1534,12 @@ static void l2cap_sock_cleanup_listen(struct sock *parent) struct l2cap_chan *chan; lock_sock_nested(sk, L2CAP_NESTING_NORMAL); - chan = l2cap_chan_hold_unless_zero(l2cap_pi(sk)->chan); + chan = l2cap_pi(sk)->chan; + if (chan) + l2cap_chan_hold(chan); release_sock(sk); if (!chan) { - /* l2cap_conn_del() already tearing this child down */ + /* Already torn down */ sock_put(sk); continue; }