Thread (12 messages) 12 messages, 4 authors, 18d ago

Re: [PATCH net-next v12 3/6] tls: Move decrypt-failure abort into tls_rx_one_record()

From: "Chuck Lever" <cel@kernel.org>
Date: 2026-06-07 03:42:37


On Thu, Jun 4, 2026, at 1:48 PM, Chuck Lever wrote:
quoted hunk ↗ jump to hunk
From: Chuck Lever <redacted>

Three receive paths -- recvmsg, read_sock, and splice_read --
each follow tls_rx_one_record() with the same tls_err_abort()
call. Consolidate the abort into tls_rx_one_record() so the
decrypt-and-abort sequence lives in one place.

A tls_check_pending_rekey() failure after successful
decryption no longer triggers tls_err_abort(). That path
fires only when skb_copy_bits() fails on a valid skb,
which is not a realistic scenario.

Suggested-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Chuck Lever <redacted>
---
 net/tls/tls_sw.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index fd8d3c979368..798f2535ddf7 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1827,6 +1827,9 @@ static int tls_check_pending_rekey(struct sock 
*sk, struct tls_context *ctx,
 	return 0;
 }

+/* On decrypt failure the connection is aborted (sk_err set) before
+ * returning a negative errno.
+ */
 static int tls_rx_one_record(struct sock *sk, struct msghdr *msg,
 			     struct tls_decrypt_arg *darg)
 {
@@ -1838,8 +1841,10 @@ static int tls_rx_one_record(struct sock *sk, 
struct msghdr *msg,
 	err = tls_decrypt_device(sk, msg, tls_ctx, darg);
 	if (!err)
 		err = tls_decrypt_sw(sk, tls_ctx, msg, darg);
-	if (err < 0)
+	if (err < 0) {
+		tls_err_abort(sk, -EBADMSG);
 		return err;
+	}

 	rxm = strp_msg(darg->skb);
 	rxm->offset += prot->prepend_size;
@@ -2150,10 +2155,8 @@ int tls_sw_recvmsg(struct sock *sk,
 			darg.async = false;

 		err = tls_rx_one_record(sk, msg, &darg);
-		if (err < 0) {
-			tls_err_abort(sk, -EBADMSG);
+		if (err < 0)
 			goto recv_end;
-		}

 		async |= darg.async;
@@ -2312,10 +2315,8 @@ ssize_t tls_sw_splice_read(struct socket *sock,  
loff_t *ppos,
 		memset(&darg.inargs, 0, sizeof(darg.inargs));

 		err = tls_rx_one_record(sk, NULL, &darg);
-		if (err < 0) {
-			tls_err_abort(sk, -EBADMSG);
+		if (err < 0)
 			goto splice_read_end;
-		}

 		tls_rx_rec_done(ctx);
 		skb = darg.skb;
@@ -2398,10 +2399,8 @@ int tls_sw_read_sock(struct sock *sk, 
read_descriptor_t *desc,
 			memset(&darg.inargs, 0, sizeof(darg.inargs));

 			err = tls_rx_one_record(sk, NULL, &darg);
-			if (err < 0) {
-				tls_err_abort(sk, -EBADMSG);
+			if (err < 0)
 				goto read_sock_end;
-			}

 			released = tls_read_flush_backlog(sk, prot, INT_MAX,
 							  0, decrypted,

-- 
2.54.0
I reviewed the Sashiko findings for this patch.

Finding 1 is a false positive. The multi-step scenario described in the
finding (unrealistic skb_copy_bits failure -> no-abort -> retry ->
double-decrement underflow) requires a precondition the commit message
explicitly calls "not a realistic scenario."

Finding 2 is a pre-existing issue. The ZC fallback path lives entirely
in tls_decrypt_sw(), which this patch does not touch.


-- 
Chuck Lever
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help