Thread (10 messages) 10 messages, 4 authors, 2023-02-28

Re: [PATCH] net: tls: fix possible race condition between do_tls_getsockopt_conf() and do_tls_setsockopt_conf()

From: Florian Westphal <fw@strlen.de>
Date: 2023-02-24 12:06:49
Also in: lkml

Hangyu Hua [off-list ref] wrote:
ctx->crypto_send.info is not protected by lock_sock in
do_tls_getsockopt_conf(). A race condition between do_tls_getsockopt_conf()
and do_tls_setsockopt_conf() can cause a NULL point dereference or
use-after-free read when memcpy.
Its good practice to quote the relevant parts of the splat here.
quoted hunk ↗ jump to hunk
Fixes: 3c4d7559159b ("tls: kernel TLS support")
Signed-off-by: Hangyu Hua <redacted>
---
 net/tls/tls_main.c | 2 ++
 1 file changed, 2 insertions(+)
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 3735cb00905d..4956f5149b8e 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -374,6 +374,7 @@ static int do_tls_getsockopt_conf(struct sock *sk, char __user *optval,
 	}
 
 	/* get user crypto info */
+	lock_sock(sk);
 	if (tx) {
 		crypto_info = &ctx->crypto_send.info;
 		cctx = &ctx->tx;
@@ -381,6 +382,7 @@ static int do_tls_getsockopt_conf(struct sock *sk, char __user *optval,
 		crypto_info = &ctx->crypto_recv.info;
 		cctx = &ctx->rx;
 	}
+	release_sock(sk);
Could you elaborate how this fixes a UAF bug?

AFAIU do_tls_setsockopt_conf uses ctx-> as scratch space, but this means
that getsockopt can see partial states.

If so, can the setsockopt part be changed so that reads only see
consistent states?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help