[PATCH net 0/9] net/tls: Receive-path fixes for zero-length data records

[PATCH net 0/9] net/tls: Receive-path fixes for zero-length data records

[Chuck Lever]

Commit 3be28e2c9cd0 ("net/tls: Consume empty data records in
tls_sw_read_sock()") fixed one reader. TLS 1.2 and TLS 1.3 both
permit a zero-length application_data record as a traffic-analysis
countermeasure (RFC 5246, Section 6.2.1; RFC 8446, Section 5.1), so
a peer that pads its stream emits them by design. The other two
software readers still mishandle one. splice(2) reports the empty
record as EOF and the caller tears down a connection that is still
live. recvmsg(2) neither advances nor returns, so a peer that
streams such records holds the caller in the kernel past SIGKILL
while rx_list grows without bound.

Which fix a reader gets depends on where it returns to. splice and
recvmsg return to userspace and drop the socket lock, so consuming
the record and testing signal_pending() is enough. read_sock runs
from kernel context and holds the lock across the whole call, so it
needs the return boundary a system call would otherwise supply: a
deadline armed by the first record that delivers no bytes and
disarmed by the first that delivers some (patch 1). Scoping that
cap to read_sock alone is deliberate, since a flood on the other
two paths costs the caller only its own scheduler time.

Two user-visible changes follow, both toward what a plain TCP
socket already does. splice(2) on a nonblocking socket, and
sendfile(2) from one, now return -EAGAIN where they used to block.
A splice that reaches a control record behind an empty one now
returns -EINVAL rather than the zero that was the false EOF.

No existing selftest variant reads a zero-length record back any
way but recv(2), so neither the splice path nor MSG_PEEK was
exercised against a record that decrypts to no payload. New
variants cover both.

---
Chuck Lever (9):
      net/tls: Bound time spent on no-data records in tls_sw_read_sock()
      net/tls: Consume empty data records in tls_sw_splice_read()
      net/tls: Fail tls_sw_splice_read() after a failed async decrypt
      net/tls: Honor O_NONBLOCK in tls_sw_splice_read()
      net/tls: Consume empty data records in tls_sw_recvmsg()
      selftests: tls: add peek and splice coverage for zero-length records
      selftests: tls: skip the zero_len tests when TLS is unavailable
      selftests: tls: cover splice on a nonblocking socket
      selftests: tls: cover splice after a failed decrypt

 net/tls/tls_sw.c                  | 113 +++++++++++--
 tools/testing/selftests/net/tls.c | 322 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 414 insertions(+), 21 deletions(-)
---
base-commit: 53658c6f3682967a5e76ed4bc7462c4bdcddaec3
change-id: 20260726-tls-follow-on-486f1ba8bbb0

Best regards,
--  
Chuck Lever <cel@kernel.org>

[PATCH net 1/9] net/tls: Bound time spent on no-data records in tls_sw_read_sock()

[Chuck Lever]

An empty TLS 1.3 data record delivers no payload, so it leaves
tls_sw_read_sock() in its loop without advancing the caller's read
descriptor. A peer that streams such records keeps the receive loop
running, and the socket lock held, for as long as they arrive.

Bound a run of such records, as net_rx_action() bounds a softirq
poll. The first record that delivers no bytes arms a deadline
TLS_RX_NODATA_NS ahead; any record that delivers bytes disarms it,
so a normal stream never trips it. Breaking out with nothing copied
returns zero, which a read_sock consumer reads as "no progress"
rather than EOF, so the connection stays up. Records left queued
draw no fresh sk_data_ready() of their own, so fire the socket's
current callback before returning.

Only tls_sw_read_sock() needs this bound. Its consumers drive the
receive loop from kernel context and hold the socket lock across
the whole call, so the deadline supplies the return boundary a
system call would otherwise provide. tls_sw_splice_read() and
tls_sw_recvmsg() return to userspace and drop the lock, so a flood
there costs the caller only its own scheduler time.

Fixes: 3be28e2c9cd0 ("net/tls: Consume empty data records in tls_sw_read_sock()")
Suggested-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index d4afc90fd796..d45c945a3d1d 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -37,6 +37,7 @@
 
 #include <linux/bug.h>
 #include <linux/sched/signal.h>
+#include <linux/timekeeping.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/splice.h>
@@ -2049,6 +2050,11 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	goto splice_read_end;
 }
 
