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
