From ba316be1b6a00db7126ed9a39f9bee434a508043 Mon Sep 17 00:00:00 2001

From: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>

Date: Tue, 10 Aug 2021 12:14:05 +0800

Subject: [PATCH] Bluetooth: schedule SCO timeouts with delayed_work

Git-commit: ba316be1b6a00db7126ed9a39f9bee434a508043

Patch-mainline: v5.15-rc1

References: CVE-2021-3640 bsc#1188172

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 and sco_conn_ready 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.

As a note, cancel_delayed_work is used instead of

cancel_delayed_work_sync in sco_sock_set_timer and

sco_sock_clear_timer to avoid a deadlock. In the future, the call to

bh_lock_sock inside sco_sock_timeout should be changed to lock_sock to

synchronize with other functions using lock_sock. However, since

sco_sock_set_timer and sco_sock_clear_timer are sometimes called under

the locked socket (in sco_connect and __sco_sock_close),

cancel_delayed_work_sync might cause them to sleep until an

sco_sock_timeout that has started finishes running. But

sco_sock_timeout would also sleep until it can grab the lock_sock.

Using cancel_delayed_work is fine because sco_sock_timeout does not

change from run to run, hence there is no functional difference

Between: 

1. waiting for a timeout to finish running before scheduling another

timeout

2. scheduling another timeout while a timeout is running.

Link: https://syzkaller.appspot.com/bug?id=9089d89de0502e120f234ca0fc8a703f7368b31e [1]

Reported-by: syzbot+2f6d7c28bb4bf7e82060@syzkaller.appspotmail.com

Tested-by: syzbot+2f6d7c28bb4bf7e82060@syzkaller.appspotmail.com

Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@gmail.com>

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Acked-by: Takashi Iwai <tiwai@suse.de>

---

 net/bluetooth/sco.c |   35 +++++++++++++++++++++++++++++------

 1 file changed, 29 insertions(+), 6 deletions(-)

--- 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;

 };

@@ -73,9 +75,20 @@ struct sco_pinfo {

 #define SCO_CONN_TIMEOUT	(HZ * 40)

 #define SCO_DISCONN_TIMEOUT	(HZ * 2)

-static void sco_sock_timeout(unsigned long arg)

+static void sco_sock_timeout(struct work_struct *work)

 {

-	struct sock *sk = (struct sock *)arg;

+	struct sco_conn *conn = container_of(work, struct sco_conn,

+					     timeout_work.work);

+	struct sock *sk;

+

+	sco_conn_lock(conn);

+	sk = conn->sk;

+	if (sk)

+		sock_hold(sk);

+	sco_conn_unlock(conn);

+

+	if (!sk)

+		return;

 	BT_DBG("sock %p state %d", sk, sk->sk_state);

@@ -90,14 +103,21 @@ static void sco_sock_timeout(unsigned lo

 static void sco_sock_set_timer(struct sock *sk, long timeout)

 {

+	if (!sco_pi(sk)->conn)

+		return;

+

 	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(&sco_pi(sk)->conn->timeout_work);

+	schedule_delayed_work(&sco_pi(sk)->conn->timeout_work, timeout);

 }

 static void sco_sock_clear_timer(struct sock *sk)

 {

+	if (!sco_pi(sk)->conn)

+		return;

+

 	BT_DBG("sock %p state %d", sk, sk->sk_state);

-	sk_stop_timer(sk, &sk->sk_timer);

+	cancel_delayed_work(&sco_pi(sk)->conn->timeout_work);

 }

 /* ---- SCO connections ---- */

@@ -178,6 +198,9 @@ static void sco_conn_del(struct hci_conn

 		bh_unlock_sock(sk);

 		sco_sock_kill(sk);

 		sock_put(sk);

+

+		/* Ensure no more work items will run before freeing conn. */

+		cancel_delayed_work_sync(&conn->timeout_work);

 	}

 	hcon->sco_data = NULL;

@@ -192,6 +215,8 @@ static void __sco_chan_add(struct sco_co

 	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);

 }

@@ -488,8 +513,6 @@ static struct sock *sco_sock_alloc(struc

 	sco_pi(sk)->setting = BT_VOICE_CVSD_16BIT;

-	setup_timer(&sk->sk_timer, sco_sock_timeout, (unsigned long)sk);

-

 	bt_sock_link(&sco_sk_list, sk);

 	return sk;

 }