+/* Bound the time that consecutive empty ingress data records keep
+ * the socket lock held without releasing it.
+ */
+#define TLS_RX_NODATA_NS NSEC_PER_MSEC
+
 int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
 		     sk_read_actor_t read_actor)
 {
@@ -2057,6 +2063,7 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
 	struct tls_prot_info *prot = &tls_ctx->prot_info;
 	struct strp_msg *rxm = NULL;
 	struct sk_buff *skb = NULL;
+	u64 nodata_deadline = 0;
 	struct sk_psock *psock;
 	size_t flushed_at = 0;
 	bool released = true;
@@ -2122,7 +2129,19 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
 		 * here instead.
 		 */
 		if (rxm->full_len == 0) {
+			err = 0;
 			consume_skb(skb);
+			if (!nodata_deadline) {
+				nodata_deadline = ktime_get_ns() +
+						  TLS_RX_NODATA_NS;
+			} else if (ktime_get_ns() >= nodata_deadline) {
+				/* Queued records raise no new sk_data_ready(),
+				 * and tls_rx_reader_release() announces only to
+				 * saved_data_ready(), not the consumer's own.
+				 */
+				sk->sk_data_ready(sk);
+				break;
+			}
 			continue;
 		}
 
@@ -2133,6 +2152,7 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
 			goto read_sock_requeue;
 		}
 		copied += used;
+		nodata_deadline = 0;
 		if (used < rxm->full_len) {
 			rxm->offset += used;
 			rxm->full_len -= used;

-- 
2.54.0

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

[Chuck Lever]

A peer may send a zero-length application_data record. After
decryption such a record has full_len == 0, so tls_sw_splice_read()
splices zero bytes and returns zero. A zero return from a splice
read signals EOF, and the caller tears down a connection that is
still live.

Consume such a record and fetch the next one, as tls_sw_recvmsg()
already does. A peer that streams empty records can hold the
splicing task in that loop, so test for a pending signal where the
record is consumed.

Where an empty record precedes a close_notify, splice(2) now returns
-EINVAL from the control-record test rather than the zero it
returned before. That zero was the false EOF this patch removes, and
a splice that reaches an alert record has always reported -EINVAL.

The open-coded test in tls_sw_read_sock() becomes
tls_rx_empty_data_rec(); the tls_sw_recvmsg() paths reuse it and
tls_rx_intr_errno() in the patches that follow.

Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 7 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index d45c945a3d1d..44ae0fad780d 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1788,6 +1788,24 @@ static void tls_rx_reader_unlock(struct sock *sk, struct tls_sw_context_rx *ctx)
 	release_sock(sk);
 }
 
+/* TLS 1.2 and TLS 1.3 both permit a zero-length application_data
+ * record as a traffic-analysis countermeasure (RFC 5246, Section
+ * 6.2.1; RFC 8446, Section 5.1).
+ */
+static bool tls_rx_empty_data_rec(int len, unsigned char control)
+{
+	return !len && control == TLS_RECORD_TYPE_DATA;
+}
+
+/* sock_intr_errno() maps the zero timeo of a reader that cannot wait
+ * to -EINTR, but such a reader has no blocking to interrupt. The rest
+ * of the receive side reports that case as -EAGAIN.
+ */
+static int tls_rx_intr_errno(long timeo)
+{
+	return timeo ? sock_intr_errno(timeo) : -EAGAIN;
+}
+
 int tls_sw_recvmsg(struct sock *sk,
 		   struct msghdr *msg,
 		   size_t len,
@@ -1991,6 +2009,7 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	struct sock *sk = sock->sk;
 	struct tls_msg *tlm;
 	struct sk_buff *skb;
+	bool released = true;
 	ssize_t copied = 0;
 	int chunk;
 	int err;
@@ -1999,13 +2018,14 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	if (err < 0)
 		return err;
 
+retry:
 	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, flags & SPLICE_F_NONBLOCK,
-				      true, false);
+				      released, false);
 		if (err <= 0)
 			goto splice_read_end;
 
