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

[PATCH net 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

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 | 348 ++++++++++++++++++++++++++++--
 3 files changed, 385 insertions(+), 41 deletions(-)

-- 
2.56.0

[PATCH net 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.

Attempt the simplest possible fix, which is to break the connection
after 128 consecutive empty data records without sleeping or returning
to the caller.

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()")
Signed-off-by: Jakub Kicinski <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 771e02bff519..adf3b26b8a75 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 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>
---
Jakub:
 - rewrite as a loop to avoid the non-error-handling goto
---
 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 adf3b26b8a75..b05b9e76bd25 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 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>
---
 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 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. An unfixed kernel
returns 0 from splice().

Signed-off-by: Chuck Lever <cel@kernel.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/net/tls.c | 221 +++++++++++++++++++++++++++---
 1 file changed, 204 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index ca47576220d3..ed7feeac1cc4 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,172 @@ 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.
+ */
+TEST_F(zero_len_peek, test)
+{
+	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);
+		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 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.

Signed-off-by: Jakub Kicinski <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 ed7feeac1cc4..48409f06a975 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2889,6 +2889,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 0/5] net: tls: break the connection after a burst of empty data records

[Chuck Lever]

In patch 4/5, zero_len_peek lost the poll() check that v6 had after
the EAGAIN case. Without the poll() check, the 0data_0data_0data
variant passes on a kernel that lacks 1/5 when decryption is
synchronous. The peek queues all three empty records on rx_list and
recv() still returns EAGAIN. With patch 1/5 applied, the peek consumes
the empty records, so poll() reports the socket as not readable.

Restoring

	/* A record left on rx_list keeps the socket readable */
	EXPECT_EQ(poll(&pfd, 1, 0), 0);

after the errno check, along with the pfd declaration, would cover
the MSG_PEEK change in patch 1/5. But I have not run this series. If
the check fails with patch 1/5 applied, I'd like to understand why.

The loop rewrite in patch 2/5 looks fine to me. For patches 1/5 and
5/5:

Reviewed-by: Chuck Lever <cel@kernel.org>


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)