[PATCH net v2 0/5] net: tls: break the connection after a burst of empty data records

[PATCH net v2 0/5] net: tls: break the connection after a burst of empty data records

[Jakub Kicinski]

Deal with overload of zero-length TLS records. This is the next iteration
of Chuck's series. I tried to make it as simple as possible - break the
connection if sender floods us with zero-length records. Consume / discard
those records even on PEEK to avoid infinite rx_list growth.

https://lore.kernel.org/20261019-tls-follow-on-v6-0-ab01c4ecca0c@kernel.org

v2:
 - add missing poll() back to the selftests
v1: https://lore.kernel.org/20261109175836.1246840-1-kuba@kernel.org

Chuck Lever (3):
  tls: Consume empty data records in tls_sw_splice_read()
  selftests: tls: Skip the zero_len tests when TLS is unavailable
  selftests: tls: Extend coverage for zero-length records

Jakub Kicinski (2):
  tls: Break the connection after a burst of empty data records
  selftests: tls: Cover the cap on consecutive empty records

 include/net/tls.h                 |   1 +
 net/tls/tls_sw.c                  |  77 +++++--
 tools/testing/selftests/net/tls.c | 353 ++++++++++++++++++++++++++++--
 3 files changed, 390 insertions(+), 41 deletions(-)

-- 
2.56.0

[PATCH net v2 1/5] tls: Break the connection after a burst of empty data records

[Jakub Kicinski]

TLS permits zero-length application_data records, and RFC 8446
suggests them for generating cover traffic. Nothing bounds how many of
them a reader processes back to back, though. They advance neither
loop bound in tls_sw_recvmsg() and tls_sw_read_sock(), so a peer
streaming them faster than the reader decrypts them keeps it in the
kernel for as long as they arrive. recvmsg() does not check for signals
while records are ready, and its peek and async paths queue every empty
record on rx_list. read_sock consumers like nvme-tcp hold the socket
lock throughout and never get control back.

Break the connection once a reader decrypts more than 128 consecutive
empty data records without sleeping or returning to its caller in
between, treating the burst as a protocol violation like a malformed
record header. A stream of empty records which the reader keeps up with
costs no more than any other traffic, so the count starts over on every
read and whenever the reader waits for the next record. The strparser
is stopped before it parses the next record, so the reader gets the
error from tls_rx_rec_wait() and no later record is delivered.
Returning to the caller and keeping the connection alive would need a
way to wake read_sock consumers which installed their own sk_data_ready
after the handshake, a lot of code for a peer that is either broken or
malicious.