@@ -2017,6 +2037,11 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 
 		tls_rx_rec_done(ctx);
 		skb = darg.skb;
+
+		/* The socket lock stays held to the retry, so the
+		 * anchor this wait loaded survives it.
+		 */
+		released = false;
 	}
 
 	rxm = strp_msg(skb);
@@ -2028,6 +2053,21 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 		goto splice_requeue;
 	}
 
+	/* Splicing an empty data record delivers zero bytes, which the
+	 * caller reads as EOF. tls_rx_rec_wait() skips its signal check
+	 * while a record is parsed, so test for a signal here.
+	 */
+	if (tls_rx_empty_data_rec(rxm->full_len, tlm->control)) {
+		long timeo = sock_rcvtimeo(sk, flags & SPLICE_F_NONBLOCK);
+
+		consume_skb(skb);
+		if (signal_pending(current)) {
+			err = tls_rx_intr_errno(timeo);
+			goto splice_read_end;
+		}
+		goto retry;
+	}
+
 	chunk = min_t(unsigned int, rxm->full_len, len);
 	copied = skb_splice_bits(skb, sk, rxm->offset, pipe, chunk, flags);
 	if (copied < 0)
@@ -2122,13 +2162,12 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
 			goto read_sock_requeue;
 		}
 
-		/* An empty data record (legal in TLS 1.3) gives a zero
-		 * read_actor return, indistinguishable from the consumer
-		 * stalling; the used <= 0 path would requeue it at the
-		 * head of rx_list and block all later records. Consume it
-		 * here instead.
+		/* An empty data record gives a zero read_actor return,
+		 * indistinguishable from the consumer stalling; the
+		 * used <= 0 path would requeue it at the head of rx_list
+		 * and block all later records. Consume it here instead.
 		 */
