From d48a797364306a5c4a3d378204d56facf5233fa9 Mon Sep 17 00:00:00 2001
From: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
Date: Sat, 31 Jul 2021 00:01:34 +0800
Subject: [PATCH] Squashed commit of the following:

commit e5a80b069fa91e8a431d8ae107b130fcd7d39949
Author: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
Date:   Fri Jul 30 16:27:50 2021 +0800

    Bluetooth: lock sock

commit 792685101a07485c8058e450fb223fa53f9b37bc
Author: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
Date:   Fri Jul 30 16:26:32 2021 +0800

    aBluetooth: break deadlocks in SCO

commit c56245205746129ea94e57250aabebc3c86e0d10
Author: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
Date:   Tue Jul 27 21:32:19 2021 +0800

    Bluetooth: schedule SCO timeouts with delayed_work

    struct sock.sk_timer should be used as a sock cleanup timer. However,
    SCO uses it to implement sock timeouts.

    This causes issues because struct sock.sk_timer's callback is run in
    an IRQ context, and the timer callback function sco_sock_timeout takes
    a spin lock on the socket. However, other functions such as
    sco_conn_del, sco_conn_ready, rfcomm_connect_ind, and
    bt_accept_enqueue also take the spin lock with interrupts enabled.

    This inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} lock usage could
    lead to deadlocks as reported by Syzbot [1]:
           CPU0
           ----
      lock(slock-AF_BLUETOOTH-BTPROTO_SCO);
      <Interrupt>
        lock(slock-AF_BLUETOOTH-BTPROTO_SCO);

    To fix this, we use delayed work to implement SCO sock timouts
    instead. This allows us to avoid taking the spin lock on the socket in
    an IRQ context, and corrects the misuse of struct sock.sk_timer.

    Additionally, we have to move the call to sco_sock_clear_timer out of
    the bh_lock_sock critical section in sco_conn_del since
    sco_sock_clear_timer might sleep. This doesn't create any new races
    since sco_sock_set_timer was protected by lock_sock, and not
    bh_lock_sock, which are incompatible.

    Link: https://syzkaller.appspot.com/bug?id=9089d89de0502e120f234ca0fc8a703f7368b31e [1]
    Reported-by: syzbot+2f6d7c28bb4bf7e82060@syzkaller.appspotmail.com
    Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>

commit 711500b46fcc213aa13d37aaabb4b46df7a8d113
Author: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
Date:   Thu Jun 24 19:13:01 2021 +0800

    Bluetooth: skip invalid hci_sync_conn_complete_evt

    Syzbot reported a corrupted list in kobject_add_internal [1]. This
    happens when multiple HCI_EV_SYNC_CONN_COMPLETE event packets with
    status 0 are sent for the same HCI connection. This causes us to
    register the device more than once which corrupts the kset list.

    As this is forbidden behavior, we add a check for whether we're
    trying to process the same HCI_EV_SYNC_CONN_COMPLETE event multiple
    times for one connection. If that's the case, the event is invalid, so
    we report an error that the device is misbehaving, and ignore the
    packet.

    Link: https://syzkaller.appspot.com/bug?extid=66264bf2fd0476be7e6c [1]
    Reported-by: syzbot+66264bf2fd0476be7e6c@syzkaller.appspotmail.com
    Tested-by: syzbot+66264bf2fd0476be7e6c@syzkaller.appspotmail.com

Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>
---
 net/bluetooth/sco.c | 88 +++++++++++++++++++++++++--------------------
 1 file changed, 49 insertions(+), 39 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d9a4e88dacbb..16e7436a6c13 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -48,6 +48,8 @@ struct sco_conn {
 	spinlock_t	lock;
 	struct sock	*sk;
 
+	struct delayed_work	timeout_work;
+
 	unsigned int    mtu;
 };
 
