Re: Help needed in patching CVE-2021-3640

[Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>]

On 2021/09/02 21:33, LinMa wrote:
> 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.

Since hdev->lock is held when sco_conn_del() is called,

 3 locks held by poc/6686:
  #0: ffff8880158690e0 (&hdev->req_lock){+.+.}-{3:3}, at: hci_dev_do_close+0x44/0x6a0 [bluetooth]
  #1: ffff888015868080 (&hdev->lock){+.+.}-{3:3}, at: hci_dev_do_close+0x1ac/0x6a0 [bluetooth]
  #2: ffffffffa0630030 (hci_cb_list_lock){+.+.}-{3:3}, at: hci_conn_hash_flush+0x6f/0x140 [bluetooth]

I guess that holding hdev->lock when sco_send_frame() is called would avoid use-after-free.

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d9a4e88dacbb..f5339bfba4a5 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -727,10 +727,17 @@ static int sco_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 
 	lock_sock(sk);
 
-	if (sk->sk_state == BT_CONNECTED)
-		err = sco_send_frame(sk, msg, len);
-	else
-		err = -ENOTCONN;
+	err = -ENOTCONN;
+	if (sk->sk_state == BT_CONNECTED) {
+		struct hci_dev *hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
+
+		if (hdev) {
+			hci_dev_lock(hdev);
+			err = sco_send_frame(sk, msg, len);
+			hci_dev_unlock(hdev);
+			hci_dev_put(hdev);
+		}
+	}
 
 	release_sock(sk);
 	return err;

But I'm not happy with calling hci_get_route() every time.
Can we cache the hdev found upon sco_connect() ?