-		if (rxm->full_len == 0) {
+		if (tls_rx_empty_data_rec(rxm->full_len, tlm->control)) {
 			err = 0;
 			consume_skb(skb);
 			if (!nodata_deadline) {

-- 
2.54.0

[PATCH net 3/9] net/tls: Fail tls_sw_splice_read() after a failed async decrypt

[Chuck Lever]

When an async decrypt fails, tls_decrypt_done() records the error in
ctx->async_wait.err and calls tls_err_abort(), which stores it in
sk_err. tls_sw_recvmsg() and tls_sw_read_sock() each read
async_wait.err once they hold the reader lock and fail the call: a
record that did not authenticate breaks the connection.

tls_sw_splice_read() has no such check, and sk_err does not stand in
for one. tls_rx_rec_wait() tests sk_err only inside the loop it
skips whenever a record is already parsed, and the first reader to
reach sock_error() clears it, while async_wait.err persists. A
splice therefore keeps delivering records on a connection that
recvmsg() and read_sock() refuse to read.

Read async_wait.err in tls_sw_splice_read() as the other two readers
do.

Fixes: f314bfee81b1 ("tls: rx: return the already-copied data on crypto error")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 44ae0fad780d..99e9a9aa995c 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2018,6 +2018,11 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	if (err < 0)
 		return err;
 
+	/* If crypto failed the connection is broken */
+	err = ctx->async_wait.err;
+	if (err)
+		goto splice_read_end;
+
 retry:
 	if (!skb_queue_empty(&ctx->rx_list)) {
 		skb = __skb_dequeue(&ctx->rx_list);

-- 
2.54.0

[PATCH net 4/9] net/tls: Honor O_NONBLOCK in tls_sw_splice_read()

[Chuck Lever]

tls_sw_splice_read() currently derives its blocking behavior from
SPLICE_F_NONBLOCK alone; the socket's own O_NONBLOCK is invisible
to it. A splice(2) call without SPLICE_F_NONBLOCK on a nonblocking
socket therefore sleeps in tls_rx_rec_wait() until a record
arrives, where tcp_splice_read() reads sock->file->f_flags and
returns -EAGAIN.

The sleep is reachable through poll. tls_sw_sock_is_readable()
reports a socket readable while any record sits on rx_list,
including a zero-length data record that delivers no bytes to the
pipe. The splice path consumes it and waits for the next one. The
readiness test cannot screen such a record out, since the strparser
announces a record before decryption, when the plaintext length is
not yet known. An event loop that polls, then splices, stalls on
that connection and starves every other one it multiplexes.

Note that a caller that sets O_NONBLOCK and then splices without
SPLICE_F_NONBLOCK, taking that flag to govern only the pipe, now
gets -EAGAIN where it previously blocked. sendfile(2) from a TLS
socket changes the same way, because do_sendfile() leaves the input
file's O_NONBLOCK out of the splice flags. Both then behave as they
do on a plain TCP socket.

Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 99e9a9aa995c..f85d8a639731 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2011,10 +2011,14 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	struct sk_buff *skb;
 	bool released = true;
 	ssize_t copied = 0;
+	bool nonblock;
 	int chunk;
 	int err;
 
-	err = tls_rx_reader_lock(sk, ctx, flags & SPLICE_F_NONBLOCK);
+	nonblock = (flags & SPLICE_F_NONBLOCK) ||
+		   (sock->file->f_flags & O_NONBLOCK);
+
+	err = tls_rx_reader_lock(sk, ctx, nonblock);
 	if (err < 0)
 		return err;
 
@@ -2029,8 +2033,7 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	} else {
 		struct tls_decrypt_arg darg;
 
-		err = tls_rx_rec_wait(sk, flags & SPLICE_F_NONBLOCK,
-				      released, false);
+		err = tls_rx_rec_wait(sk, nonblock, released, false);
 		if (err <= 0)
 			goto splice_read_end;
 
@@ -2063,7 +2066,7 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	 * while a record is parsed, so test for a signal here.
 	 */
 	if (tls_rx_empty_data_rec(rxm->full_len, tlm->control)) {
-		long timeo = sock_rcvtimeo(sk, flags & SPLICE_F_NONBLOCK);
+		long timeo = sock_rcvtimeo(sk, nonblock);
 
 		consume_skb(skb);
 		if (signal_pending(current)) {

-- 
2.54.0

[PATCH net 5/9] net/tls: Consume empty data records in tls_sw_recvmsg()

[Chuck Lever]

TLS 1.2 and TLS 1.3 both permit zero-length application_data
records as a traffic-analysis countermeasure (RFC 5246, Section
6.2.1; RFC 8446, Section 5.1). Such a record decrypts to
full_len == 0, so every arm of the receive loop reaches
"decrypted += chunk" and "len -= chunk" with chunk == 0: len
never reaches zero, and tls_strp_msg_ready() holds the second
loop term true while the peer keeps records arriving. The peek
arm and the async arm also queue each record on rx_list, which
then grows without bound. tls_rx_rec_wait() returns without
waiting whenever a record is already parsed, so its signal check
never runs, and no other test in the loop consults
signal_pending(). A peer streaming empty records therefore holds
the caller in recvmsg(), unresponsive to SIGKILL, until it stops.

Consume an empty data record as soon as the receive loop has it,
before the paths diverge on darg.zc, and test for a pending
signal there. Freeing the record's skb requires its decryption to
have completed, so an empty record is no longer decrypted
asynchronously, and the new branch sets MSG_EOR itself because
the record no longer reaches the assignment at the bottom of the
loop. Bytes already received take precedence over the signal:
they are returned, and the signal is handled when the caller next
enters the kernel.

Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index f85d8a639731..0e76b31b1291 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1877,9 +1877,12 @@ 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 if it
+		 * is empty: the receive loop frees an empty record's skb,
+		 * so its decryption must have completed.
+		 */
 		if (tlm->control == TLS_RECORD_TYPE_DATA)
-			darg.async = ctx->async_capable;
+			darg.async = ctx->async_capable && to_decrypt;
 		else
 			darg.async = false;
 
@@ -1915,6 +1918,27 @@ int tls_sw_recvmsg(struct sock *sk,
 		chunk = rxm->full_len;
 		tls_rx_rec_done(ctx);
 
+		/* An empty record advances neither loop bound, so a flood
+		 * of them can be interrupted only here. On the zero-copy
+		 * path darg.skb is the strparser anchor, already released
+		 * by tls_rx_rec_done().
+		 */
+		if (tls_rx_empty_data_rec(chunk, control)) {
+			long timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+
+			if (!darg.zc)
+				consume_skb(darg.skb);
+
+			/* An empty record still marks a boundary. */
+			msg->msg_flags |= MSG_EOR;
+
+			if (signal_pending(current)) {
+				err = tls_rx_intr_errno(timeo);
+				goto recv_end;
+			}
+			continue;
+		}
+
 		if (!darg.zc) {
 			bool partially_consumed = chunk > len;
 			struct sk_buff *skb = darg.skb;

-- 
2.54.0

[PATCH net 6/9] selftests: tls: add peek and splice coverage for zero-length records

[Chuck Lever]

The zero_len fixture injects raw pre-encrypted records over a socket
carrying a TLS_RX key only, and its record table already holds
zero-length application_data records. Every variant reads them back
with a plain recv(), so neither the splice path nor MSG_PEEK is
exercised against a record that decrypts to no payload.

Add zero_len_splice. An empty data record splices zero bytes, and a
zero return from splice() means EOF, so a peer that sends one ends a
connection that is still live. Its variants expect the payload's
length when one sits behind a run of empty records, EAGAIN when
nothing does, and EINVAL when a control record does; an unfixed
kernel reports 0 for all three. Add zero_len_peek, which checks that
a peek reaches the payload behind such a run and leaves it in place
for the read that follows. That one reproduces no failure: the
unbounded rx_list growth it accompanies needs a sustained flood that
three fixed-sequence records cannot supply.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 tools/testing/selftests/net/tls.c | 213 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 213 insertions(+)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index cbdd3ea28b99..089e940ac43d 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2573,6 +2573,219 @@ TEST_F(zero_len, test)
 	}
 };
 
+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 stays keyless; 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; 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_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
+ * rather than the zero return that means EOF.
+ */
+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);
+		if (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);
+
+	/* Peeking left the payload in place for the read that follows */
+	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);
+	if (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);
+}
+
+/* An empty data record splices zero bytes, which a splice caller reads
+ * as EOF. Splicing must skip past such a record to the payload behind
+ * it, and report EAGAIN when a run of them has no payload behind it.
+ * A control record behind the run reports EINVAL, the error splice
+ * already reports for a control record it meets first.
+ */
+TEST_F(zero_len_splice, test)
+{
+	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);
+		if (ret == -1)
+			EXPECT_EQ(errno, -variant->splice_ret);
+	} else {
+		/* Assert: a zero return, which is what an unfixed kernel
+		 * gives here, leaves the pipe empty, and the read below
+		 * would then block 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);
+
+		/* Reaching the payload consumed the empty records ahead
+		 * of it rather than leaving them on the receive queue
+		 */
+		ret = recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT);
+		EXPECT_EQ(ret, -1);
+		if (ret == -1)
+			EXPECT_EQ(errno, EAGAIN);
+	}
+
+	close(p[0]);
+	close(p[1]);
+}
+
 FIXTURE(tls_err)
 {
 	int fd, cfd;

-- 
2.54.0

Re: [PATCH net 6/9] selftests: tls: add peek and splice coverage for zero-length records

[Sabrina Dubroca]

2026-07-26, 20:33:34 -0400, Chuck Lever wrote:
> +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 stays keyless; 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; 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;
> +}

