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

[PATCH net-next v2 0/8] 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 them. splice(2) reports an empty
record as EOF, and the caller tears down a connection that is still
live. recvmsg(2) makes no progress on one, so a peer that streams
them holds the caller in the kernel past SIGKILL. With MSG_PEEK or
async decryption, each record is also queued on rx_list, which
grows without bound.

This series supersedes "[PATCH net] tls: skip empty data records in
tls_sw_splice_read()", which fixes the splice case alone:
https://lore.kernel.org/netdev/20260930052636.166007-1-qingfang.deng@linux.dev/

Which fix a reader gets depends on its caller. splice(2) and
recvmsg(2) are system calls, so consuming the record and testing
signal_pending() is enough. A signal ends the call. Only
tls_sw_read_sock() needs a bound of its own. Its callers hold the
socket lock across the whole call and cannot act on a signal, so
nothing else can stop the loop. A count of consecutive records that
deliver no bytes supplies the bound (patch 1). The count is scoped
to read_sock deliberately. A flood on the other two paths costs the
caller CPU time and nothing else.

The series changes user-visible behavior. splice(2) on a
nonblocking socket, and sendfile(2) from one, now return -EAGAIN
where they used to block, as they do on a plain TCP socket. A
splice that reaches a control record behind an empty one now
returns -EINVAL rather than the zero that was the false EOF. A
nonblocking recvmsg(2) or splice(2) that has copied nothing and
finds a signal pending after an empty record returns -EINTR.

SO_RCVTIMEO does not bound these calls. It is applied at the
reader lock and again on each call to tls_rx_rec_wait(), so a call
that keeps retrying can wait past it. recvmsg(2) behaves this way
today. The retry added to splice(2) extends the same behavior to
that path.

Tested on x86_64. The tls selftest suite passes, 935 tests with
no skips.

---
Changes in v2:
- Bound no-data records by count, not elapsed time (Jakub).
- Drop the tls_rx_empty_data_rec() helper (Sabrina).
- Keep the strparser anchor out of tls_sw.c comments (Sabrina).
- Split the recvmsg signal test into its own patch.
- Drop tls_rx_intr_errno(); a nonblocking reader now gets -EINTR.
- Say why do_splice() misses the socket's O_NONBLOCK (Sabrina).
- Point the recvmsg patch's Fixes: at the commit that added rx_list.
- Reuse the new zero_len helpers in the existing fixture (Sabrina).
- Check errno unconditionally in the zero_len tests (Sabrina).
- Split the splice crypto-error fix and its test out (Jakub):
  https://patch.msgid.link/20260806-tls-splice-crypto-fix-v1-0-a2624005a286@kernel.org
- Link to v1: https://patch.msgid.link/20260726-tls-follow-on-v1-0-99bf4cc1c729@kernel.org

---
Chuck Lever (8):
      tls: bound consecutive no-data records in tls_sw_read_sock()
      tls: check for a pending signal after an empty record
      tls: consume empty data records in tls_sw_splice_read()
      tls: honor O_NONBLOCK in tls_sw_splice_read()
      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

 net/tls/tls_sw.c                  |  78 ++++++++++--
 tools/testing/selftests/net/tls.c | 255 +++++++++++++++++++++++++++++++++++---
 2 files changed, 306 insertions(+), 27 deletions(-)
---
base-commit: f49defea7668d8c68ec19fa085ef3da6075561c7
change-id: 20260726-tls-follow-on-486f1ba8bbb0

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

[PATCH net-next v2 1/8] tls: bound consecutive no-data records in tls_sw_read_sock()

[Chuck Lever]

A zero-length application_data record delivers no payload, so
tls_sw_read_sock() never runs the read_actor for one and nothing
decrements desc->count. A peer that streams such records keeps the
loop running, with the socket lock held, for as long as they arrive.
The caller cannot bound the run because read_sock() has not
returned.

Stop after TLS_RX_NODATA_LIMIT consecutive records that deliver no
bytes. Any record that delivers bytes resets the count. Stopping
with nothing copied returns zero, which a read_sock consumer takes
as no progress rather than EOF. Records left queued raise no
further sk_data_ready(), so call the socket's callback before
returning.

Fixes: 3be28e2c9cd0 ("net/tls: Consume empty data records in tls_sw_read_sock()")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index d1ad31986cf2..c78471c53f2f 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2070,6 +2070,8 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	goto splice_read_end;
 }
 
+#define TLS_RX_NODATA_LIMIT	16
+
 int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
 		     sk_read_actor_t read_actor)
 {
@@ -2078,6 +2080,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;
+	unsigned int nodata = 0;
 	struct sk_psock *psock;
 	size_t flushed_at = 0;
 	bool released = true;
@@ -2136,14 +2139,22 @@ 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) {
+			err = 0;
 			consume_skb(skb);
+			if (++nodata >= TLS_RX_NODATA_LIMIT) {
+				/* tls_rx_reader_release() calls
+				 * saved_data_ready(), not the callback a
+				 * consumer installs after the handshake.
+				 */
+				sk->sk_data_ready(sk);
+				break;
+			}
 			continue;
 		}
 
@@ -2154,6 +2165,7 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
 			goto read_sock_requeue;
 		}
 		copied += used;
+		nodata = 0;
 		if (used < rxm->full_len) {
 			rxm->offset += used;
 			rxm->full_len -= used;

-- 
2.55.0

Re: [PATCH net-next v2 1/8] tls: bound consecutive no-data records in tls_sw_read_sock()

[netdev-bot+sashiko]

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

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

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).

- [High] The new limit can deadlock a CPU through recursion.
- [Low] The new call loads sk->sk_data_ready with a plain read and calls
  it, while another thread can rewrite the pointer.

--

tls: bound consecutive no-data records in tls_sw_read_sock()

This commit stops tls_sw_read_sock() once it has consumed
TLS_RX_NODATA_LIMIT (16) consecutive zero-length application_data
records in one call. A record that delivers data resets the count. When
the limit is reached, the loop breaks and calls sk->sk_data_ready() so
the records still queued are noticed.

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index d1ad31986cf2c..c78471c53f2f2 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c

[ ... ]

> @@ -2136,14 +2139,22 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,

[ ... ]

