Help needed in patching CVE-2021-3640

[LinMa <linma@zju.edu.cn>]

Hello there,

There is one bug (CVE-2021-3640: https://www.openwall.com/lists/oss-security/2021/07/22/1) that is similar to the recently fixed CVE-2021-3573.

The key point here is that the sco_conn_del() function can be called when syscalls like sco_sendmsg() is undergoing.
I think the easiest fix is to hang the sco_conn_del() using lock_sock() like below.

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d9a4e88dacbb..3da1ad441463 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -173,10 +173,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);
        }

This can make sure the kfree() will wait for the sock held by the sco_sendmsg() function. However, this patch can incur WARNING report like below. (I don't really know if this report is correct).

[   75.147515] ======================================================
[   75.149955] WARNING: possible circular locking dependency detected
[   75.150546] 5.11.11+ #58 Not tainted
[   75.150895] ------------------------------------------------------
[   75.151485] poc.sco/127 is trying to acquire lock:
[   75.151947] ffff888012212120 (sk_lock-AF_BLUETOOTH-BTPROTO_SCO){+.+.}-{0:0}, at: sco_conn_del+0xf6/0x0
[   75.152863]
[   75.152863] but task is already holding lock:
[   75.153420] ffffffff85b43948 (hci_cb_list_lock){+.+.}-{3:3}, at: hci_conn_hash_flush+0xb3/0x1f0
[   75.154256]
[   75.154256] which lock already depends on the new lock.

P.S. find the POC code in openwall report

With the lesson I learnt in last bad patch e305509e678b ("Bluetooth: use correct lock to prevent UAF of hdev object"). I don't really expect this as the final correct patch. 

I then try to use the technique in e04480920d1e ("Bluetooth: defer cleanup of resources in hci_unregister_dev()"). I mean, I want to defer the kfree of sco_conn object. However, the sco connection/disconnection mechanism is somewhat weird and I didn't really understand it by now.

Let's see this __sco_sock_close() function, which will be called from sco_sock_release().

static void __sco_sock_close(struct sock *sk)
{
	BT_DBG("sk %p state %d socket %p", sk, sk->sk_state, sk->sk_socket);

	switch (sk->sk_state) {
	case BT_LISTEN:
		sco_sock_cleanup_listen(sk);
		break;

	case BT_CONNECTED:
	case BT_CONFIG:
		if (sco_pi(sk)->conn->hcon) {
			sk->sk_state = BT_DISCONN;
			sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
			sco_conn_lock(sco_pi(sk)->conn);
			hci_conn_drop(sco_pi(sk)->conn->hcon);
			sco_pi(sk)->conn->hcon = NULL;
			sco_conn_unlock(sco_pi(sk)->conn);
		} else
			sco_chan_del(sk, ECONNRESET);
		break;

	case BT_CONNECT2:
	case BT_CONNECT:
	case BT_DISCONN:
		sco_chan_del(sk, ECONNRESET);
		break;

	default:
		sock_set_flag(sk, SOCK_ZAPPED);
		break;
	}
}

As you can see, though one socket is in BT_CONNECTED state, this function will just drop the kref of sco_pi(sk)->conn->hcon but do nothing with sco_pi(sk)->conn object. Then how this conn object is released? Where should I defer the deallocation function to?

I think I need help and discussion to settle down the solution for this. T_T

Best Wishes
Lin Ma