Consume empty records in recvmsg() rather than queuing them on rx_list,
so that repeated MSG_PEEK cannot pile them up past the per-read count.
Decrypt them synchronously, too. Once a recvmsg() call has decrypted a
record asynchronously, recv_end replaces the error with the result of
process_rx_list(), so a call that saw only empty records returns 0, a
false EOF, instead of the error or -EAGAIN.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260720-tcp-read-sock-v2-0-29545d034f3c@kernel.org?part=1
Closes: https://sashiko.dev/#/patchset/20260630191551.875664-1-cel@kernel.org?part=1
Link: https://lore.kernel.org/all/20261019-tls-follow-on-v6-0-ab01c4ecca0c@kernel.org/
Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Fixes: 3be28e2c9cd0 ("net/tls: Consume empty data records in tls_sw_read_sock()")
Reviewed-by: Chuck Lever <cel@kernel.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2: no change
v1: https://lore.kernel.org/20261109175836.1246840-2-kuba@kernel.org
---
 include/net/tls.h |  1 +
 net/tls/tls_sw.c  | 29 +++++++++++++++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index e57bef58851e..0b001b8ab1c3 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -139,6 +139,7 @@ struct tls_sw_context_rx {
 	u8 zc_capable:1;
 	u8 reader_contended:1;
 	bool key_update_pending;
+	u8 empty_run;
 
 	struct tls_strparser strp;
 
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 23eb94d9da6e..65e578c8c612 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1146,6 +1146,7 @@ tls_rx_rec_wait(struct sock *sk, bool nonblock, bool released, bool has_copied)
 		if (!timeo)
 			return -EAGAIN;
 
+		ctx->empty_run = 0;
 		released = true;
 		add_wait_queue(sk_sleep(sk), &wait);
 		sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
@@ -1565,6 +1566,7 @@ static int tls_rx_one_record(struct sock *sk, struct msghdr *msg,
 			     struct tls_decrypt_arg *darg)
 {
 	struct tls_context *tls_ctx = tls_get_ctx(sk);
+	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
 	struct tls_prot_info *prot = &tls_ctx->prot_info;
 	struct strp_msg *rxm;
 	int err;
@@ -1582,6 +1584,19 @@ static int tls_rx_one_record(struct sock *sk, struct msghdr *msg,
 	rxm->full_len -= prot->overhead_size;
 	tls_advance_record_sn(sk, prot, &tls_ctx->rx);
 
+	/* A peer may send empty data records, e.g. as cover traffic, but
+	 * a burst of them traps the reader in the kernel. Treat more than
+	 * 128 in a row, without the reader sleeping or returning to the
+	 * caller in between, as a protocol violation. Stop the strparser
+	 * before the next record gets parsed, so the reader finds the
+	 * error in tls_rx_rec_wait().
+	 */
+	if (rxm->full_len ||
+	    tls_msg(darg->skb)->control != TLS_RECORD_TYPE_DATA)
+		ctx->empty_run = 0;
+	else if (unlikely(++ctx->empty_run > 128))
+		tls_strp_abort_strp(&ctx->strp, -EBADMSG);
+
 	return tls_check_pending_rekey(sk, tls_ctx, darg->skb);
 }
 
@@ -1761,6 +1776,7 @@ static int tls_rx_reader_acquire(struct sock *sk, struct tls_sw_context_rx *ctx,
 	}
 
 	WRITE_ONCE(ctx->reader_present, 1);
+	ctx->empty_run = 0;
 
 	return 0;
 }
@@ -1874,9 +1890,11 @@ int tls_sw_recvmsg(struct sock *sk,
 		    tlm->control == TLS_RECORD_TYPE_DATA)
 			darg.zc = true;
 
-		/* Do not use async mode if record is non-data */
+		/* Do not use async mode if record is non-data or empty,
+		 * recv_end would report an error after empty records as EOF.
+		 */
 		if (tlm->control == TLS_RECORD_TYPE_DATA)
-			darg.async = ctx->async_capable;
+			darg.async = ctx->async_capable && to_decrypt;
 		else
 			darg.async = false;
 
@@ -1918,7 +1936,10 @@ int tls_sw_recvmsg(struct sock *sk,
 
 			DEBUG_NET_WARN_ON_ONCE(darg.skb == ctx->strp.anchor);
 
-			if (async) {
+			/* Consume empty records, even when peeking, so that
+			 * they cannot pile up on rx_list.
+			 */
+			if (async && chunk) {
 				/* TLS 1.2-only, to_decrypt must be text len */
 				chunk = min_t(int, to_decrypt, len);
 				async_copy_bytes += chunk;
@@ -1939,7 +1960,7 @@ int tls_sw_recvmsg(struct sock *sk,
 			if (err < 0)
 				goto put_on_rx_list_err;
 
-			if (is_peek) {
+			if (is_peek && chunk) {
 				peeked += chunk;
 				goto put_on_rx_list;
 			}
-- 
2.56.0

[PATCH net v2 2/5] tls: Consume empty data records in tls_sw_splice_read()

[Jakub Kicinski]

From: Chuck Lever <cel@kernel.org>

A zero-length application_data record decrypts to full_len == 0,
so tls_sw_splice_read() splices zero bytes and returns zero. A zero
return from a splice read means EOF, and the caller tears down a
connection that is still live.

Consume the record and fetch the next one, as tls_sw_read_sock()
does. The cap on consecutive empty records bounds the loop.

When an empty record precedes a close_notify, splice(2) now returns
-EINVAL from the control-record test instead of a false EOF. A
splice that reaches an alert record already reports -EINVAL.

Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Reported-by: Sabrina Dubroca <sd@queasysnail.net>
Closes: https://lore.kernel.org/netdev/akaoXcfamBp8_mYe@krikkit/
Signed-off-by: Chuck Lever <cel@kernel.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2: no change
v1: https://lore.kernel.org/20261109175836.1246840-3-kuba@kernel.org
---
 net/tls/tls_sw.c | 48 ++++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 65e578c8c612..0bab7175c9d9 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2044,32 +2044,40 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	if (err)
 		goto splice_read_end;
 
-	if (!skb_queue_empty(&ctx->rx_list)) {
-		skb = __skb_dequeue(&ctx->rx_list);
-	} else {
-		struct tls_decrypt_arg darg;
+	for (;;) {
+		if (!skb_queue_empty(&ctx->rx_list)) {
+			skb = __skb_dequeue(&ctx->rx_list);
+		} else {
+			struct tls_decrypt_arg darg;
 
-		err = tls_rx_rec_wait(sk, nonblock, true, false);
-		if (err <= 0)
-			goto splice_read_end;
+			err = tls_rx_rec_wait(sk, nonblock, true, false);
+			if (err <= 0)
+				goto splice_read_end;
 
-		memset(&darg.inargs, 0, sizeof(darg.inargs));
+			memset(&darg.inargs, 0, sizeof(darg.inargs));
 
-		err = tls_rx_one_record(sk, NULL, &darg);
-		if (err < 0)
-			goto splice_read_end;
+			err = tls_rx_one_record(sk, NULL, &darg);
+			if (err < 0)
+				goto splice_read_end;
 
-		tls_rx_rec_done(ctx);
-		skb = darg.skb;
-	}
+			tls_rx_rec_done(ctx);
+			skb = darg.skb;
+		}
 
-	rxm = strp_msg(skb);
-	tlm = tls_msg(skb);
+		rxm = strp_msg(skb);
+		tlm = tls_msg(skb);
 
-	/* splice does not support reading control messages */
-	if (tlm->control != TLS_RECORD_TYPE_DATA) {
-		err = -EINVAL;
-		goto splice_requeue;
+		/* splice does not support reading control messages */
+		if (tlm->control != TLS_RECORD_TYPE_DATA) {
+			err = -EINVAL;
+			goto splice_requeue;
+		}
+
+		/* Splicing zero bytes would read as EOF */
+		if (rxm->full_len)
+			break;
+
+		consume_skb(skb);
 	}
 
 	chunk = min_t(unsigned int, rxm->full_len, len);
-- 
2.56.0

[PATCH net v2 3/5] selftests: tls: Skip the zero_len tests when TLS is unavailable

[Jakub Kicinski]

From: Chuck Lever <cel@kernel.org>

FIXTURE_SETUP(zero_len) returns early when ulp_sock_pair() reports
that the TCP_ULP setsockopt failed, so on a kernel built without
CONFIG_TLS the TLS_RX setsockopt never runs. TEST_F(zero_len, test)
then sends its raw records over a plain TCP socket, which returns
the ciphertext verbatim. All eight variants FAIL on a machine that
lacks TLS, and those failures mask any real regression in the same
run.

Skip the test when the fixture recorded notls, as the other
fixtures in this file do.

Fixes: a61a3e961baf ("selftests: tls: add tests for zero-length records")
Signed-off-by: Chuck Lever <cel@kernel.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2: no change
v1: https://lore.kernel.org/20261109175836.1246840-4-kuba@kernel.org
---
 tools/testing/selftests/net/tls.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index c4203401c4bf..ca47576220d3 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2670,6 +2670,9 @@ TEST_F(zero_len, test)
 	int rec_off;
 	int i;
 
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
 	for (i = 0; i < 4 && variant->recs[i]; i++)
 		EXPECT_EQ(send(self->fd, variant->recs[i]->cipher_data,
 			       variant->recs[i]->cipher_len, 0),
-- 
2.56.0

[PATCH net v2 4/5] selftests: tls: Extend coverage for zero-length records

[Jakub Kicinski]

From: Chuck Lever <cel@kernel.org>

The zero_len fixture injects raw pre-encrypted records over a
socket carrying a TLS_RX key only, and some of those records are
zero-length application_data records. Every variant reads the
records back with recvmsg() without MSG_PEEK, so neither splice nor
MSG_PEEK is tested against an empty record.

Add zero_len_splice and zero_len_peek. A zero return from splice()
means EOF, so zero_len_splice checks that a run of empty data
records does not end a live connection. zero_len_peek checks that a
peek reaches the payload behind a run of empty records and leaves
the payload in place for the read that follows. When a payload does
not follow the run, the peek must leave the socket unreadable. An
unfixed kernel returns 0 from splice(), and after the peek poll()
reports the socket readable.

Signed-off-by: Chuck Lever <cel@kernel.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - bring back the poll() check after a peek at only empty records (Chuck)
v1: https://lore.kernel.org/20261109175836.1246840-5-kuba@kernel.org
---
 tools/testing/selftests/net/tls.c | 226 +++++++++++++++++++++++++++---
 1 file changed, 209 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index ca47576220d3..c505508afde5 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2580,6 +2580,42 @@ static const struct raw_rec id2_data_l0 = {
 	},
 };
 
+static void zero_len_sock_pair(struct __test_metadata *_metadata,
+			       int *fd, int *cfd, bool *notls)
+{
+	struct tls_crypto_info_keys tls12;
+	int ret;
+
+	tls_crypto_info_init(TLS_1_2_VERSION, TLS_CIPHER_AES_CCM_128,
+			     &tls12, 0);
+
+	ulp_sock_pair(_metadata, fd, cfd, notls);
+	if (*notls)
+		return;
+
+	/* fd gets no keys, since these fixtures send raw records over it */
+	ret = setsockopt(*cfd, SOL_TLS, TLS_RX, &tls12, tls12.len);
+	ASSERT_EQ(ret, 0);
+}
+
+/* Send a variant's records and return the last one carrying payload */
+static const struct raw_rec *
+zero_len_send_recs(struct __test_metadata *_metadata, int fd,
+		   const struct raw_rec *const *recs)
+{
+	const struct raw_rec *payload = NULL;
+	int i;
+
+	for (i = 0; i < 4 && recs[i]; i++) {
+		EXPECT_EQ(send(fd, recs[i]->cipher_data, recs[i]->cipher_len, 0),
+			  recs[i]->cipher_len);
+		if (recs[i]->plain_len)
+			payload = recs[i];
+	}
+
+	return payload;
+}
+
 FIXTURE(zero_len)
 {
 	int fd, cfd;
@@ -2642,19 +2678,7 @@ FIXTURE_VARIANT_ADD(zero_len, data_0data_0data)
 
 FIXTURE_SETUP(zero_len)
 {
-	struct tls_crypto_info_keys tls12;
-	int ret;
-
-	tls_crypto_info_init(TLS_1_2_VERSION, TLS_CIPHER_AES_CCM_128,
-			     &tls12, 0);
-
-	ulp_sock_pair(_metadata, &self->fd, &self->cfd, &self->notls);
-	if (self->notls)
-		return;
-
-	/* Don't install keys on fd, we'll send raw records */
-	ret = setsockopt(self->cfd, SOL_TLS, TLS_RX, &tls12, tls12.len);
-	ASSERT_EQ(ret, 0);
+	zero_len_sock_pair(_metadata, &self->fd, &self->cfd, &self->notls);
 }
 
 FIXTURE_TEARDOWN(zero_len)
@@ -2673,10 +2697,7 @@ TEST_F(zero_len, test)
 	if (self->notls)
 		SKIP(return, "no TLS support");
 
-	for (i = 0; i < 4 && variant->recs[i]; i++)
-		EXPECT_EQ(send(self->fd, variant->recs[i]->cipher_data,
-			       variant->recs[i]->cipher_len, 0),
-			  variant->recs[i]->cipher_len);
+	zero_len_send_recs(_metadata, self->fd, variant->recs);
 
 	rec = &variant->recs[0];
 	rec_off = 0;
@@ -2702,6 +2723,177 @@ TEST_F(zero_len, test)
 	}
 };
 
+FIXTURE(zero_len_peek)
+{
+	int fd, cfd;
+	bool notls;
+};
+
+FIXTURE_VARIANT(zero_len_peek)
+{
+	const struct raw_rec *recs[4];
+	ssize_t peek_ret;
+};
+
+FIXTURE_VARIANT_ADD(zero_len_peek, 0data_0data_data)
+{
+	.recs = { &id0_data_l0, &id1_data_l0, &id2_data_l11, },
+	.peek_ret = 11,
+};
+
+FIXTURE_VARIANT_ADD(zero_len_peek, 0data_0data_0data)
+{
+	.recs = { &id0_data_l0, &id1_data_l0, &id2_data_l0, },
+	.peek_ret = -EAGAIN,
+};
+
+FIXTURE_SETUP(zero_len_peek)
+{
+	zero_len_sock_pair(_metadata, &self->fd, &self->cfd, &self->notls);
+}
+
+FIXTURE_TEARDOWN(zero_len_peek)
+{
+	close(self->fd);
+	close(self->cfd);
+}
+
+/* Peeking past a run of empty data records must reach the payload
+ * behind them, and a run with no payload behind it must report EAGAIN
+ * and leave no record queued.
+ */
+TEST_F(zero_len_peek, test)
+{
+	struct pollfd pfd = { .fd = self->cfd, .events = POLLIN };
+	const struct raw_rec *payload;
+	unsigned char buf[128];
+	ssize_t ret;
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	payload = zero_len_send_recs(_metadata, self->fd, variant->recs);
+
+	if (variant->peek_ret < 0) {
+		ret = recv(self->cfd, buf, sizeof(buf),
+			   MSG_DONTWAIT | MSG_PEEK);
+		EXPECT_EQ(ret, -1);
+		EXPECT_EQ(errno, -variant->peek_ret);
+
+		/* A record left on rx_list keeps the socket readable */
+		EXPECT_EQ(poll(&pfd, 1, 0), 0);
+		return;
+	}
+
+	ret = recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT | MSG_PEEK);
+	EXPECT_EQ(ret, variant->peek_ret);
+	if (ret == variant->peek_ret)
+		EXPECT_EQ(memcmp(buf, payload->plain_data,
+				 variant->peek_ret), 0);
+
+	ret = recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT);
+	EXPECT_EQ(ret, variant->peek_ret);
+	if (ret == variant->peek_ret)
+		EXPECT_EQ(memcmp(buf, payload->plain_data,
+				 variant->peek_ret), 0);
+
+	ret = recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT);
+	EXPECT_EQ(ret, -1);
+	EXPECT_EQ(errno, EAGAIN);
+}
+
+FIXTURE(zero_len_splice)
+{
+	int fd, cfd;
+	bool notls;
+};
+
+FIXTURE_VARIANT(zero_len_splice)
+{
+	const struct raw_rec *recs[4];
+	ssize_t splice_ret;
+};
+
+FIXTURE_VARIANT_ADD(zero_len_splice, 0data_data)
+{
+	.recs = { &id0_data_l0, &id1_data_l11, },
+	.splice_ret = 11,
+};
+
+FIXTURE_VARIANT_ADD(zero_len_splice, 0data_0data_data)
+{
+	.recs = { &id0_data_l0, &id1_data_l0, &id2_data_l11, },
+	.splice_ret = 11,
+};
+
+FIXTURE_VARIANT_ADD(zero_len_splice, 0data_0data_0data)
+{
+	.recs = { &id0_data_l0, &id1_data_l0, &id2_data_l0, },
+	.splice_ret = -EAGAIN,
+};
+
+FIXTURE_VARIANT_ADD(zero_len_splice, 0data_0ctrl)
+{
+	.recs = { &id0_data_l0, &id1_ctrl_l0, },
+	.splice_ret = -EINVAL,
+};
+
+FIXTURE_SETUP(zero_len_splice)
+{
+	zero_len_sock_pair(_metadata, &self->fd, &self->cfd, &self->notls);
+}
+
+FIXTURE_TEARDOWN(zero_len_splice)
+{
+	close(self->fd);
+	close(self->cfd);
+}
+
+/* Splicing must skip a run of empty data records to reach the payload
+ * behind it, since a zero-byte splice reads as EOF. Splice reports
+ * EAGAIN for a run with no payload behind it, and EINVAL for a control
+ * record behind the run.
+ */
+TEST_F(zero_len_splice, test)
+{
+	struct pollfd pfd = { .fd = self->cfd, .events = POLLIN };
+	const struct raw_rec *payload;
+	unsigned char buf[128];
+	ssize_t ret;
+	int p[2];
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	ASSERT_GE(pipe(p), 0);
+
+	payload = zero_len_send_recs(_metadata, self->fd, variant->recs);
+
+	if (variant->splice_ret < 0) {
+		ret = splice(self->cfd, NULL, p[1], NULL, sizeof(buf),
+			     SPLICE_F_NONBLOCK);
+		EXPECT_EQ(ret, -1);
+		EXPECT_EQ(errno, -variant->splice_ret);
+	} else {
+		/* ASSERT so a zero return stops the test before the read
+		 * below blocks until the harness timeout.
+		 */
+		ASSERT_EQ(splice(self->cfd, NULL, p[1], NULL, sizeof(buf),
+				 SPLICE_F_NONBLOCK), variant->splice_ret);
+		ret = read(p[0], buf, sizeof(buf));
+		EXPECT_EQ(ret, variant->splice_ret);
+		if (ret == variant->splice_ret)
+			EXPECT_EQ(memcmp(buf, payload->plain_data,
+					 variant->splice_ret), 0);
+
+		/* The empty records were consumed, not left queued */
+		EXPECT_EQ(poll(&pfd, 1, 0), 0);
+	}
+
+	close(p[0]);
+	close(p[1]);
+}
+
 FIXTURE(tls_err)
 {
 	int fd, cfd;
-- 
2.56.0

[PATCH net v2 5/5] selftests: tls: Cover the cap on consecutive empty records

[Jakub Kicinski]

Check through both recv() and splice() that a run of 128 empty data
records is consumed and the data record behind it is delivered, and
that a run of 129 breaks the connection with EBADMSG before the data
record can be read. Two runs of 128 with a read returning EAGAIN in
between must not break the connection, the cap applies to a burst the
reader cannot keep up with, not to the connection as a whole.

kTLS cannot send an empty record. A TLS 1.3 record of type 0 whose
only byte is the data content type decrypts to one, since the receiver
strips the 0 as padding.

Reviewed-by: Chuck Lever <cel@kernel.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2: no change
v1: https://lore.kernel.org/20261109175836.1246840-6-kuba@kernel.org
---
 tools/testing/selftests/net/tls.c | 124 ++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index c505508afde5..bd2797f9f619 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2894,6 +2894,130 @@ TEST_F(zero_len_splice, test)
 	close(p[1]);
 }
 
+FIXTURE(zero_len_run)
+{
+	int fd, cfd;
+	bool notls;
+};
+
+/* Before each of two reads, send n[i] empty records followed by a data
+ * record if data[i] is set. ret[i] is the result of the read.
+ */
+FIXTURE_VARIANT(zero_len_run)
+{
+	unsigned int n[2];
+	bool data[2];
+	int ret[2];
+};
+
+FIXTURE_VARIANT_ADD(zero_len_run, 128)
+{
+	.n = { 128, 0, },
+	.data = { true, false, },
+	.ret = { 4, -EAGAIN, },
+};
+
+/* More than 128 consecutive empty records break the connection */
+FIXTURE_VARIANT_ADD(zero_len_run, 129)
+{
+	.n = { 129, 0, },
+	.data = { true, false, },
+	.ret = { -EBADMSG, -EAGAIN, },
+};
+
+/* The count starts over on every read */
+FIXTURE_VARIANT_ADD(zero_len_run, 128_128)
+{
+	.n = { 128, 128, },
+	.data = { false, true, },
+	.ret = { -EAGAIN, 4, },
+};
+
+FIXTURE_SETUP(zero_len_run)
+{
+	struct tls_crypto_info_keys tls13;
+	int ret;
+
+	tls_crypto_info_init(TLS_1_3_VERSION, TLS_CIPHER_AES_GCM_128,
+			     &tls13, 0);
+
+	ulp_sock_pair(_metadata, &self->fd, &self->cfd, &self->notls);
+	if (self->notls)
+		return;
+
+	ret = setsockopt(self->fd, SOL_TLS, TLS_TX, &tls13, tls13.len);
+	ASSERT_EQ(ret, 0);
+	ret = setsockopt(self->cfd, SOL_TLS, TLS_RX, &tls13, tls13.len);
+	ASSERT_EQ(ret, 0);
+}
+
+FIXTURE_TEARDOWN(zero_len_run)
+{
+	close(self->fd);
+	close(self->cfd);
+}
+
+static void zero_len_run_send(struct __test_metadata *_metadata, int fd,
+			      const FIXTURE_VARIANT(zero_len_run) *variant,
+			      int i)
+{
+	/* kTLS cannot send an empty record. Send the data content type as
+	 * the only byte of a record of type 0 instead, the receiver strips
+	 * the 0 as padding and sees an empty data record.
+	 */
+	char data_type = 0x17;
+	unsigned int j;
+
+	for (j = 0; j < variant->n[i]; j++)
+		ASSERT_EQ(tls_send_cmsg(fd, 0, &data_type, 1, 0), 1);
+	if (variant->data[i])
+		ASSERT_EQ(send(fd, "data", 4, 0), 4);
+}
+
+static void zero_len_run_expect(struct __test_metadata *_metadata,
+				ssize_t ret, int expected)
+{
+	EXPECT_EQ(ret, expected >= 0 ? expected : -1);
+	if (expected < 0)
+		EXPECT_EQ(errno, -expected);
+}
+
+TEST_F(zero_len_run, recv)
+{
+	char buf[16];
+	int i;
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	for (i = 0; i < 2; i++) {
+		zero_len_run_send(_metadata, self->fd, variant, i);
+		zero_len_run_expect(_metadata,
+				    recv(self->cfd, buf, sizeof(buf),
+					 MSG_DONTWAIT),
+				    variant->ret[i]);
+	}
+}
+
+TEST_F(zero_len_run, splice)
+{
+	int i, p[2];
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	ASSERT_GE(pipe(p), 0);
+	for (i = 0; i < 2; i++) {
+		zero_len_run_send(_metadata, self->fd, variant, i);
+		zero_len_run_expect(_metadata,
+				    splice(self->cfd, NULL, p[1], NULL, 16,
+					   SPLICE_F_NONBLOCK),
+				    variant->ret[i]);
+	}
+	close(p[0]);
+	close(p[1]);
+}
+
 FIXTURE(tls_err)
 {
 	int fd, cfd;
-- 
2.56.0

Re: [PATCH net v2 0/5] net: tls: break the connection after a burst of empty data records

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 11 Nov 2026 10:55:17 -0800 you wrote:
> Deal with overload of zero-length TLS records. This is the next iteration
> of Chuck's series. I tried to make it as simple as possible - break the
> connection if sender floods us with zero-length records. Consume / discard
> those records even on PEEK to avoid infinite rx_list growth.
> 
> https://lore.kernel.org/20261019-tls-follow-on-v6-0-ab01c4ecca0c@kernel.org
> 
> [...]

Here is the summary with links:
  - [net,v2,1/5] tls: Break the connection after a burst of empty data records
    https://git.kernel.org/netdev/net/c/201f0531e413
  - [net,v2,2/5] tls: Consume empty data records in tls_sw_splice_read()
    https://git.kernel.org/netdev/net/c/db6a6c6863bd
  - [net,v2,3/5] selftests: tls: Skip the zero_len tests when TLS is unavailable
    https://git.kernel.org/netdev/net/c/77389a6d30a2
  - [net,v2,4/5] selftests: tls: Extend coverage for zero-length records
    https://git.kernel.org/netdev/net/c/512f2cb73664
  - [net,v2,5/5] selftests: tls: Cover the cap on consecutive empty records
    https://git.kernel.org/netdev/net/c/e6dfa7bfdf15

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net v2 1/5] tls: Break the connection after a burst of empty data records

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] ctx->empty_run counts only consecutive empty DATA records.
- [Low] Documentation/networking/tls.rst (line 123) still says only
  '``EBADMSG`` is returned if decryption failed for any other reason.'…
- [Low] In tls_sw_recvmsg() the new check 'if (is_peek && chunk)'
  (net/tls/tls_sw.c:1963) applies to every record type, not only…

--

tls: Break the connection after a burst of empty data records

This patch adds a per-reader counter of consecutive empty TLS data
records. Once more than 128 are decrypted without the reader sleeping or
returning to its caller, it aborts the strparser with EBADMSG. It also
makes tls_sw_recvmsg() decrypt empty records synchronously, and consume
them rather than queue them on rx_list when peeking.

Link: https://lore.kernel.org/all/20261019-tls-follow-on-v6-0-ab01c4ecca0c@kernel.org/

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 23eb94d9da6ee..65e578c8c6124 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c

[ ... ]

> @@ -1582,6 +1584,19 @@ static int tls_rx_one_record(struct sock *sk, struct msghdr *msg,
>  	rxm->full_len -= prot->overhead_size;
>  	tls_advance_record_sn(sk, prot, &tls_ctx->rx);
>  
> +	/* A peer may send empty data records, e.g. as cover traffic, but
> +	 * a burst of them traps the reader in the kernel. Treat more than
> +	 * 128 in a row, without the reader sleeping or returning to the
> +	 * caller in between, as a protocol violation. Stop the strparser
> +	 * before the next record gets parsed, so the reader finds the
> +	 * error in tls_rx_rec_wait().
> +	 */
> +	if (rxm->full_len ||
> +	    tls_msg(darg->skb)->control != TLS_RECORD_TYPE_DATA)
> +		ctx->empty_run = 0;

[Severity: Medium]
Can a peer get around the cap by mixing in non-empty records? Any record
with a non-zero rxm->full_len resets ctx->empty_run. So a peer that sends
repeated groups like this never triggers the abort:

  [128 empty data records][1 one-byte data record]

With that pattern, each byte delivered costs the reader 129 decryptions,
but the loop bounds only advance by one byte per group. In
tls_sw_recvmsg() the loop is:

  while (len && (decrypted + copied < target || tls_strp_msg_ready(ctx))) {

The sk_flush_backlog() calls in tls_read_flush_backlog() and
tls_rx_rec_wait() keep feeding in new records, and signals are only
checked when the reader sleeps. One recvmsg() with a 1 MiB buffer could
process around 135M records. With plain one-byte records it would be
about 1M.

tls_sw_read_sock() loops on desc->count. svc_tcp_recvfrom() starts with:

  .count		= serv->sv_max_mesg + sizeof(rpc_fraghdr),

and only lowers it by the bytes consumed. That gives an NFS-over-TLS
client the same ~129x amplification while the socket lock is held.
nvme-tcp uses rd_desc.count = 1 and never lowers it, so for nvme-tcp the
interleaved stream looks as unbounded as before.

The commit message says empty records "advance neither loop bound" and
keep the reader "in the kernel for as long as they arrive". With
interleaving, empty records are still 128/129 of the work, and the bounds
barely advance.

Would a per-call budget (empty records per reader call, or per byte
delivered), or a signal/need_resched check, be needed to close this?

> +	else if (unlikely(++ctx->empty_run > 128))
> +		tls_strp_abort_strp(&ctx->strp, -EBADMSG);

[Severity: Low]
Should Documentation/networking/tls.rst be updated for this new error
case? It currently says only:

  EBADMSG is returned if decryption failed for any other reason.

After this change, a record that decrypts and authenticates fine can also
cause EBADMSG. On the 129th consecutive empty data record,
tls_rx_one_record() calls tls_strp_abort_strp(). That sets sk->sk_err and
stops the strparser for good. Every later recvmsg(), splice() or
read_sock caller then gets EBADMSG from tls_rx_rec_wait().

Reusing EBADMSG matches existing practice, since tls_rx_msg_size()
already returns it when data_len < cipher_overhead. However, the
128-record limit and the fact that it breaks the connection permanently
are not documented anywhere that userspace TLS libraries would find them.

> +
>  	return tls_check_pending_rekey(sk, tls_ctx, darg->skb);
>  }

[ ... ]

> @@ -1939,7 +1960,7 @@ int tls_sw_recvmsg(struct sock *sk,
>  			if (err < 0)
>  				goto put_on_rx_list_err;
>  
> -			if (is_peek) {
> +			if (is_peek && chunk) {
>  				peeked += chunk;
>  				goto put_on_rx_list;
>  			}

[Severity: Low]
Does this also consume empty non-data records under MSG_PEEK? The chunk
check applies to every record type, not just TLS_RECORD_TYPE_DATA.

Take an empty alert or handshake record that is read with MSG_PEEK.
full_len is 0 after decryption, which can happen with TLS 1.2 when
data_len == cipher_overhead, or with a TLS 1.3 inner type on an empty
body. Then:

  chunk is 0, so "async && chunk" is false
  skb_copy_datagram_msg() copies 0 bytes
  "is_peek && chunk" is false
  partially_consumed is false

The code then falls through to consume_skb(skb) and frees the record.

Before this patch, a peek always queued the skb on rx_list. The next
non-peek recvmsg() returned it from process_rx_list() along with its
TLS_GET_RECORD_TYPE cmsg. Now the next real read skips straight to the
following record.

The pile-up described in the commit message doesn't seem to apply to
non-data records. If one is already on rx_list, process_rx_list() sets
control != TLS_RECORD_TYPE_DATA and tls_sw_recvmsg() jumps to end, so
repeated peeks can't stack them up. The counter in tls_rx_one_record()
also only counts data records.

Would limiting the new condition to TLS_RECORD_TYPE_DATA records keep
the old peek behaviour for control records?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111185522.2859004-1-kuba%40kernel.org