>  		if (rxm->full_len == 0) {
> +			err = 0;
>  			consume_skb(skb);
> +			if (++nodata >= TLS_RX_NODATA_LIMIT) {
> +				/* tls_rx_reader_release() calls
> +				 * saved_data_ready(), not the callback a
> +				 * consumer installs after the handshake.
> +				 */
> +				sk->sk_data_ready(sk);

[Severity: High]
Can this direct call to sk->sk_data_ready() re-enter tls_sw_read_sock()
and deadlock on sk_lock.slock?

When this line runs, ctx->reader_present is still 1 and the caller
still holds the socket lock.

Suppose KCM is attached to the TCP socket after TLS_RX has been set up.
kcm_attach() does not reject ULP sockets, and it installs
psock_data_ready() as csk->sk_data_ready. The KCM psock sits in
sk_user_data without SK_USER_DATA_PSOCK, so the sk_psock_get() check at
the top of tls_sw_read_sock() does not catch it.

In softirq the chain would look like this:

tcp_v4_rcv()
  bh_lock_sock(sk)
  tcp_data_ready()
    psock_data_ready()
      strp_data_ready()            /* socket not owned by user */
        strp_read_sock()
          tls_sw_read_sock()       /* reader_present = 1 */
            /* 16 empty records consumed */
            sk->sk_data_ready(sk)  /* psock_data_ready() */
              strp_data_ready()
                strp_read_sock()
                  tls_sw_read_sock()
                    tls_rx_reader_acquire()
                      sk_wait_event()
                        release_sock()
                          spin_lock_bh(&sk->sk_lock.slock)

The nested tls_rx_reader_acquire() sees reader_present == 1 and calls
sk_wait_event(). That calls release_sock() before the zero timeout is
checked. release_sock() then spins on sk_lock.slock, which this CPU
already holds through bh_lock_sock().

A remote peer could trigger this with 16 empty application_data records.
In TLS 1.3 that is about 352 bytes, which fits in one segment.

Before this patch, with TLS set up first and KCM second, the only
notification from inside the reader was tls_rx_reader_release() ->
tls_rx_msg_maybe_announce() -> ctx->saved_data_ready. That is the
callback that was installed before TLS (sock_def_readable here), so the
consumer's callback was never called re-entrantly. If KCM attaches
first, saved_data_ready is already psock_data_ready, so the TLS-then-KCM
ordering is the new exposure.

Would it work to defer the notification until the loop has exited and
reader_present has been cleared, or to queue it as work?

[Severity: Low]
Should this be READ_ONCE(sk->sk_data_ready)(sk), as the TCP call sites
use?

nvme_tcp_restore_sock_ops() does a plain store to sk->sk_data_ready
while holding only sk_callback_lock. __nvme_tcp_stop_queue() calls it
before cancel_work_sync(&queue->io_work). That means io_work can be
inside nvme_tcp_try_recv() -> tls_sw_read_sock() and load the pointer
at the same moment.

The result is an unannotated data race on the callback pointer, which
KCSAN would report.

> +				break;
> +			}
>  			continue;
>  		}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001-tls-follow-on-v2-0-2dd1947bb642%40kernel.org

Re: [PATCH net-next v2 1/8] tls: bound consecutive no-data records in tls_sw_read_sock()

[Chuck Lever]