nit: those are identical to the existing FIXTURE_SETUP(zero_len) and
the TX part of TEST_F(zero_len, test), so they should also be used
there.

Or since you're reusing similar variants (for both zero_len_peek and
zero_len_splice), maybe just add the new TEST_F chunks as subtests of
the existing zero_len FIXTURE? There's only partial overlap in the
current version, but there should be a set of {recv_ret, peek_ret,
splice_ret} that works for each.


-- 
Sabrina

[PATCH net 7/9] selftests: tls: skip the zero_len tests when TLS is unavailable

[Chuck Lever]

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 hands the
ciphertext back verbatim: every one of the eight variants reports
FAIL on a machine that merely lacks TLS, and those failures mask any
real regression in the same run.

Skip the test when the fixture recorded notls, the convention the
other fixtures in this file already follow.

Fixes: a61a3e961baf ("selftests: tls: add tests for zero-length records")
Signed-off-by: Chuck Lever <cel@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 089e940ac43d..af47d9a6098c 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2544,6 +2544,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.54.0

[PATCH net 8/9] selftests: tls: cover splice on a nonblocking socket

[Chuck Lever]

Every existing splice call in this file either passes
SPLICE_F_NONBLOCK or runs on a blocking socket, so nothing exercises
the socket's own O_NONBLOCK on the splice path. A change that stops
consulting sock->file->f_flags puts splice(2) and sendfile(2) back to
sleeping in tls_rx_rec_wait() while a peer streams empty records, and
the suite still reports pass.