@@ -74,16 +76,18 @@ struct sco_pinfo {
 #define SCO_CONN_TIMEOUT	(HZ * 40)
 #define SCO_DISCONN_TIMEOUT	(HZ * 2)
 
-static void sco_sock_timeout(struct timer_list *t)
+static void sco_sock_timeout(struct work_struct *work)
 {
-	struct sock *sk = from_timer(sk, t, sk_timer);
+	struct sco_conn *conn = container_of(work, struct sco_conn,
+					     timeout_work.work);
+	struct sock *sk = conn->sk;
 
 	BT_DBG("sock %p state %d", sk, sk->sk_state);
 
-	bh_lock_sock(sk);
+	lock_sock(sk);
 	sk->sk_err = ETIMEDOUT;
 	sk->sk_state_change(sk);
-	bh_unlock_sock(sk);
+	release_sock(sk);
 
 	sco_sock_kill(sk);
 	sock_put(sk);
@@ -91,14 +95,27 @@ static void sco_sock_timeout(struct timer_list *t)
 
 static void sco_sock_set_timer(struct sock *sk, long timeout)
 {
+	struct delayed_work *work;
+
+	if (!sco_pi(sk)->conn)
+		return;
+	work = &sco_pi(sk)->conn->timeout_work;
+
 	BT_DBG("sock %p state %d timeout %ld", sk, sk->sk_state, timeout);
-	sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout);
+	cancel_delayed_work_sync(work);
+	schedule_delayed_work(work, timeout);
 }
 
 static void sco_sock_clear_timer(struct sock *sk)
 {
+	struct delayed_work *work;
+
+	if (!sco_pi(sk)->conn)
+		return;
+	work = &sco_pi(sk)->conn->timeout_work;
+
 	BT_DBG("sock %p state %d", sk, sk->sk_state);
-	sk_stop_timer(sk, &sk->sk_timer);
+	cancel_delayed_work_sync(work);
 }
 
 /* ---- SCO connections ---- */
@@ -173,10 +190,10 @@ static void sco_conn_del(struct hci_conn *hcon, int err)
 
 	if (sk) {
 		sock_hold(sk);
-		bh_lock_sock(sk);
+		lock_sock(sk);
 		sco_sock_clear_timer(sk);
 		sco_chan_del(sk, err);
-		bh_unlock_sock(sk);
+		release_sock(sk);
 		sco_sock_kill(sk);
 		sock_put(sk);
 	}
@@ -193,6 +210,8 @@ static void __sco_chan_add(struct sco_conn *conn, struct sock *sk,
 	sco_pi(sk)->conn = conn;
 	conn->sk = sk;
 
+	INIT_DELAYED_WORK(&conn->timeout_work, sco_sock_timeout);
+
 	if (parent)
 		bt_accept_enqueue(parent, sk, true);
 }
@@ -212,44 +231,32 @@ static int sco_chan_add(struct sco_conn *conn, struct sock *sk,
 	return err;
 }
 
-static int sco_connect(struct sock *sk)
+static int sco_connect(struct hci_dev *hdev, struct sock *sk)
 {
 	struct sco_conn *conn;
 	struct hci_conn *hcon;
-	struct hci_dev  *hdev;
 	int err, type;
 
 	BT_DBG("%pMR -> %pMR", &sco_pi(sk)->src, &sco_pi(sk)->dst);
 
-	hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
-	if (!hdev)
-		return -EHOSTUNREACH;
-
-	hci_dev_lock(hdev);
-
 	if (lmp_esco_capable(hdev) && !disable_esco)
 		type = ESCO_LINK;
 	else
 		type = SCO_LINK;
 
 	if (sco_pi(sk)->setting == BT_VOICE_TRANSPARENT &&
-	    (!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev))) {
-		err = -EOPNOTSUPP;
-		goto done;
-	}
+	    (!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev)))
+		return -EOPNOTSUPP;
 
 	hcon = hci_connect_sco(hdev, type, &sco_pi(sk)->dst,
 			       sco_pi(sk)->setting);
