lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <VI1P193MB0752428D259D066379242BD099D3A@VI1P193MB0752.EURP193.PROD.OUTLOOK.COM>
Date:   Thu, 12 Oct 2023 19:02:51 +0800
From:   Juntong Deng <juntong.deng@...look.com>
To:     borisp@...dia.com, john.fastabend@...il.com, kuba@...nel.org,
        davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        syzbot+29c22ea2d6b2c5fd2eae@...kaller.appspotmail.com
Subject: [PATCH] net/tls: Fix slab-use-after-free in tls_encrypt_done

In the current implementation, ctx->async_wait.completion is completed
after spin_lock_bh, which causes tls_sw_release_resources_tx to
continue executing and return to tls_sk_proto_cleanup, then return
to tls_sk_proto_close, and after that enter tls_sw_free_ctx_tx to kfree
the entire struct tls_context (including ctx->encrypt_compl_lock).

Since ctx->encrypt_compl_lock has been freed, subsequent spin_unlock_bh
will result in slab-use-after-free error. Due to SMP, even using
spin_lock_bh does not prevent tls_sw_release_resources_tx from continuing
on other CPUs. After tls_sw_release_resources_tx is woken up, there is no
attempt to hold ctx->encrypt_compl_lock again, therefore everything
described above is possible.

The fix is to put complete(&ctx->async_wait.completion) after
spin_unlock_bh, making the release after the unlock. Since complete is
only executed if pending is 0, which means this is the last record, there
is no need to worry about race condition causing duplicate completes.

Reported-by: syzbot+29c22ea2d6b2c5fd2eae@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=29c22ea2d6b2c5fd2eae
Signed-off-by: Juntong Deng <juntong.deng@...look.com>
---
 net/tls/tls_sw.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 270712b8d391..7abe5a6aa989 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -441,6 +441,7 @@ static void tls_encrypt_done(void *data, int err)
 	struct sk_msg *msg_en;
 	bool ready = false;
 	struct sock *sk;
+	int async_notify;
 	int pending;
 
 	msg_en = &rec->msg_encrypted;
@@ -482,10 +483,11 @@ static void tls_encrypt_done(void *data, int err)
 
 	spin_lock_bh(&ctx->encrypt_compl_lock);
 	pending = atomic_dec_return(&ctx->encrypt_pending);
+	async_notify = ctx->async_notify;
+	spin_unlock_bh(&ctx->encrypt_compl_lock);
 
-	if (!pending && ctx->async_notify)
+	if (!pending && async_notify)
 		complete(&ctx->async_wait.completion);
-	spin_unlock_bh(&ctx->encrypt_compl_lock);
 
 	if (!ready)
 		return;
-- 
2.39.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