Run the zero-length record variants a second time with O_NONBLOCK set
on the socket and SPLICE_F_NONBLOCK left out of the splice flags. The
two forms are required to reach the same outcome, so the existing
per-variant expectations carry over unchanged. A kernel that ignores
the socket flag sleeps in the EAGAIN variant until the harness
timeout rather than returning.

Only the flags the two forms pass differ, so the body of the
existing test moves into zero_len_do_splice() and both call it. The
check that a successful splice consumed the empty records ahead of
the payload now covers the nonblocking socket as well.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 tools/testing/selftests/net/tls.c | 41 +++++++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index af47d9a6098c..7c178541edf4 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2743,23 +2743,24 @@ FIXTURE_TEARDOWN(zero_len_splice)
  * A control record behind the run reports EINVAL, the error splice
  * already reports for a control record it meets first.
  */
-TEST_F(zero_len_splice, test)
+static void
+zero_len_do_splice(struct __test_metadata *_metadata,
+		   struct _test_data_zero_len_splice *self,
+		   const struct _fixture_variant_zero_len_splice *variant,
+		   unsigned int splice_flags)
 {
 	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);
+			     splice_flags);
 		EXPECT_EQ(ret, -1);
 		if (ret == -1)
 			EXPECT_EQ(errno, -variant->splice_ret);
@@ -2769,7 +2770,7 @@ TEST_F(zero_len_splice, test)
 		 * would then block until the harness timeout.
 		 */
 		ASSERT_EQ(splice(self->cfd, NULL, p[1], NULL, sizeof(buf),
-				 SPLICE_F_NONBLOCK), variant->splice_ret);
+				 splice_flags), variant->splice_ret);
 		ret = read(p[0], buf, sizeof(buf));
 		EXPECT_EQ(ret, variant->splice_ret);
 		if (ret == variant->splice_ret)
@@ -2789,6 +2790,34 @@ TEST_F(zero_len_splice, test)
 	close(p[1]);
 }
 
+TEST_F(zero_len_splice, test)
+{
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	zero_len_do_splice(_metadata, self, variant, SPLICE_F_NONBLOCK);
+}
+
+/* The socket's own O_NONBLOCK governs the record wait, as it does on a
+ * plain TCP socket, so a splice that omits SPLICE_F_NONBLOCK reaches
+ * the same outcome as one that sets it. An unfixed kernel derives the
+ * wait from SPLICE_F_NONBLOCK alone and sleeps here until the harness
+ * timeout.
+ */
+TEST_F(zero_len_splice, nonblock_socket)
+{
+	int sflags;
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	sflags = fcntl(self->cfd, F_GETFL, 0);
+	ASSERT_GE(sflags, 0);
+	ASSERT_EQ(fcntl(self->cfd, F_SETFL, sflags | O_NONBLOCK), 0);
+
+	zero_len_do_splice(_metadata, self, variant, 0);
+}
+
 FIXTURE(tls_err)
 {
 	int fd, cfd;

-- 
2.54.0

[PATCH net 9/9] selftests: tls: cover splice after a failed decrypt

[Chuck Lever]

Nothing in this file splices a socket whose last decrypt failed, so
the check that fails tls_sw_splice_read() on a broken connection can
be removed without a test noticing. Such a splice hands the
application plaintext that recvmsg() and read_sock() already refuse
to return.

Extend the bad_auth pattern: corrupt an authenticated record, confirm
recvmsg() reports EBADMSG, then splice the same socket and require
EBADMSG again. A synchronous decrypt fails again on the still-queued
record, so only an asynchronous decrypt, which needs TLS 1.2 and an
AEAD advertising CRYPTO_ALG_ASYNC, reaches EBADMSG solely through the
recorded-failure check.

bad_auth builds the same corrupted record, so its construction moves
into a helper the two tests share.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 tools/testing/selftests/net/tls.c | 77 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 67 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index 7c178541edf4..8b68dbfcc592 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -24,6 +24,7 @@
 #include "kselftest_harness.h"
 
 #define TLS_PAYLOAD_MAX_LEN 16384
+#define TLS_HDR_LEN 5
 #define SOL_TLS 282
 
 static int fips_enabled;
@@ -2883,28 +2884,85 @@ TEST_F(tls_err, bad_rec)
 	EXPECT_EQ(errno, EAGAIN);
 }
 
