All of lore.kernel.org
 help / color / mirror / Atom feed
From: LinMa <linma@zju.edu.cn>
To: penguin-kernel@i-love.sakura.ne.jp, linux-bluetooth@vger.kernel.org
Subject: Help needed in patching CVE-2021-3640
Date: Thu, 2 Sep 2021 20:33:43 +0800 (GMT+08:00)	[thread overview]
Message-ID: <15f5a46.b79d9.17ba6802ccd.Coremail.linma@zju.edu.cn> (raw)

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

             reply	other threads:[~2021-09-02 12:33 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02 12:33 LinMa [this message]
2021-09-02 15:32 ` Tetsuo Handa
2021-09-03  2:44   ` [PATCH] Bluetooth: avoid page fault from sco_send_frame() Tetsuo Handa
2021-09-03  3:48     ` Luiz Augusto von Dentz
2021-09-03  4:40       ` Tetsuo Handa
2021-09-04  2:02         ` Tetsuo Handa
2021-10-11  7:00           ` Salvatore Bonaccorso
2021-10-11  7:13             ` Takashi Iwai
2021-09-02 18:46 ` Help needed in patching CVE-2021-3640 Luiz Augusto von Dentz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=15f5a46.b79d9.17ba6802ccd.Coremail.linma@zju.edu.cn \
    --to=linma@zju.edu.cn \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --subject='Re: Help needed in patching CVE-2021-3640' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.