[PATCH net-next v2 1/2] tls: read sk_err once in tls_encrypt_done()

[Rishikesh Jethwani]

tls_encrypt_done() runs from the crypto completion, without the socket
lock, and reads sk->sk_err twice. getsockopt(SO_ERROR) and sock_error()
clear sk_err locklessly, so it can go from non-zero to zero between the
two reads. A successful completion (err == 0) then takes the "no error
set on the socket" branch and calls tls_err_abort(sk, 0), which hits
WARN_ON_ONCE(err >= 0) and clears sk_err.

Read sk_err once and use that value for both checks.

Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records for performance")
Reported-by: syzbot+cca46a9d1276f38af2ae@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=cca46a9d1276f38af2ae
Suggested-by: Sabrina Dubroca <sd@xxxxxxxxxxxxxxx>
Signed-off-by: Rishikesh Jethwani <rjethwani@xxxxxxxxxxxxxxx>
---
 net/tls/tls_sw.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 771e02bff519..b4535139438a 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -473,6 +473,7 @@ static void tls_encrypt_done(void *data, int err)
 	struct scatterlist *sge;
 	struct sk_msg *msg_en;
 	struct sock *sk;
+	int sk_err;
 
 	if (err == -EINPROGRESS) /* see the comment in tls_decrypt_done() */
 		return;
@@ -489,12 +490,13 @@ static void tls_encrypt_done(void *data, int err)
 	sge->length += prot->prepend_size;
 
 	/* Check if error is previously set on socket */
-	if (err || sk->sk_err) {
+	sk_err = READ_ONCE(sk->sk_err);
+	if (err || sk_err) {
 		rec = NULL;
 
 		/* If err is already set on socket, return the same code */
-		if (sk->sk_err) {
-			ctx->async_wait.err = -sk->sk_err;
+		if (sk_err) {
+			ctx->async_wait.err = -sk_err;
 		} else {
 			ctx->async_wait.err = err;
 			tls_err_abort(sk, err);
-- 
2.50.1