From: Siwei Zhang <oss@fourdim.xyz> stable inclusion from stable-v5.10.259 commit 3634cbdc2eb414b69ffa752ddbe5e0458518e321 category: bugfix bugzilla: NA Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=... -------------------------------- [ Upstream commit 8c8e620467a7b51562dbcefbd1f09f288d7d710d ] 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> Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Chen Jinghuang <chenjinghuang2@huawei.com> --- net/bluetooth/l2cap_sock.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index 644ec4bf06f0..4b4a9268f007 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1441,6 +1441,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; @@ -1458,14 +1462,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); -- 2.34.1