Commit 8c8e6204 authored by Siwei Zhang's avatar Siwei Zhang Committed by Luiz Augusto von Dentz
Browse files

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: 3df91ea2 ("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: default avatarSiwei Zhang <oss@fourdim.xyz>
Signed-off-by: default avatarLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
parent 9dbd8499

net/bluetooth/l2cap_sock.c

+9 −7
Original line number Diff line number Diff line
@@ -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);