Commit 1d80d57f authored by Ying Hsu's avatar Ying Hsu Committed by Luiz Augusto von Dentz
Browse files

Bluetooth: Fix possible deadlock in rfcomm_sk_state_change 



syzbot reports a possible deadlock in rfcomm_sk_state_change [1].
While rfcomm_sock_connect acquires the sk lock and waits for
the rfcomm lock, rfcomm_sock_release could have the rfcomm
lock and hit a deadlock for acquiring the sk lock.
Here's a simplified flow:

rfcomm_sock_connect:
  lock_sock(sk)
  rfcomm_dlc_open:
    rfcomm_lock()

rfcomm_sock_release:
  rfcomm_sock_shutdown:
    rfcomm_lock()
    __rfcomm_dlc_close:
        rfcomm_k_state_change:
	  lock_sock(sk)

This patch drops the sk lock before calling rfcomm_dlc_open to
avoid the possible deadlock and holds sk's reference count to
prevent use-after-free after rfcomm_dlc_open completes.

Reported-by: default avatar <syzbot+d7ce59...@syzkaller.appspotmail.com>
Fixes: 1804fdf6 ("Bluetooth: btintel: Combine setting up MSFT extension")
Link: https://syzkaller.appspot.com/bug?extid=d7ce59b06b3eb14fd218

 [1]

Signed-off-by: default avatarYing Hsu <yinghsu@chromium.org>
Signed-off-by: default avatarLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
parent 506d9b40

net/bluetooth/rfcomm/sock.c

+6 −1
Original line number Diff line number Diff line
@@ -391,6 +391,7 @@ static int rfcomm_sock_connect(struct socket *sock, struct sockaddr *addr, int a 
	    addr->sa_family != AF_BLUETOOTH)

		return -EINVAL;



	sock_hold(sk);

	lock_sock(sk);



	if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND) {
@@ -410,14 +411,18 @@ static int rfcomm_sock_connect(struct socket *sock, struct sockaddr *addr, int a 
	d->sec_level = rfcomm_pi(sk)->sec_level;

	d->role_switch = rfcomm_pi(sk)->role_switch;



	/* Drop sock lock to avoid potential deadlock with the RFCOMM lock */

	release_sock(sk);

	err = rfcomm_dlc_open(d, &rfcomm_pi(sk)->src, &sa->rc_bdaddr,

			      sa->rc_channel);

	if (!err)

	lock_sock(sk);

	if (!err && !sock_flag(sk, SOCK_ZAPPED))

		err = bt_sock_wait_state(sk, BT_CONNECTED,

				sock_sndtimeo(sk, flags & O_NONBLOCK));



done:

	release_sock(sk);

	sock_put(sk);

	return err;

}