On Mon, Oct 05, 2026, netdev-bot+sashiko@kernel.org wrote:
> > +			if (++nodata >= TLS_RX_NODATA_LIMIT) {
> > +				/* tls_rx_reader_release() calls
> > +				 * saved_data_ready(), not the callback a
> > +				 * consumer installs after the handshake.
> > +				 */
> > +				sk->sk_data_ready(sk);
>
> [Severity: High]
> Can this direct call to sk->sk_data_ready() re-enter tls_sw_read_sock()
> and deadlock on sk_lock.slock?

Yes. kcm_attach() accepts a TCP socket that already has TLS_RX set
up, and strp_data_ready() calls strp_read_sock() directly when the
socket is not owned by user. From softirq, the nested
tls_sw_read_sock() finds reader_present set, and sk_wait_event()
calls release_sock() before it looks at the zero timeout. I have
not reproduced the deadlock.

Making the call after tls_rx_reader_release() does not fix it. The
nested call then acquires the reader, consumes its own 16 empty
records, and recurses again, so the depth is set by the peer.

The notification has to leave the reader's call chain. I'll rework
it for v3.


> [Severity: Low]
> Should this be READ_ONCE(sk->sk_data_ready)(sk), as the TCP call sites
> use?

The direct call goes away with the rework above.

pw-bot: cr

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

[PATCH net-next v2 2/8] tls: check for a pending signal after an empty record

[Chuck Lever]

tls_rx_rec_wait() tests signal_pending() only after it sleeps, and
it skips the wait loop when a record is already parsed.
tls_sw_recvmsg() calls it once per record, and a zero-length
application_data record advances neither of its loop bounds. A
peer that streams such records holds the caller in recvmsg(),
unresponsive to SIGKILL, for as long as they arrive.

Test for a pending signal in tls_sw_recvmsg() before it fetches the
record after an empty one. A reader that has copied data returns
it, and a nonblocking reader gets -EINTR. The test stays out of
tls_rx_rec_wait() because tls_sw_read_sock() also calls it, from
callers that cannot act on a signal.

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

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index c78471c53f2f..ee50b9028264 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1824,6 +1824,7 @@ int tls_sw_recvmsg(struct sock *sk,
 	bool is_peek = flags & MSG_PEEK;
 	bool rx_more = false;
 	bool released = true;
+	bool nodata = false;
 	bool zc_capable;
 
 	if (unlikely(flags & MSG_ERRQUEUE))
@@ -1858,6 +1859,16 @@ int tls_sw_recvmsg(struct sock *sk,
 		struct tls_decrypt_arg darg;
 		int to_decrypt, chunk;
 
+		/* A run of empty records advances neither loop bound, and
+		 * tls_rx_rec_wait() tests for a signal only after it sleeps.
+		 */
+		if (nodata && signal_pending(current)) {
+			long timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+
+			err = sock_intr_errno(timeo);
+			goto recv_end;
+		}
+
 		err = tls_rx_rec_wait(sk, flags & MSG_DONTWAIT,
 				      released, !!(decrypted + copied));
 		if (err <= 0)
@@ -1910,6 +1921,7 @@ int tls_sw_recvmsg(struct sock *sk,
 		/* TLS 1.3 may have updated the length by more than overhead */
 		rxm = strp_msg(darg.skb);
 		chunk = rxm->full_len;
+		nodata = !chunk;
 		tls_rx_rec_done(ctx);
 
 		if (!darg.zc) {

-- 
2.55.0

Re: [PATCH net-next v2 2/8] tls: check for a pending signal after an empty record

[netdev-bot+sashiko]

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

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

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] In tls_sw_recvmsg() (net/tls/tls_sw.c), the new early exit `if
  (nodata && signal_pending(current)) { err = sock_intr_errno(timeo);…

--

tls: check for a pending signal after an empty record

This patch adds a signal_pending() check to the tls_sw_recvmsg() receive
loop. The check runs before the next record is fetched, if the previous
record was a zero-length application_data record. Without it, a peer
streaming empty records could keep the reader in recvmsg() and SIGKILL
would not interrupt it.

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index c78471c53f2f2..ee50b9028264c 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c

[ ... ]

> @@ -1858,6 +1859,16 @@ int tls_sw_recvmsg(struct sock *sk,
>  		struct tls_decrypt_arg darg;
>  		int to_decrypt, chunk;
>  
> +		/* A run of empty records advances neither loop bound, and
> +		 * tls_rx_rec_wait() tests for a signal only after it sleeps.
> +		 */
> +		if (nodata && signal_pending(current)) {
> +			long timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> +
> +			err = sock_intr_errno(timeo);
> +			goto recv_end;
> +		}

[Severity: Medium]
Can this errno be lost when the empty record that set nodata was decrypted
asynchronously?

At this commit, tls_sw_recvmsg() still enables async mode for every DATA
record, including empty ones:

	if (tlm->control == TLS_RECORD_TYPE_DATA)
		darg.async = ctx->async_capable;

With TLS 1.2 and a CRYPTO_ALG_ASYNC AEAD, an empty record gives chunk == 0,
so nodata becomes true. It then takes the async arm, where to_decrypt is 0:

	if (async) {
		chunk = min_t(int, to_decrypt, len);
		async_copy_bytes += chunk;
		...
		__skb_queue_tail(&ctx->rx_list, skb);
		...
		continue;
	}

On the next iteration the new check sets err to -ERESTARTSYS or -EINTR and
jumps to recv_end. Because async is true there, err is overwritten:

	err = process_rx_list(ctx, msg, &control, 0,
			      async_copy_bytes, is_peek, NULL);

async_copy_bytes is 0 here, so process_rx_list() returns 0. copied also
stays 0, so this path returns 0:

	return copied ? : err;

Would a blocking reader that catches a non-fatal signal see EOF instead of a
restart, and would a nonblocking reader get 0 instead of the -EINTR the
commit message describes?

The overwrite of err at recv_end already existed before this patch. This
patch adds an exit that can fire right after an empty async record.

Later in the series, "tls: consume empty data records in tls_sw_recvmsg()"
changes the assignment to:

	darg.async = ctx->async_capable && to_decrypt;

That keeps empty records out of async mode, so the false EOF does not happen
once the whole series is applied. It is still reachable at this commit,
which matters for bisection or if this patch is backported on its own.

>  		err = tls_rx_rec_wait(sk, flags & MSG_DONTWAIT,
>  				      released, !!(decrypted + copied));
>  		if (err <= 0)

[ ... ]

> @@ -1910,6 +1921,7 @@ int tls_sw_recvmsg(struct sock *sk,
>  		/* TLS 1.3 may have updated the length by more than overhead */
>  		rxm = strp_msg(darg.skb);
>  		chunk = rxm->full_len;
> +		nodata = !chunk;
>  		tls_rx_rec_done(ctx);
>  
>  		if (!darg.zc) {

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001-tls-follow-on-v2-0-2dd1947bb642%40kernel.org

Re: [PATCH net-next v2 2/8] tls: check for a pending signal after an empty record

[Chuck Lever]

On Mon, Oct 05, 2026, netdev-bot+sashiko@kernel.org wrote:
> > +		if (nodata && signal_pending(current)) {
> > +			long timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> > +
> > +			err = sock_intr_errno(timeo);
> > +			goto recv_end;
> > +		}
>
> [Severity: Medium]
> Can this errno be lost when the empty record that set nodata was decrypted
> asynchronously?

Yes, at this commit, with TLS 1.2 and an async AEAD. The empty
record goes onto rx_list through the async arm, and recv_end then
replaces err with the zero that process_rx_list() returns for
async_copy_bytes == 0.

The same assignment already discards the -EAGAIN from
tls_rx_rec_wait() in that configuration, so a nonblocking reader
gets the zero return before this patch too. But the patch
description promises -EINTR, and at this commit the reader does
not get it.

5/8 stops decrypting empty records asynchronously. In v3 that
change will move into this patch.


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

[PATCH net-next v2 3/8] tls: consume empty data records in tls_sw_splice_read()

[Chuck Lever]

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_recvmsg()
does. Test for a pending signal before each retry, so a peer that
streams empty records cannot make the splicing task unkillable.

When an empty record precedes a close_notify, splice(2) now returns
-EINVAL from the control-record test instead of the false EOF it
returned before. 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>
---
 net/tls/tls_sw.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index ee50b9028264..79a807e51bc7 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2018,6 +2018,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;
@@ -2031,13 +2032,14 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	if (err)
 		goto splice_read_end;
 
+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;
 
@@ -2049,6 +2051,9 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 
 		tls_rx_rec_done(ctx);
 		skb = darg.skb;
+
+		/* The retry's wait runs with the socket lock still held. */
+		released = false;
 	}
 
 	rxm = strp_msg(skb);
@@ -2060,6 +2065,19 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 		goto splice_requeue;
 	}
 
+	/* Splicing zero bytes reads as EOF to the caller. */
+	if (rxm->full_len == 0) {
+		consume_skb(skb);
+		if (signal_pending(current)) {
+			long timeo;
+
+			timeo = sock_rcvtimeo(sk, flags & SPLICE_F_NONBLOCK);
+			err = sock_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)

-- 
2.55.0

Re: [PATCH net-next v2 3/8] tls: consume empty data records in tls_sw_splice_read()

[netdev-bot+sashiko]

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

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

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] This commit introduces a new blocking point in
  tls_sw_splice_read() (net/tls/tls_sw.c); a later patch in the series…
- [Medium] The new `goto retry` loop in tls_sw_splice_read()
  (net/tls/tls_sw.c:2069-2079 at this commit, 2088-2095 at the end of…

--

tls: consume empty data records in tls_sw_splice_read()

This patch makes tls_sw_splice_read() consume zero-length
application_data records and fetch the next record, so that callers no
longer see a false EOF. A signal_pending() check before each retry is
meant to keep the splicing task killable.

Closes: https://lore.kernel.org/netdev/akaoXcfamBp8_mYe@krikkit/

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index ee50b9028264c..79a807e51bc7d 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c

[ ... ]

> @@ -2031,13 +2032,14 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
>  	if (err)
>  		goto splice_read_end;
>  
> +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);

[Severity: Medium]
With the new goto retry below, can this wait now block on a socket that
has O_NONBLOCK set?

Here the only input to the nonblocking decision is
flags & SPLICE_F_NONBLOCK. On the socket-to-pipe path, do_splice() sets
that flag only from the output pipe:

fs/splice.c:do_splice() {
    ...
		if (out->f_flags & O_NONBLOCK)
			flags |= SPLICE_F_NONBLOCK;

		ret = splice_file_to_pipe(in, opipe, &offset, len, flags);
    ...
}

tcp_splice_read(), by contrast, honours the socket's own flag:

	timeo = sock_rcvtimeo(sk, sock->file->f_flags & O_NONBLOCK);

Before this patch, an empty record made splice return 0 right away. Now
the empty record is consumed and tls_rx_rec_wait() is called again with
a blocking timeout. It then sleeps until the peer sends another record,
and SO_RCVTIMEO has no limit by default.

Take an event loop with an O_NONBLOCK kTLS socket and a blocking pipe.
Could it hang in splice() after getting EPOLLIN for a single empty
record?

The later patch in this series, "tls: honor O_NONBLOCK in
tls_sw_splice_read()", appears to fix this. It computes nonblock from
both SPLICE_F_NONBLOCK and sock->file->f_flags. Could that patch be
ordered before this one, so the hang does not exist in the bisect
window? Or could the commit message at least mention it?

>  		if (err <= 0)
>  			goto splice_read_end;
>  

[ ... ]

> @@ -2060,6 +2065,19 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
>  		goto splice_requeue;
>  	}
>  
> +	/* Splicing zero bytes reads as EOF to the caller. */
> +	if (rxm->full_len == 0) {
> +		consume_skb(skb);
> +		if (signal_pending(current)) {
> +			long timeo;
> +
> +			timeo = sock_rcvtimeo(sk, flags & SPLICE_F_NONBLOCK);
> +			err = sock_intr_errno(timeo);
> +			goto splice_read_end;
> +		}
> +		goto retry;

[Severity: Medium]
How many consecutive empty records can this loop consume while it holds
the socket lock?

After each record, tls_rx_rec_done()->tls_strp_check_rcv() parses the
next one. On retry, tls_rx_rec_wait() sees tls_strp_msg_ready() return
true and skips its wait loop entirely.

When the receive queue runs dry, sk_flush_backlog() refills it, and the
loop breaks out before reaching sk_wait_event():

net/tls/tls_sw.c:tls_rx_rec_wait() {
	while (!tls_strp_msg_ready(ctx)) {
		...
		if (sk_flush_backlog(sk))
			released = true;
		if (!skb_queue_empty(&sk->sk_receive_queue)) {
			...
			tls_strp_check_rcv(&ctx->strp, false);
			if (tls_strp_msg_ready(ctx))
				break;
		}
		...
		ret = sk_wait_event(sk, &timeo,
				    tls_strp_msg_ready(ctx), &wait);
		...
}

sk_wait_event() is the only place where socket ownership is released.
__release_sock() drops only the slock spinlock, and it calls
cond_resched() only once every 16 backlog skbs.

The only exit is signal_pending(current), and that helps only the
splicing task.

Suppose a peer streams minimal empty application_data records (under 30
bytes each) at least as fast as they are decrypted. Could this:

  - leave other tasks that call lock_sock() on this socket (sendmsg,
    setsockopt, shutdown, close) stuck in uninterruptible sleep?

  - make the CPU busy-loop through back-to-back AEAD decrypts on
    !PREEMPT kernels?

  - pin a nonblocking event loop inside a single splice() call? The
    O_NONBLOCK fix later in the series does not seem to help, because
    nonblock only matters when no record is ready.

Before this patch, each splice(2) call handled at most one record and
then released the lock.

In the same series, "tls: bound consecutive no-data records in
tls_sw_read_sock()" caps this same pattern with TLS_RX_NODATA_LIMIT.
Should the splice path get a similar cap? At the end of the series this
loop still appears to have no limit.

> +	}
> +
>  	chunk = min_t(unsigned int, rxm->full_len, len);
>  	copied = skb_splice_bits(skb, sk, rxm->offset, pipe, chunk, flags);
>  	if (copied < 0)

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001-tls-follow-on-v2-0-2dd1947bb642%40kernel.org

Re: [PATCH net-next v2 3/8] tls: consume empty data records in tls_sw_splice_read()

[Chuck Lever]

On Mon, Oct 05, 2026, netdev-bot+sashiko@kernel.org wrote:
> >  		err = tls_rx_rec_wait(sk, flags & SPLICE_F_NONBLOCK,
> > -				      true, false);
> > +				      released, false);
>
> [Severity: Medium]
> With the new goto retry below, can this wait now block on a socket that
> has O_NONBLOCK set?

Yes, between this patch and 4/8. Before this patch, a caller that
polled and then spliced found a record ready and never reached the
sleep. With the retry, an empty record followed by nothing sleeps
in tls_rx_rec_wait() unless SPLICE_F_NONBLOCK is set.

In v3 the O_NONBLOCK patch will be ordered ahead of this one.


> > +		if (signal_pending(current)) {
> > +			long timeo;
> > +
> > +			timeo = sock_rcvtimeo(sk, flags & SPLICE_F_NONBLOCK);
> > +			err = sock_intr_errno(timeo);
> > +			goto splice_read_end;
> > +		}
> > +		goto retry;
>
> [Severity: Medium]
> How many consecutive empty records can this loop consume while it holds
> the socket lock?

As many as the peer sends back to back. I don't plan to cap it.

The limit in 1/8 works because a read_sock consumer takes a zero
return with nothing copied as "no progress". A splice read has no
such return. Zero means EOF, which is the bug this patch fixes, and
a blocking caller cannot be handed -EAGAIN. The retry is what
tls_sw_recvmsg() already does with an empty record.

The loop runs in the caller's own syscall and the signal test
keeps the task killable. It ends when the peer stops sealing
records faster than the reader opens them. Other tasks that
call lock_sock() on the same socket do wait for that.


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

[PATCH net-next v2 4/8] tls: honor O_NONBLOCK in tls_sw_splice_read()

[Chuck Lever]

tls_sw_splice_read() takes its blocking behavior from
SPLICE_F_NONBLOCK alone. When splicing from a socket to a pipe,
do_splice() sets that flag from the pipe's O_NONBLOCK, not the
socket's. On a nonblocking socket, a splice(2) call without
SPLICE_F_NONBLOCK therefore sleeps in tls_rx_rec_wait() until a
record arrives. tcp_splice_read() instead reads sock->file->f_flags
and returns -EAGAIN.

Poll makes the sleep reachable. tls_sw_sock_is_readable() reports
the socket readable while any record sits on rx_list, including a
zero-length record that delivers no bytes to the pipe. The
strparser announces a record before decryption, when its plaintext
length is unknown, so the readiness test cannot screen such a
record out. The splice consumes it and waits for the next, and an
event loop that polls and then splices stalls every connection it
multiplexes.

Treat the socket's O_NONBLOCK as nonblocking too. A caller that
sets O_NONBLOCK and splices without SPLICE_F_NONBLOCK, taking that
flag to govern only the pipe, now gets -EAGAIN where it blocked
before. 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 now 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 | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 79a807e51bc7..6ca1e9f4e504 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2020,10 +2020,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;
 
@@ -2038,8 +2042,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;
 
@@ -2069,10 +2072,7 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	if (rxm->full_len == 0) {
 		consume_skb(skb);
 		if (signal_pending(current)) {
-			long timeo;
-
-			timeo = sock_rcvtimeo(sk, flags & SPLICE_F_NONBLOCK);
-			err = sock_intr_errno(timeo);
+			err = sock_intr_errno(sock_rcvtimeo(sk, nonblock));
 			goto splice_read_end;
 		}
 		goto retry;

-- 
2.55.0

Re: [PATCH net-next v2 4/8] tls: honor O_NONBLOCK in tls_sw_splice_read()

[netdev-bot+sashiko]

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

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

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] The commit message says the poll-then-splice stall goes back
  to c46234ebb4d1.

--

tls: honor O_NONBLOCK in tls_sw_splice_read()

This patch makes tls_sw_splice_read() treat the socket's O_NONBLOCK as
nonblocking as well as SPLICE_F_NONBLOCK. The combined flag goes to
tls_rx_reader_lock(), tls_rx_rec_wait() and sock_rcvtimeo(). A
poll-then-splice loop on a nonblocking TLS socket then gets -EAGAIN
instead of sleeping.

> Poll makes the sleep reachable. tls_sw_sock_is_readable() reports
> the socket readable while any record sits on rx_list, including a
> zero-length record that delivers no bytes to the pipe. The
> strparser announces a record before decryption, when its plaintext
> length is unknown, so the readiness test cannot screen such a
> record out. The splice consumes it and waits for the next, and an
> event loop that polls and then splices stalls every connection it
> multiplexes.

[ ... ]

> Fixes: c46234ebb4d1 ("tls: RX path for ktls")

[Severity: Medium]
Is c46234ebb4d1 the right commit to blame for this stall? It looks like
the previous patch in this series added the poll-then-splice sleep:
"tls: consume empty data records in tls_sw_splice_read()".

Before that patch, tls_sw_splice_read() had no retry loop. Also,
tls_sk_poll() only reports EPOLLIN when a record is parsed or rx_list is
non-empty:

net/tls/tls_main.c:tls_sk_poll() {
    ...
	if ((skb_queue_empty_lockless(&ctx->rx_list) &&
	     !tls_strp_msg_ready(ctx)) ||
	    READ_ONCE(ctx->key_update_pending))
		mask &= ~(EPOLLIN | EPOLLRDNORM);
    ...
}

So a single reader that polled and then spliced would either dequeue
from rx_list or reach tls_rx_rec_wait() with a record already parsed.
Either way it never slept. A zero-length record gave a false EOF, not a
hang.

The previous patch added this retry:

	if (rxm->full_len == 0) {
		consume_skb(skb);
		...
		goto retry;
	}

On retry, that patch calls tls_rx_rec_wait() with
flags & SPLICE_F_NONBLOCK. That value is false for an O_NONBLOCK socket
spliced into a blocking pipe. The wait then uses sk_rcvtimeo and sleeps
until the next record arrives.

This creates a bisect window between the two commits. In that window, a
peer that sends one empty application_data record and then goes quiet
can stall a poll/splice event loop.

Both commits also carry the same Fixes: tag. The earlier one applies on
its own, but this one rewrites the earlier one's signal_pending() branch
and can't apply without it. A stable backport of only the earlier
commit would turn the false EOF into a hang that a remote peer can
trigger.

Could this patch go before "tls: consume empty data records in
tls_sw_splice_read()" in the series? If not, could this commit message
say that the stall depends on that commit and that the two must be
backported together?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001-tls-follow-on-v2-0-2dd1947bb642%40kernel.org

Re: [PATCH net-next v2 4/8] tls: honor O_NONBLOCK in tls_sw_splice_read()

[Chuck Lever]

On Mon, Oct 05, 2026, netdev-bot+sashiko@kernel.org wrote:
> > Fixes: c46234ebb4d1 ("tls: RX path for ktls")
>
> [Severity: Medium]
> Is c46234ebb4d1 the right commit to blame for this stall? It looks like
> the previous patch in this series added the poll-then-splice sleep:
> "tls: consume empty data records in tls_sw_splice_read()".

The tag is right for what the patch changes. tls_sw_splice_read()
has ignored the socket's O_NONBLOCK since c46234ebb4d1, and a
splice on a nonblocking socket with no record queued sleeps without
any help from 3/8.

The second paragraph of the patch description is wrong for a tree
without 3/8, though. There, a caller that polls and then splices an
empty record gets the false EOF, not a sleep. The stall that
paragraph describes needs the retry 3/8 adds.


> Could this patch go before "tls: consume empty data records in
> tls_sw_splice_read()" in the series?

Yes. v3 will move this patch first and reword that paragraph.


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

[PATCH net-next v2 5/8] 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() keeps the second loop term
true while records keep arriving. The peek arm and the async arm
also queue each record on rx_list, which then grows without bound.

Consume an empty data record as soon as the receive loop has it,
before the paths diverge on darg.zc. Freeing the skb requires its
decryption to have completed, so an empty record is no longer
decrypted asynchronously. The new branch sets MSG_EOR itself,
because the record no longer reaches the assignment at the bottom
of the loop.

Fixes: 692d7b5d1f91 ("tls: Fix recvmsg() to be able to peek across multiple records")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 6ca1e9f4e504..4fecac8a0b0d 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1885,9 +1885,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;
 
@@ -1924,6 +1927,19 @@ int tls_sw_recvmsg(struct sock *sk,
 		nodata = !chunk;
 		tls_rx_rec_done(ctx);
 
+		/* Keep an empty record off rx_list. On the zero-copy path
+		 * the strparser owns darg.skb, and tls_rx_rec_done() has
+		 * released it.
+		 */
+		if (!chunk && control == TLS_RECORD_TYPE_DATA) {
+			if (!darg.zc)
+				consume_skb(darg.skb);
+
+			/* An empty record still marks a boundary. */
+			msg->msg_flags |= MSG_EOR;
+			continue;
+		}
+
 		if (!darg.zc) {
 			bool partially_consumed = chunk > len;
 			struct sk_buff *skb = darg.skb;

-- 
2.55.0

Re: [PATCH net-next v2 5/8] tls: consume empty data records in tls_sw_recvmsg()

[netdev-bot+sashiko]

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

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

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).

- [High] The patch stops empty records from piling up on rx_list, but the
  receive loop in tls_sw_recvmsg() (net/tls/tls_sw.c) still never…

--

tls: consume empty data records in tls_sw_recvmsg()

This patch makes tls_sw_recvmsg() free a zero-length application_data
record as soon as it is decrypted. Before, the peek and async arms queued
it on rx_list. Empty records are no longer decrypted asynchronously, and
the new branch sets MSG_EOR itself.

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 6ca1e9f4e5040..4fecac8a0b0da 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c

[ ... ]

> @@ -1924,6 +1927,19 @@ int tls_sw_recvmsg(struct sock *sk,
>  		nodata = !chunk;
>  		tls_rx_rec_done(ctx);
>  
> +		/* Keep an empty record off rx_list. On the zero-copy path
> +		 * the strparser owns darg.skb, and tls_rx_rec_done() has
> +		 * released it.
> +		 */
> +		if (!chunk && control == TLS_RECORD_TYPE_DATA) {
> +			if (!darg.zc)
> +				consume_skb(darg.skb);
> +
> +			/* An empty record still marks a boundary. */
> +			msg->msg_flags |= MSG_EOR;
> +			continue;
> +		}

[Severity: High]
This keeps empty records off rx_list. Does the receive loop still fail to
make progress on them, though? The commit message describes this part of
the problem as well:

    len never reaches zero, and tls_strp_msg_ready() keeps the second
    loop term true while records keep arriving.

The new branch does not change len, decrypted or copied. It also places no
limit on how many empty records can arrive in a row. Once
decrypted + copied >= target, only the strparser keeps the loop going:

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

Could a peer that keeps sending zero-length application_data records hold
the reader in this loop forever? Three things keep the loop supplied:

  tls_rx_rec_done()->tls_strp_check_rcv() parses the next record right
  away.

  tls_read_flush_backlog() flushes the backlog on every pass whenever
  tcp_inq(sk) <= max_rec, because done never moves:

      if (done - *flushed_at < SZ_128K && tcp_inq(sk) > max_rec)
          return false;

      *flushed_at = done;
      return sk_flush_backlog(sk);

  tls_rx_rec_wait() returns 1 at once when a record is ready, so the
  loop never reaches its -EAGAIN exit for nonblocking callers.

So during such a flood, a MSG_DONTWAIT caller would not get -EAGAIN. Bytes
already copied would not be returned until the flood stops.

SO_RCVTIMEO readers have a related problem. tls_rx_rec_wait() starts a new
timeout on every call:

    timeo = sock_rcvtimeo(sk, nonblock);

    while (!tls_strp_msg_ready(ctx)) {

If a reader has copied nothing yet, can a peer that sends one empty record
per timeout period stop it from ever timing out?

The only way out seems to be the check added earlier in this series:

    if (nodata && signal_pending(current)) {

In-kernel MSG_DONTWAIT callers never receive a signal, so they cannot use
this exit. One example is svc_tcp_sock_recvmsg() in net/sunrpc/svcsock.c:

    ret = sock_recvmsg(sock, msg, MSG_DONTWAIT);

This same series already limits tls_sw_read_sock() with TLS_RX_NODATA_LIMIT
in "tls: bound consecutive no-data records in tls_sw_read_sock()". The
later patches in the series only change selftests, so tls_sw_recvmsg()
still has no limit at the end of the series.

The zero-copy arm could already spin like this before this patch. Since
the commit message names this loop behavior as the defect, should the
empty-record branch break out once decrypted + copied >= target? The other
option would be a limit on consecutive empty records, as tls_sw_read_sock()
has.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001-tls-follow-on-v2-0-2dd1947bb642%40kernel.org

Re: [PATCH net-next v2 5/8] tls: consume empty data records in tls_sw_recvmsg()

[Chuck Lever]

On Mon, Oct 05, 2026, netdev-bot+sashiko@kernel.org wrote:
> > +		if (!chunk && control == TLS_RECORD_TYPE_DATA) {
> > +			if (!darg.zc)
> > +				consume_skb(darg.skb);
> > +
> > +			/* An empty record still marks a boundary. */
> > +			msg->msg_flags |= MSG_EOR;
> > +			continue;
> > +		}
>
> [Severity: High]
> This keeps empty records off rx_list. Does the receive loop still fail to
> make progress on them, though?

It does, and this patch does not change that. Before it, every arm
of the loop reached "decrypted += chunk" and "len -= chunk" with
chunk == 0 and went around again. The patch stops the peek and
async arms from queueing those records on rx_list and leaves the
loop's bounds alone.

The patch description cites the loop to explain why rx_list grows,
and I can see it reads as a claim that the patch bounds the loop.
I'll reword it in v3.


> In-kernel MSG_DONTWAIT callers never receive a signal, so they cannot use
> this exit. One example is svc_tcp_sock_recvmsg() in net/sunrpc/svcsock.c:

True, and also true before this series.


> should the empty-record branch break out once decrypted + copied >= target?

Returning bytes already copied instead of waiting out a run of
empty records is reasonable, and I'll look at it for v3. With
nothing copied, a blocking reader has no return value that is not
EOF or an error, so that case keeps looping until a record with
data, a signal, or the end of the run.


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

[PATCH net-next v2 6/8] 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 splice nor MSG_PEEK is
tested against a record that decrypts to no payload.

Add zero_len_splice. A zero return from splice() means EOF, so an
empty data record must not end a live connection. Its variants
expect the payload's length when one sits behind a run of empty
records, EAGAIN when none does, and EINVAL when a control record
does. An unfixed kernel returns 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. It reproduces no
failure, because the unbounded rx_list growth 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 | 224 +++++++++++++++++++++++++++++++++++---
 1 file changed, 207 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index 9d3cd4fff062..419c6cc0cc5e 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2549,6 +2549,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 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)
 {
 	int fd, cfd;
@@ -2611,19 +2647,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)
@@ -2639,10 +2663,7 @@ TEST_F(zero_len, test)
 	int rec_off;
 	int i;
 
-	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;
@@ -2668,6 +2689,175 @@ 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
+ * 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);
+		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);
+}
+
+/* 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);
+		EXPECT_EQ(errno, -variant->splice_ret);
+	} else {
+		/* ASSERT so a zero return stops the test here; the read
+		 * below would 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);
+
+		/* The empty records were consumed, not left queued */
+		ret = recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT);
+		EXPECT_EQ(ret, -1);
+		EXPECT_EQ(errno, EAGAIN);
+	}
+
+	close(p[0]);
+	close(p[1]);
+}
+
 FIXTURE(tls_err)
 {
 	int fd, cfd;

-- 
2.55.0

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] zero_len_peek in tools/testing/selftests/net/tls.c passes on a
  kernel that lacks the fix from commit "tls: consume empty data records…
- [Low] In TEST_F(zero_len_splice, test), the last check is commented
  "The empty records were consumed, not left queued".

--

selftests: tls: add peek and splice coverage for zero-length records

This adds two fixtures to tools/testing/selftests/net/tls.c,
zero_len_peek and zero_len_splice. They inject zero-length
application_data records over a socket that has only a TLS_RX key, then
exercise MSG_PEEK and splice() against them. The socket setup and record
sending code is moved into shared helpers used by all three zero_len
fixtures.

> diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
> index 9d3cd4fff062b..419c6cc0cc5e1 100644
> --- a/tools/testing/selftests/net/tls.c
> +++ b/tools/testing/selftests/net/tls.c

[ ... ]

> +/* 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);
> +		EXPECT_EQ(errno, -variant->peek_ret);
> +		return;
> +	}

[Severity: Low]
Can this test tell a fixed kernel from an unfixed one? It seems to pass on
a kernel without the "tls: consume empty data records in tls_sw_recvmsg()"
fix, which adds this branch in tls_sw_recvmsg():

net/tls/tls_sw.c:tls_sw_recvmsg() {
    ...
		if (!chunk && control == TLS_RECORD_TYPE_DATA) {
			if (!darg.zc)
				consume_skb(darg.skb);
    ...
}

Without that fix, the peek path queues each empty record on ctx->rx_list
via put_on_rx_list. In the 0data_0data_0data variant this leaves three
empty skbs on rx_list, and then tls_rx_rec_wait() returns -EAGAIN from:

	if (!timeo)
		return -EAGAIN;

That is the same -1/EAGAIN this branch expects. The comment above the test
contrasts EAGAIN with "the zero return that means EOF". Did the peek path
ever return 0 in this case?

The 0data_0data_data variant behaves the same way on both kernels. The peek
returns the 11-byte payload. On the next non-peek recv(), process_rx_list()
computes chunk as 0 for each empty skb:

	int chunk = min_t(unsigned int, rxm->full_len - skip, len);

so each one is unlinked and freed without any visible effect:

	if (!is_peek) {
		__skb_unlink(skb, &ctx->rx_list);
		consume_skb(skb);
	}

The commit message says:

    It reproduces no failure, because the unbounded rx_list growth needs a
    sustained flood that three fixed-sequence records cannot supply.

The retained records do appear to be visible with three records, though.
tls_sk_poll() in net/tls/tls_main.c reports readable while rx_list is
non-empty:

	if ((skb_queue_empty_lockless(&ctx->rx_list) &&
	     !tls_strp_msg_ready(ctx)) ||

Would a poll(cfd, POLLIN, 0) after the peek in the 0data_0data_0data
variant catch this? It should report readable on an unfixed kernel and not
readable on a fixed one.

[ ... ]

> +	} else {
> +		/* ASSERT so a zero return stops the test here; the read
> +		 * below would 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);
> +
> +		/* The empty records were consumed, not left queued */
> +		ret = recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT);
> +		EXPECT_EQ(ret, -1);
> +		EXPECT_EQ(errno, EAGAIN);

[Severity: Low]
Does this check prove what the comment says? It looks like recv() returns
-1/EAGAIN whether or not the empty records are still on ctx->rx_list.

If zero-length data skbs were still queued, recv() would go through
process_rx_list(). There, chunk is 0, so it copies nothing, frees each
empty skb and returns 0 without setting *more.

Back in tls_sw_recvmsg(), copied is 0, rx_more is false and control is
TLS_RECORD_TYPE_DATA, so it does not take this exit:

	if (len <= copied || rx_more ||
	    (control && control != TLS_RECORD_TYPE_DATA))
		goto end;

It then enters the receive loop, where tls_rx_rec_wait() returns -EAGAIN
for the non-blocking call.

The payload is the last record in every variant that reaches this branch,
so this assertion passes in both states. Should the comment be reworded, or
the check replaced with one that can see records left on rx_list, such as
the poll() check suggested above?

> +	}
> +
> +	close(p[0]);
> +	close(p[1]);
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001-tls-follow-on-v2-0-2dd1947bb642%40kernel.org

[PATCH net-next v2 7/8] 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 returns
the ciphertext verbatim. All eight variants 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, 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>
---
 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 419c6cc0cc5e..e6876b8caac8 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2663,6 +2663,9 @@ TEST_F(zero_len, test)
 	int rec_off;
 	int i;
 
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
 	zero_len_send_recs(_metadata, self->fd, variant->recs);
 
 	rec = &variant->recs[0];

-- 
2.55.0

Re: [PATCH net-next v2 7/8] selftests: tls: skip the zero_len tests when TLS is unavailable

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Backport problem.

--

selftests: tls: skip the zero_len tests when TLS is unavailable

This patch makes TEST_F(zero_len, test) skip when the fixture has recorded
notls. Without the check, all eight variants fail on a kernel built without
CONFIG_TLS, because the raw records go over a plain TCP socket.

> diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
> index 419c6cc0cc5e1..e6876b8caac89 100644
> --- a/tools/testing/selftests/net/tls.c
> +++ b/tools/testing/selftests/net/tls.c
> @@ -2663,6 +2663,9 @@ TEST_F(zero_len, test)
>  	int rec_off;
>  	int i;
>  
> +	if (self->notls)
> +		SKIP(return, "no TLS support");
> +
>  	zero_len_send_recs(_metadata, self->fd, variant->recs);
                ^^^^

[Severity: Low]
This isn't a bug, but will this apply cleanly when it is backported on its
own?

The Fixes: tag points at a61a3e961baf ("selftests: tls: add tests for
zero-length records"), so stable tooling will probably try to pick this up.
However, the trailing context line above, the zero_len_send_recs() call,
comes from the previous patch in this series, "selftests: tls: add peek and
splice coverage for zero-length records". That patch has no Fixes: tag.

A tree that has a61a3e961baf but not that refactor still has the open-coded
send loop at this spot in TEST_F(zero_len, test):

	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);

So the hunk context won't match there, and someone would have to rebase the
fix by hand. Until then, stable kernels without TLS would keep reporting the
false zero_len failures.

The missing notls check has been there since a61a3e961baf. Could this patch
go first in the series, written against the original send loop, so it can
be backported without the refactor?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001-tls-follow-on-v2-0-2dd1947bb642%40kernel.org

[PATCH net-next v2 8/8] 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. Both forms must reach the same outcome, so the existing
per-variant expectations carry over. A kernel that ignores the
socket flag sleeps in the EAGAIN variant until the harness
timeout rather than returning.

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

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index e6876b8caac8..6207a90c5f07 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2820,23 +2820,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,
+		   FIXTURE_DATA(zero_len_splice) *self,
+		   const 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);
 		EXPECT_EQ(errno, -variant->splice_ret);
 	} else {
@@ -2844,7 +2845,7 @@ TEST_F(zero_len_splice, test)
 		 * below would 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)
@@ -2861,6 +2862,33 @@ 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 O_NONBLOCK governs the record wait, as on a plain TCP
+ * socket, so a splice without SPLICE_F_NONBLOCK behaves like one that
+ * sets it. Deriving the wait from SPLICE_F_NONBLOCK alone 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.55.0

Re: [PATCH net-next v2 0/8] net/tls: Receive-path fixes for zero-length data records

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next v2 0/8] net/tls: Receive-path fixes for zero-length data records

[Chuck Lever]

>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.

Sabrina reported the splice(2) false EOF fixed in patch 3 while
reviewing commit 3be28e2c9cd0 ("net/tls: Consume empty data records
in tls_sw_read_sock()"):

https://lore.kernel.org/netdev/akaoXcfamBp8_mYe@krikkit/

Sashiko's review of that patch flagged the same defect, and also
the rx_list growth under MSG_PEEK fixed in patch 5. Its review of
the read_sock series I posted on July 20 flagged the missing signal
check in tls_sw_recvmsg() fixed in patch 2.

The defects fixed in patches 1 and 4 were found by an LLM audit and
review of the TLS read code paths.


>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.

None of these was hit in production, so there is no stack trace or
error message to include.

The selftests in this series reproduce these two: On an unfixed
kernel, splice(2) returns zero when it reaches a zero-length record
(patches 3 and 6); and a splice from a nonblocking socket without
SPLICE_F_NONBLOCK sleeps until the harness timeout (patches 4 and 8).

The fixes in patches 1, 2, and 5 have not been reproduced because
these need a peer that streams zero-length records without pause.

I'll update the commit messages I'm carrying locally so subsequent
postings will carry this information.


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

Re: [PATCH net-next v2 0/8] net/tls: Receive-path fixes for zero-length data records

[Qingfang Deng]

Hi,

On 10/2/2026 6:41 AM, Chuck Lever wrote:
> 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 them. splice(2) reports an empty
> record as EOF, and the caller tears down a connection that is still
> live. recvmsg(2) makes no progress on one, so a peer that streams
> them holds the caller in the kernel past SIGKILL. With MSG_PEEK or
> async decryption, each record is also queued on rx_list, which
> grows without bound.
> 
> This series supersedes "[PATCH net] tls: skip empty data records in
> tls_sw_splice_read()", which fixes the splice case alone:
> https://lore.kernel.org/netdev/20260930052636.166007-1-qingfang.deng@linux.dev/
tls_sw_splice_read() currently returns at most 16KiB (one record) per 
call. You may add a loop like the one in tls_sw_recvmsg().

Best regards,
Qingfang