+/* Encrypt a record on the TX-only socket pair, corrupt its last
+ * byte, and hand the result to the socket under test. cfd carries a
+ * byte stream, so one recv() can return part of a record: take the
+ * fragment length from the record header and wait for the remainder.
+ */
+static void tls_send_bad_auth(struct __test_metadata *_metadata,
+			      int fd, int cfd, int fd2)
+{
+	char buf[128];
+	int len;
+
+	memrnd(buf, sizeof(buf) / 2);
+	ASSERT_EQ(send(fd, buf, sizeof(buf) / 2, 0), sizeof(buf) / 2);
+
+	ASSERT_EQ(recv(cfd, buf, TLS_HDR_LEN, MSG_WAITALL), TLS_HDR_LEN);
+
+	len = ((unsigned char)buf[3] << 8) | (unsigned char)buf[4];
+	ASSERT_GT(len, 0);
+	ASSERT_LE(len, (int)sizeof(buf) - TLS_HDR_LEN);
+
+	ASSERT_EQ(recv(cfd, buf + TLS_HDR_LEN, len, MSG_WAITALL), len);
+
+	buf[TLS_HDR_LEN + len - 1]++;
+
+	ASSERT_EQ(send(fd2, buf, TLS_HDR_LEN + len, 0), TLS_HDR_LEN + len);
+}
+
 TEST_F(tls_err, bad_auth)
 {
 	char buf[128];
-	int n;
 
 	if (self->notls)
 		SKIP(return, "no TLS support");
 
-	memrnd(buf, sizeof(buf) / 2);
-	EXPECT_EQ(send(self->fd, buf, sizeof(buf) / 2, 0), sizeof(buf) / 2);
-	n = recv(self->cfd, buf, sizeof(buf), 0);
-	EXPECT_GT(n, sizeof(buf) / 2);
+	tls_send_bad_auth(_metadata, self->fd, self->cfd, self->fd2);
 
-	buf[n - 1]++;
-
-	EXPECT_EQ(send(self->fd2, buf, n, 0), n);
 	EXPECT_EQ(recv(self->cfd2, buf, sizeof(buf), 0), -1);
 	EXPECT_EQ(errno, EBADMSG);
 	EXPECT_EQ(recv(self->cfd2, buf, sizeof(buf), 0), -1);
 	EXPECT_EQ(errno, EBADMSG);
 }
 