-	if (IS_ERR(hcon)) {
-		err = PTR_ERR(hcon);
-		goto done;
-	}
+	if (IS_ERR(hcon))
+		return PTR_ERR(hcon);
 
 	conn = sco_conn_add(hcon);
 	if (!conn) {
 		hci_conn_drop(hcon);
-		err = -ENOMEM;
-		goto done;
+		return -ENOMEM;
 	}
 
 	/* Update source addr of the socket */
@@ -257,7 +264,7 @@ static int sco_connect(struct sock *sk)
 
 	err = sco_chan_add(conn, sk, NULL);
 	if (err)
-		goto done;
+		return err;
 
 	if (hcon->state == BT_CONNECTED) {
 		sco_sock_clear_timer(sk);
@@ -267,9 +274,6 @@ static int sco_connect(struct sock *sk)
 		sco_sock_set_timer(sk, sk->sk_sndtimeo);
 	}
 
-done:
-	hci_dev_unlock(hdev);
-	hci_dev_put(hdev);
 	return err;
 }
 
@@ -443,8 +447,8 @@ static void __sco_sock_close(struct sock *sk)
 /* Must be called on unlocked socket. */
 static void sco_sock_close(struct sock *sk)
 {
-	sco_sock_clear_timer(sk);
 	lock_sock(sk);
+	sco_sock_clear_timer(sk);
 	__sco_sock_close(sk);
 	release_sock(sk);
 	sco_sock_kill(sk);
@@ -500,8 +504,6 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock,
 
 	sco_pi(sk)->setting = BT_VOICE_CVSD_16BIT;
 
-	timer_setup(&sk->sk_timer, sco_sock_timeout, 0);
-
 	bt_sock_link(&sco_sk_list, sk);
 	return sk;
 }
@@ -566,6 +568,7 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
 {
 	struct sockaddr_sco *sa = (struct sockaddr_sco *) addr;
 	struct sock *sk = sock->sk;
+	struct hci_dev *hdev;
 	int err;
 
 	BT_DBG("sk %p", sk);
@@ -580,12 +583,17 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
 	if (sk->sk_type != SOCK_SEQPACKET)
 		return -EINVAL;
 
+	hdev = hci_get_route(&sa->sco_bdaddr, &sco_pi(sk)->src, BDADDR_BREDR);
+	if (!hdev)
+		return -EHOSTUNREACH;
+	hci_dev_lock(hdev);
+
 	lock_sock(sk);
 
 	/* Set destination address and psm */
 	bacpy(&sco_pi(sk)->dst, &sa->sco_bdaddr);
 
-	err = sco_connect(sk);
+	err = sco_connect(hdev, sk);
 	if (err)
 		goto done;
 
@@ -594,6 +602,8 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
 
 done:
 	release_sock(sk);
+	hci_dev_unlock(hdev);
+	hci_dev_put(hdev);
 	return err;
 }
 
@@ -1083,11 +1093,11 @@ static void sco_conn_ready(struct sco_conn *conn)
 	BT_DBG("conn %p", conn);
 
 	if (sk) {
+		lock_sock(sk);
 		sco_sock_clear_timer(sk);
-		bh_lock_sock(sk);
 		sk->sk_state = BT_CONNECTED;
 		sk->sk_state_change(sk);
-		bh_unlock_sock(sk);
+		release_sock(sk);
 	} else {
 		sco_conn_lock(conn);
 
@@ -1102,12 +1112,12 @@ static void sco_conn_ready(struct sco_conn *conn)
 			return;
 		}
 
-		bh_lock_sock(parent);
+		lock_sock(parent);
 
 		sk = sco_sock_alloc(sock_net(parent), NULL,
 				    BTPROTO_SCO, GFP_ATOMIC, 0);
 		if (!sk) {
-			bh_unlock_sock(parent);
+			release_sock(parent);
 			sco_conn_unlock(conn);
 			return;
 		}
@@ -1128,7 +1138,7 @@ static void sco_conn_ready(struct sco_conn *conn)
 		/* Wake up parent */
 		parent->sk_data_ready(parent);
 
-		bh_unlock_sock(parent);
+		release_sock(parent);
 
 		sco_conn_unlock(conn);
 	}
-- 
2.25.1

