Bluetooth: L2CAP: use chan timer to close channels in cleanup_listen()

l2cap_chan_close() removes the channel from conn->chan_l, which
must be done under conn->lock.  cleanup_listen() runs under the
parent sk_lock, so acquiring conn->lock would invert the
established conn->lock -> chan->lock -> sk_lock order.

Instead of calling l2cap_chan_close() directly, schedule
l2cap_chan_timeout with delay 0 to close the channel
asynchronously.  The timeout handler already acquires conn->lock
and chan->lock in the correct order.

The timer is only armed when chan->conn is still set: if it is
already NULL, l2cap_conn_del() has already processed this channel
(l2cap_chan_del + l2cap_sock_teardown_cb + l2cap_sock_close_cb),
so there is nothing left to do.  If l2cap_conn_del() races in
after the timer is armed, __clear_chan_timer() inside
l2cap_chan_del() cancels it; if the timer has already fired, the
handler returns harmlessly because chan->conn was cleared.

Fixes: 3df91ea20e74 ("Bluetooth: Revert to mutexes from RCU list")
Cc: <stable@vger.kernel.org> # 0b58004: Bluetooth: fix UAF in l2cap_sock_cleanup_listen() vs l2cap_conn_del()
Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index b34e7da8d90672150859ea2035f47fd26d98a329..c138aa4ae26690277b812a376835872aea7e3ddf 100644 (file)
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1499,6 +1499,10 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
         * 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.
+        *
+        * Since we cannot call l2cap_chan_close() without conn->lock,
+        * schedule l2cap_chan_timeout to close the channel; it already
+        * acquires conn->lock -> chan->lock in the correct order.
         */
        while ((sk = bt_accept_dequeue(parent, NULL))) {
                struct l2cap_chan *chan;
@@ -1516,14 +1520,12 @@ static void l2cap_sock_cleanup_listen(struct sock *parent)
                       state_to_string(chan->state));
 
                l2cap_chan_lock(chan);
-               __clear_chan_timer(chan);
-               l2cap_chan_close(chan, ECONNRESET);
-               /* l2cap_conn_del() may already have killed this socket
-                * (it sets SOCK_DEAD); skip the duplicate to avoid a
-                * double sock_put()/l2cap_chan_put().
+               /* Since we cannot call l2cap_chan_close() without
+                * conn->lock, schedule its timer to trigger the close
+                * and cleanup of this channel.
                 */
-               if (!sock_flag(sk, SOCK_DEAD))
-                       l2cap_sock_kill(sk);
+               if (chan->conn)
+                       __set_chan_timer(chan, 0);
                l2cap_chan_unlock(chan);
 
                l2cap_chan_put(chan);