+/* A record that did not authenticate breaks the connection for every
+ * reader, splice included.
+ *
+ * The two decrypt paths reach that result differently. A synchronous
+ * decrypt leaves the record parsed, so the splice re-runs the decrypt
+ * and fails on the record itself; the ctx->async_wait.err check in
+ * tls_sw_splice_read() is not what stops it. Only an asynchronous
+ * decrypt, which needs a TLS 1.2 socket and an AEAD advertising
+ * CRYPTO_ALG_ASYNC, consumes the record before the failure is
+ * recorded, leaving that check the sole reason the splice fails.
+ */
+TEST_F(tls_err, bad_auth_splice)
+{
+	char buf[128];
+	ssize_t ret;
+	int p[2];
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	tls_send_bad_auth(_metadata, self->fd, self->cfd, self->fd2);
+
+	EXPECT_EQ(recv(self->cfd2, buf, sizeof(buf), 0), -1);
+	EXPECT_EQ(errno, EBADMSG);
+
+	ASSERT_GE(pipe(p), 0);
+
+	ret = splice(self->cfd2, NULL, p[1], NULL, sizeof(buf),
+		     SPLICE_F_NONBLOCK);
+	EXPECT_EQ(ret, -1);
+	if (ret == -1)
+		EXPECT_EQ(errno, EBADMSG);
+
+	close(p[0]);
+	close(p[1]);
+}
+
 TEST_F(tls_err, bad_in_large_read)
 {
 	char txt[3][64];
@@ -3160,7 +3218,6 @@ static size_t parse_tls_records(struct __test_metadata *_metadata,
 {
 	const __u8 *rec = rx_buf;
 	size_t total_plaintext_rx = 0;
-	const __u8 rec_header_len = 5;
 
 	while (rec < rx_buf + rx_len) {
 		__u16 record_payload_len;
@@ -3180,7 +3237,7 @@ static size_t parse_tls_records(struct __test_metadata *_metadata,
 
 		/* Plaintext must not exceed the specified limit */
 		ASSERT_LE(plaintext_len, max_payload_len);
-		rec += rec_header_len + record_payload_len;
+		rec += TLS_HDR_LEN + record_payload_len;
 	}
 
 	return total_plaintext_rx;

-- 
2.54.0

Re: [PATCH net 9/9] selftests: tls: cover splice after a failed decrypt

[Sabrina Dubroca]

2026-07-26, 20:33:37 -0400, Chuck Lever wrote:
> Nothing in this file splices a socket whose last decrypt failed, so
> the check that fails tls_sw_splice_read() on a broken connection can
> be removed without a test noticing. Such a splice hands the
> application plaintext that recvmsg() and read_sock() already refuse
> to return.
> 
> Extend the bad_auth pattern: corrupt an authenticated record, confirm
> recvmsg() reports EBADMSG, then splice the same socket and require
> EBADMSG again. A synchronous decrypt fails again on the still-queued
> record, so only an asynchronous decrypt, which needs TLS 1.2 and an
> AEAD advertising CRYPTO_ALG_ASYNC, reaches EBADMSG solely through the
> recorded-failure check.

async crypto is going away soon, I don't think it's worth mentioning
it. (but first I'm finishing the surgery to get rid of skmsg)

> bad_auth builds the same corrupted record, so its construction moves
> into a helper the two tests share.
> 
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
>  tools/testing/selftests/net/tls.c | 77 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 67 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
> index 7c178541edf4..8b68dbfcc592 100644
> --- a/tools/testing/selftests/net/tls.c
> +++ b/tools/testing/selftests/net/tls.c
> @@ -24,6 +24,7 @@
>  #include "kselftest_harness.h"
>  
>  #define TLS_PAYLOAD_MAX_LEN 16384
> +#define TLS_HDR_LEN 5
>  #define SOL_TLS 282
>  
>  static int fips_enabled;
> @@ -2883,28 +2884,85 @@ TEST_F(tls_err, bad_rec)
>  	EXPECT_EQ(errno, EAGAIN);
>  }
>  
> +/* Encrypt a record on the TX-only socket pair, corrupt its last
> + * byte, and hand the result to the socket under test. cfd carries a
> + * byte stream, so one recv() can return part of a record: take the
> + * fragment length from the record header and wait for the remainder.
> + */

I'm not a fan of adding a 5-line comment for every 10-line function.

[...]
> +TEST_F(tls_err, bad_auth_splice)
> +{
> +	char buf[128];
> +	ssize_t ret;
> +	int p[2];
> +
> +	if (self->notls)
> +		SKIP(return, "no TLS support");
> +
> +	tls_send_bad_auth(_metadata, self->fd, self->cfd, self->fd2);
> +
> +	EXPECT_EQ(recv(self->cfd2, buf, sizeof(buf), 0), -1);
> +	EXPECT_EQ(errno, EBADMSG);
> +
> +	ASSERT_GE(pipe(p), 0);
> +
> +	ret = splice(self->cfd2, NULL, p[1], NULL, sizeof(buf),
> +		     SPLICE_F_NONBLOCK);
> +	EXPECT_EQ(ret, -1);
> +	if (ret == -1)

nit: The rest of the tls tests just check errno when an error was
expected. If we didn't get -1, the errno check will probably fail too,
but at this point the test has already failed so IMO it's not worth
adding extra conditionals.

> +		EXPECT_EQ(errno, EBADMSG);
> +
> +	close(p[0]);
> +	close(p[1]);
> +}

-- 
Sabrina