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

[PATCH net-next v3 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 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. An in-kernel
caller of sock_recvmsg() cannot take a signal, so a flood holds
that caller as it does today. Only tls_sw_read_sock() gets 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. A
recvmsg(2) that has copied data now returns at an empty record
rather than reading the records that follow.

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, 937 tests with
no skips.

---
Changes in v3:
- Run the consumer's sk_data_ready() from a work item (sashiko).
- Move the sync decrypt of empty records to the signal patch (sashiko).
- Put the O_NONBLOCK patch before the splice retry patch (sashiko).
- Reword the O_NONBLOCK patch description to match (sashiko).
- Say that the recvmsg patch does not bound the receive loop (sashiko).
- Add a patch that returns copied data ahead of empty records (sashiko).
- Put the zero_len skip patch before the coverage patch (sashiko).
- Use poll() to check for records left on rx_list (sashiko).
- Link to v2: https://patch.msgid.link/20261001-tls-follow-on-v2-0-2dd1947bb642@kernel.org

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 (9):
      tls: Bound consecutive no-data records in tls_sw_read_sock()
      tls: Check for a pending signal after an empty record
      tls: Honor O_NONBLOCK in tls_sw_splice_read()
      tls: Consume empty data records in tls_sw_splice_read()
      tls: Consume empty data records in tls_sw_recvmsg()
      tls: Return copied data ahead of a run of empty records
      selftests: tls: Skip the zero_len tests when TLS is unavailable
      selftests: tls: Add peek and splice coverage for zero-length records
      selftests: tls: Cover splice on a nonblocking socket

 include/net/tls.h                 |   1 +
 net/tls/tls_sw.c                  |  98 +++++++++++++--
 tools/testing/selftests/net/tls.c | 256 +++++++++++++++++++++++++++++++++++---
 3 files changed, 328 insertions(+), 27 deletions(-)
---
base-commit: a5e7d8e446af9803e37a3b6a4d416fb41178348f
change-id: 20260726-tls-follow-on-486f1ba8bbb0

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

[PATCH net-next v3 1/9] tls: Bound consecutive no-data records in tls_sw_read_sock()

[Chuck Lever]

A zero-length application_data record does not deliver a 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.

An LLM audit of the TLS read paths found this defect. The defect has
not been reproduced.

Stop after TLS_RX_NODATA_LIMIT consecutive empty data records. 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 do not raise another
sk_data_ready(), so the consumer needs one. The consumer's callback
can call tls_sw_read_sock() itself, as strp_data_ready() does when
the socket is not owned by user. Run the callback from a work item,
after the reader has returned.

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

diff --git a/include/net/tls.h b/include/net/tls.h
index e57bef58851e..42fcb9792bcc 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -145,6 +145,7 @@ struct tls_sw_context_rx {
 	atomic_t decrypt_pending;
 	struct sk_buff_head async_hold;
 	struct wait_queue_head wq;
+	struct work_struct data_ready_work;
 };
 
 struct tls_record_info {
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 312e51270f29..275a6047da9e 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2070,6 +2070,20 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	goto splice_read_end;
 }
 
+#define TLS_RX_NODATA_LIMIT	16
+
+static void tls_rx_data_ready_work(struct work_struct *w)
+{
+	struct tls_sw_context_rx *ctx =
+		container_of(w, struct tls_sw_context_rx, data_ready_work);
+	struct sock *sk = ctx->strp.sk;
+
+	/* A callback that finds the socket unowned reads from it. */
+	lock_sock(sk);
+	READ_ONCE(sk->sk_data_ready)(sk);
+	release_sock(sk);
+}
+
 int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
 		     sk_read_actor_t read_actor)
 {
@@ -2078,6 +2092,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 +2151,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() does not call the
+				 * consumer's sk_data_ready, which can re-enter
+				 * this function.
+				 */
+				schedule_work(&ctx->data_ready_work);
+				break;
+			}
 			continue;
 		}
 
@@ -2154,6 +2177,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;
@@ -2345,6 +2369,7 @@ void tls_sw_strparser_done(struct tls_context *tls_ctx)
 {
 	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
 
+	cancel_work_sync(&ctx->data_ready_work);
 	tls_strp_done(&ctx->strp);
 }
 
@@ -2476,6 +2501,7 @@ static struct tls_sw_context_rx *init_ctx_rx(struct tls_context *ctx)
 	crypto_init_wait(&sw_ctx_rx->async_wait);
 	atomic_set(&sw_ctx_rx->decrypt_pending, 1);
 	init_waitqueue_head(&sw_ctx_rx->wq);
+	INIT_WORK(&sw_ctx_rx->data_ready_work, tls_rx_data_ready_work);
 	skb_queue_head_init(&sw_ctx_rx->rx_list);
 	skb_queue_head_init(&sw_ctx_rx->async_hold);
 

-- 
2.55.0

[PATCH net-next v3 2/9] 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 tls_rx_rec_wait() once per record, and a
zero-length application_data record does not advance either bound
of the receive loop. A peer that streams such records holds the
caller in recvmsg(), unresponsive to SIGKILL, for as long as they
arrive. The hang has not been reproduced.

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

Decrypt an empty record synchronously. The code at recv_end
overwrites err once any record has been decrypted asynchronously.
With only empty records queued, the reader then gets zero, not the
errno.

Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260720-tcp-read-sock-v2-0-29545d034f3c@kernel.org?part=1
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 275a6047da9e..32be5a8f80ed 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)
@@ -1874,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: once a record has gone async, recv_end
+		 * discards the error that ends a run of empty records.
+		 */
 		if (tlm->control == TLS_RECORD_TYPE_DATA)
-			darg.async = ctx->async_capable;
+			darg.async = ctx->async_capable && to_decrypt;
 		else
 			darg.async = false;
 
@@ -1910,6 +1924,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

[PATCH net-next v3 3/9] 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.

Polling first does not avoid the sleep once tls_sw_splice_read()
consumes zero-length records. 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 the record's
plaintext length is unknown, so the readiness test cannot screen
such a record out. A splice that consumes the record and waits for
the next stalls every connection an event loop multiplexes.

An LLM audit of the TLS read paths found this defect. With empty
records consumed, the zero_len_splice selftest, run on a
nonblocking socket, reproduces the sleep.

Treat the socket's O_NONBLOCK as nonblocking too. A caller that
sets O_NONBLOCK on the socket and splices without SPLICE_F_NONBLOCK
now gets -EAGAIN where the splice 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 | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 32be5a8f80ed..6c21897b03ee 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2022,10 +2022,14 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
 	struct tls_msg *tlm;
 	struct sk_buff *skb;
 	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;
 
@@ -2039,8 +2043,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,
-				      true, false);
+		err = tls_rx_rec_wait(sk, nonblock, true, false);
 		if (err <= 0)
 			goto splice_read_end;
 

-- 
2.55.0

[PATCH net-next v3 4/9] 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. The zero_len_splice selftest
reproduces the zero return.

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 a false EOF. A
splice that reaches an alert record already reports -EINVAL.

Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Reported-by: Sabrina Dubroca <sd@queasysnail.net>
Closes: https://lore.kernel.org/netdev/akaoXcfamBp8_mYe@krikkit/
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 6c21897b03ee..ff82e2f4e9e5 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2021,6 +2021,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;
 	bool nonblock;
 	int chunk;
@@ -2038,12 +2039,13 @@ 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, nonblock, true, false);
+		err = tls_rx_rec_wait(sk, nonblock, released, false);
 		if (err <= 0)
 			goto splice_read_end;
 
@@ -2055,6 +2057,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);
@@ -2066,6 +2071,16 @@ 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)) {
+			err = sock_intr_errno(sock_rcvtimeo(sk, nonblock));
+			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

[PATCH net-next v3 5/9] 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 and RFC 8446 Section 5.1). A record that decrypts to
full_len == 0 advances neither len nor decrypted, so
tls_sw_recvmsg() keeps reading empty records for as long as they
arrive. The peek arm and the async arm queue each empty record on
rx_list, and a peer that streams empty records grows rx_list
without bound. The growth has not been reproduced.

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, and an empty record is already
decrypted synchronously. The new branch sets MSG_EOR itself,
because the record no longer reaches the assignment at the bottom
of the loop.

After this change, tls_sw_recvmsg() still reads empty records for
as long as they arrive, until a signal is pending.

Fixes: 692d7b5d1f91 ("tls: Fix recvmsg() to be able to peek across multiple records")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260630191551.875664-1-cel@kernel.org?part=1
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index ff82e2f4e9e5..b7f3edf9828e 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1886,8 +1886,8 @@ int tls_sw_recvmsg(struct sock *sk,
 			darg.zc = true;
 
 		/* Do not use async mode if record is non-data, or if it
-		 * is empty: once a record has gone async, recv_end
-		 * discards the error that ends a run of empty records.
+		 * is empty: the receive loop frees an empty record's skb,
+		 * which an async decrypt would still be using.
 		 */
 		if (tlm->control == TLS_RECORD_TYPE_DATA)
 			darg.async = ctx->async_capable && to_decrypt;
@@ -1927,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

[PATCH net-next v3 6/9] tls: Return copied data ahead of a run of empty records

[Chuck Lever]

Once tls_sw_recvmsg() has copied enough data to meet the receive
low-water mark, the receive loop continues only while another
record is ready. A zero-length application_data record is consumed
without advancing len, so a peer that follows data with a stream of
empty records keeps the reader in the loop. The reader does not get
the data already copied until the stream stops or a signal is
pending. The delay has not been reproduced.

Leave the receive loop at an empty record once the low-water mark
is met. A reader that has copied nothing still reads empty records
for as long as they arrive.

Suggested-by: sashiko-bot <netdev-bot+sashiko@kernel.org>
Link: https://lore.kernel.org/r/179124254665.434549.12245680127262431659@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/tls/tls_sw.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index b7f3edf9828e..fd6f944ffd67 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1937,6 +1937,12 @@ int tls_sw_recvmsg(struct sock *sk,
 
 			/* An empty record still marks a boundary. */
 			msg->msg_flags |= MSG_EOR;
+
+			/* Return data already copied instead of holding it
+			 * through a run of empty records.
+			 */
+			if (decrypted + copied >= target)
+				break;
 			continue;
 		}
 

-- 
2.55.0

[PATCH net-next v3 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 returns
the ciphertext verbatim. All eight variants FAIL on a machine that
lacks TLS, and those failures mask any real regression in the same
run.

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

Fixes: a61a3e961baf ("selftests: tls: add tests for zero-length records")
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 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 9d3cd4fff062..0bc43728262e 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2639,6 +2639,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.55.0

[PATCH net-next v3 8/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 some of those records are
zero-length application_data records. Every variant reads the
records back with a plain recv(), so neither splice nor MSG_PEEK is
tested against an empty record.

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

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

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index 0bc43728262e..5ff77f1c40c6 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)
@@ -2642,10 +2666,7 @@ TEST_F(zero_len, test)
 	if (self->notls)
 		SKIP(return, "no TLS support");
 
-	for (i = 0; i < 4 && variant->recs[i]; i++)
-		EXPECT_EQ(send(self->fd, variant->recs[i]->cipher_data,
-			       variant->recs[i]->cipher_len, 0),
-			  variant->recs[i]->cipher_len);
+	zero_len_send_recs(_metadata, self->fd, variant->recs);
 
 	rec = &variant->recs[0];
 	rec_off = 0;
@@ -2671,6 +2692,177 @@ TEST_F(zero_len, test)
 	}
 };
 
+FIXTURE(zero_len_peek)
+{
+	int fd, cfd;
+	bool notls;
+};
+
+FIXTURE_VARIANT(zero_len_peek)
+{
+	const struct raw_rec *recs[4];
+	ssize_t peek_ret;
+};
+
+FIXTURE_VARIANT_ADD(zero_len_peek, 0data_0data_data)
+{
+	.recs = { &id0_data_l0, &id1_data_l0, &id2_data_l11, },
+	.peek_ret = 11,
+};
+
+FIXTURE_VARIANT_ADD(zero_len_peek, 0data_0data_0data)
+{
+	.recs = { &id0_data_l0, &id1_data_l0, &id2_data_l0, },
+	.peek_ret = -EAGAIN,
+};
+
+FIXTURE_SETUP(zero_len_peek)
+{
+	zero_len_sock_pair(_metadata, &self->fd, &self->cfd, &self->notls);
+}
+
+FIXTURE_TEARDOWN(zero_len_peek)
+{
+	close(self->fd);
+	close(self->cfd);
+}
+
+/* Peeking past a run of empty data records must reach the payload
+ * behind them, and a run with no payload behind it must report EAGAIN
+ * and leave no record queued.
+ */
+TEST_F(zero_len_peek, test)
+{
+	struct pollfd pfd = { .fd = self->cfd, .events = POLLIN };
+	const struct raw_rec *payload;
+	unsigned char buf[128];
+	ssize_t ret;
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	payload = zero_len_send_recs(_metadata, self->fd, variant->recs);
+
+	if (variant->peek_ret < 0) {
+		ret = recv(self->cfd, buf, sizeof(buf),
+			   MSG_DONTWAIT | MSG_PEEK);
+		EXPECT_EQ(ret, -1);
+		EXPECT_EQ(errno, -variant->peek_ret);
+
+		/* A record left on rx_list keeps the socket readable */
+		EXPECT_EQ(poll(&pfd, 1, 0), 0);
+		return;
+	}
+
+	ret = recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT | MSG_PEEK);
+	EXPECT_EQ(ret, variant->peek_ret);
+	if (ret == variant->peek_ret)
+		EXPECT_EQ(memcmp(buf, payload->plain_data,
+				 variant->peek_ret), 0);
+
+	ret = recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT);
+	EXPECT_EQ(ret, variant->peek_ret);
+	if (ret == variant->peek_ret)
+		EXPECT_EQ(memcmp(buf, payload->plain_data,
+				 variant->peek_ret), 0);
+
+	ret = recv(self->cfd, buf, sizeof(buf), MSG_DONTWAIT);
+	EXPECT_EQ(ret, -1);
+	EXPECT_EQ(errno, EAGAIN);
+}
+
+FIXTURE(zero_len_splice)
+{
+	int fd, cfd;
+	bool notls;
+};
+
+FIXTURE_VARIANT(zero_len_splice)
+{
+	const struct raw_rec *recs[4];
+	ssize_t splice_ret;
+};
+
+FIXTURE_VARIANT_ADD(zero_len_splice, 0data_data)
+{
+	.recs = { &id0_data_l0, &id1_data_l11, },
+	.splice_ret = 11,
+};
+
+FIXTURE_VARIANT_ADD(zero_len_splice, 0data_0data_data)
+{
+	.recs = { &id0_data_l0, &id1_data_l0, &id2_data_l11, },
+	.splice_ret = 11,
+};
+
+FIXTURE_VARIANT_ADD(zero_len_splice, 0data_0data_0data)
+{
+	.recs = { &id0_data_l0, &id1_data_l0, &id2_data_l0, },
+	.splice_ret = -EAGAIN,
+};
+
+FIXTURE_VARIANT_ADD(zero_len_splice, 0data_0ctrl)
+{
+	.recs = { &id0_data_l0, &id1_ctrl_l0, },
+	.splice_ret = -EINVAL,
+};
+
+FIXTURE_SETUP(zero_len_splice)
+{
+	zero_len_sock_pair(_metadata, &self->fd, &self->cfd, &self->notls);
+}
+
+FIXTURE_TEARDOWN(zero_len_splice)
+{
+	close(self->fd);
+	close(self->cfd);
+}
+
+/* Splicing must skip a run of empty data records to reach the payload
+ * behind it, since a zero-byte splice reads as EOF. Splice reports
+ * EAGAIN for a run with no payload behind it, and EINVAL for a control
+ * record behind the run.
+ */
+TEST_F(zero_len_splice, test)
+{
+	struct pollfd pfd = { .fd = self->cfd, .events = POLLIN };
+	const struct raw_rec *payload;
+	unsigned char buf[128];
+	ssize_t ret;
+	int p[2];
+
+	if (self->notls)
+		SKIP(return, "no TLS support");
+
+	ASSERT_GE(pipe(p), 0);
+
+	payload = zero_len_send_recs(_metadata, self->fd, variant->recs);
+
+	if (variant->splice_ret < 0) {
+		ret = splice(self->cfd, NULL, p[1], NULL, sizeof(buf),
+			     SPLICE_F_NONBLOCK);
+		EXPECT_EQ(ret, -1);
+		EXPECT_EQ(errno, -variant->splice_ret);
+	} else {
+		/* ASSERT so a zero return stops the test 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 */
+		EXPECT_EQ(poll(&pfd, 1, 0), 0);
+	}
+
+	close(p[0]);
+	close(p[1]);
+}
+
 FIXTURE(tls_err)
 {
 	int fd, cfd;

-- 
2.55.0

[PATCH net-next v3 9/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. Each variant expects the same result with either flag. 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 | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index 5ff77f1c40c6..557dae37b3f1 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -2823,7 +2823,11 @@ FIXTURE_TEARDOWN(zero_len_splice)
  * EAGAIN for a run with no payload behind it, and EINVAL for a control
  * record behind the run.
  */
-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)
 {
 	struct pollfd pfd = { .fd = self->cfd, .events = POLLIN };
 	const struct raw_rec *payload;
@@ -2831,16 +2835,13 @@ TEST_F(zero_len_splice, test)
 	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 {
@@ -2848,7 +2849,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)
@@ -2863,6 +2864,32 @@ 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.
+ */
+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 v3 1/9] 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] In net/tls/tls_sw.c:tls_sw_read_sock(), the new budget exit
  rests on a false premise.
- [Low] When tls_sw_read_sock() hits TLS_RX_NODATA_LIMIT, the only thing
  that restarts the read_sock consumer is a work item queued with…

--

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

This commit limits how many zero-length application_data records in a
row tls_sw_read_sock() will consume in one call. The limit is
TLS_RX_NODATA_LIMIT. When the limit is reached, the loop breaks and
schedules a work item that calls sk->sk_data_ready() under lock_sock(),
so the read_sock consumer can resume.

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 312e51270f293..275a6047da9e3 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -2070,6 +2070,20 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
>  	goto splice_read_end;
>  }
>  
> +#define TLS_RX_NODATA_LIMIT	16
> +
> +static void tls_rx_data_ready_work(struct work_struct *w)
> +{
> +	struct tls_sw_context_rx *ctx =
> +		container_of(w, struct tls_sw_context_rx, data_ready_work);
> +	struct sock *sk = ctx->strp.sk;
> +
> +	/* A callback that finds the socket unowned reads from it. */
> +	lock_sock(sk);
> +	READ_ONCE(sk->sk_data_ready)(sk);
> +	release_sock(sk);
> +}

[ ... ]

> @@ -2136,14 +2151,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() does not call the
> +				 * consumer's sk_data_ready, which can re-enter
> +				 * this function.
> +				 */

[Severity: High]
Is this comment accurate? tls_rx_reader_release() begins by calling
tls_rx_msg_maybe_announce(). That function calls the consumer's callback
synchronously, and reader_present is still set when it does:

net/tls/tls_sw.c:tls_rx_reader_release() {
	tls_rx_msg_maybe_announce(&ctx->strp);
	...
	WRITE_ONCE(ctx->reader_present, 0);
}

net/tls/tls_sw.c:tls_rx_msg_maybe_announce() {
	if (!READ_ONCE(strp->msg_ready) || strp->msg_announced)
		return;
	strp->msg_announced = 1;
	...
	ctx->saved_data_ready(strp->sk);
}

The same concern applies to this part of the commit message:

  "Records left queued do not raise another sk_data_ready(), so the
  consumer needs one."

The new break seems to produce exactly that state. By the 16th empty
record, tls_rx_rec_done() has already run tls_strp_msg_consume(), which
clears msg_ready and msg_announced. It has also run
tls_strp_check_rcv(strp, false), which can parse the next fully received
record without announcing it. The loop then breaks while the consumer's
strparser is not paused.

Take a consumer such as KCM that installed psock_data_ready before TLS
RX was armed. tls_sw_strparser_arm() saves psock_data_ready as
saved_data_ready and installs tls_data_ready. In softirq, under
bh_lock_sock(), the outer reader is reached like this:

tcp_data_ready()
  tls_data_ready()
    tls_strp_data_ready()
      tls_strp_check_rcv()
        tls_rx_msg_maybe_announce()
          psock_data_ready()
            strp_data_ready()
              strp_read_sock()
                tls_sw_read_sock()

When that call takes the new break:

tls_rx_reader_release()
  tls_rx_msg_maybe_announce()
    psock_data_ready()
      strp_data_ready()          /* socket unowned, strp not paused */
        strp_read_sock()
          tls_sw_read_sock()     /* nested */
            tls_rx_reader_acquire()   /* reader_present == 1 */
              sk_wait_event()
                release_sock()   /* spin_lock_bh(&sk->sk_lock.slock) */

Can this self-deadlock on sk_lock.slock, which this CPU already holds
through bh_lock_sock()? It looks like a peer only has to send 17 or more
consecutive empty data records that arrive together in the receive
queue.

Before this patch the loop ran until no record was ready, so msg_ready
was clear when the reader was released.

In this configuration, does tls_rx_data_ready_work() reach the consumer
at all? There sk->sk_data_ready is tls_data_ready, not psock_data_ready.
Under lock_sock(), tls_data_ready only queues the tls_strp work, and
tls_strp_check_rcv() then returns early because msg_ready is already
set. The release-time announce goes to saved_data_ready while the work
item goes to sk->sk_data_ready, so the two notification paths seem to
disagree.

> +				schedule_work(&ctx->data_ready_work);

[Severity: Low]
schedule_work() queues this on system_wq, which lacks WQ_MEM_RECLAIM.
The existing TLS RX deferral uses tls_strp_wq instead:

net/tls/tls_strp.c:tls_strp_dev_init() {
	tls_strp_wq = create_workqueue("tls-strp");
	...
}

Some read_sock consumers, such as nvme-tcp over kTLS, sit in the memory
reclaim I/O path. After a burst of empty records, this work item is the
only thing that restarts the consumer. Could RX stall under memory
pressure until another event, such as new TCP data, arrives?

Would tls_strp_wq, or another WQ_MEM_RECLAIM workqueue, be a better
place for this work?

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

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007-tls-follow-on-v3-0-6d52d98389b3%40kernel.org

Re: [PATCH net-next v3 2/9] tls: Check for a pending signal after an empty record

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · 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).

- [High] In tls_sw_recvmsg() (net/tls/tls_sw.c), the only new way out of
  a run of empty application_data records is `if (nodata &&…
- [Medium] The new check in tls_sw_recvmsg() runs before
  tls_rx_rec_wait() and before any `!timeo` test.

--

tls: Check for a pending signal after an empty record

This commit adds a signal_pending() check to tls_sw_recvmsg(). The check
runs before the next record is fetched after a zero-length
application_data record, so a stream of empty records cannot hold the
caller in recvmsg() regardless of signals. The commit also decrypts empty
records synchronously, so the error from that check is not overwritten at
recv_end.

Closes: https://sashiko.dev/#/patchset/20260720-tcp-read-sock-v2-0-29545d034f3c@kernel.org?part=1

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 275a6047da9e3..32be5a8f80ed9 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]

Should a nonblocking reader get -EAGAIN here instead of -EINTR? With
MSG_DONTWAIT, sock_rcvtimeo(sk, true) returns 0, so sock_intr_errno(0)
returns -EINTR. This check also runs before any !timeo test.

Elsewhere the nonblocking -EAGAIN check comes first. For example,
tls_rx_reader_acquire() does:

    if (timeo <= 0)
        return -EAGAIN;
    if (signal_pending(current))
        return sock_intr_errno(timeo);

tls_rx_rec_wait() and tcp_recvmsg_locked() also test !timeo before they
check for signals.

signal_pending() is also true when only TIF_NOTIFY_SIGNAL is set. io_uring
rings without DEFER_TASKRUN queue task_work with TWA_SIGNAL or
TWA_SIGNAL_NO_IPI, and that sets TIF_NOTIFY_SIGNAL on the submitting task.
io_recv() and io_recvmsg() issue inline with MSG_DONTWAIT and retry via
poll only on -EAGAIN. Any other error fails the request:

    if (ret == -ERESTARTSYS)
        ret = -EINTR;
    req_set_fail(req);

Suppose an empty record arrives from the peer while task_work is queued to
the submitter. Could that now fail a nonblocking recv, or end a multishot
recv, with -EINTR even though no real signal was delivered? Before this
patch, the record loop did not return -EINTR to a nonblocking reader.

The same pattern also shows up in tls_sw_splice_read() later in this
series.

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

[ ... ]

> @@ -1910,6 +1924,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);

[Severity: High]

Does this put a limit on a run of empty records when the reader has copied
nothing? nodata is only used together with signal_pending(). Nothing
counts consecutive empty records or checks a timeout. With
decrypted + copied == 0, the loop condition stays true:

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

For MSG_DONTWAIT or O_NONBLOCK readers, tls_rx_rec_wait() returns 1
immediately whenever tls_strp_msg_ready() is true, so it never reaches
this check:

    if (!timeo)
        return -EAGAIN;

Because done stays 0, tls_read_flush_backlog() also keeps flushing the
backlog while tcp_inq(sk) <= max_rec, which pulls in more records during
the same call.

If a peer sends empty records faster than they can be decrypted, could it
keep an event-loop thread inside recvmsg() indefinitely, with no signal
ever arriving?

For blocking readers with SO_RCVTIMEO, tls_rx_rec_wait() recomputes
timeo = sock_rcvtimeo(sk, nonblock) on every call. Each empty record
therefore restarts the full timeout. By contrast, tcp_recvmsg_locked()
uses one timeo for the whole call.

Could one empty record per timeout period stop SO_RCVTIMEO from ever
expiring?

The commit message says "a nonblocking reader gets -EINTR". It seems a
nonblocking reader is still held unless a signal happens to arrive.

tls_sw_read_sock() in the same file already caps this case:

    if (++nodata >= TLS_RX_NODATA_LIMIT) {

Would tls_sw_recvmsg() need a similar limit, or a check against an overall
timeout?

A later commit in this series, "tls: Return copied data ahead of a run of
empty records", breaks out of the loop when decrypted + copied >= target.
That covers readers that have already met their target. Another later
commit message in the series says "A reader that has copied nothing still
reads empty records for as long as they arrive.", so the copied-nothing
cases above still look open at the end of the series.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007-tls-follow-on-v3-0-6d52d98389b3%40kernel.org

Re: [PATCH net-next v3 4/9] tls: Consume empty data records 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] tls_sw_splice_read() (net/tls/tls_sw.c) now has a `goto retry`
  loop that a peer can keep running.

--

tls: Consume empty data records in tls_sw_splice_read()

Before this commit, tls_sw_splice_read() returned zero when it hit a
zero-length application_data record, and callers read that as EOF. It
now consumes the empty record and jumps back to fetch the next one. A
signal_pending() check before each retry keeps 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 6c21897b03ee5..ff82e2f4e9e52 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c

[ ... ]

> @@ -2038,12 +2039,13 @@ 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, nonblock, true, false);
> +		err = tls_rx_rec_wait(sk, nonblock, released, false);
>  		if (err <= 0)
>  			goto splice_read_end;
>  

[ ... ]

> @@ -2066,6 +2071,16 @@ 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)) {
> +			err = sock_intr_errno(sock_rcvtimeo(sk, nonblock));
> +			goto splice_read_end;
> +		}
> +		goto retry;
> +	}

[Severity: Medium]
Can a peer keep this goto retry loop running for as long as it likes?

Nothing limits how many empty records one call consumes, and nonblock is
not checked between retries. The loop only exits on one of these:

  - a non-empty data record
  - a control record
  - tls_rx_rec_wait() returning <= 0
  - a pending signal

Suppose the peer sends authenticated empty records at least as fast as
they are decrypted. Then the next record is always ready, through one of
two paths:

  - tls_rx_rec_done()->tls_strp_check_rcv()
  - sk_flush_backlog() followed by tls_strp_check_rcv() inside
    tls_rx_rec_wait()

In that case tls_rx_rec_wait() either skips its wait loop or breaks out
before it reaches the -EAGAIN exit:

net/tls/tls_sw.c:tls_rx_rec_wait() {
    ...
	timeo = sock_rcvtimeo(sk, nonblock);

	while (!tls_strp_msg_ready(ctx)) {
		...
		if (!skb_queue_empty(&sk->sk_receive_queue)) {
			tls_strp_check_rcv(&ctx->strp, false);
			if (tls_strp_msg_ready(ctx))
				break;
		}
		...
		if (!timeo)
			return -EAGAIN;
    ...
}

If so, would a splice(2) or sendfile(2) caller using SPLICE_F_NONBLOCK or
O_NONBLOCK never see -EAGAIN, and never get back to its event loop while
the stream continues? The earlier patch in this series, "tls: Honor
O_NONBLOCK in tls_sw_splice_read()", was meant to prevent this kind of
event loop stall.

The task also holds lock_sock() and the TLS reader lock for the whole
run. Other users of the socket, such as tls_sw_sendmsg(), setsockopt()
and recvmsg(), would be blocked during that time.

Blocking callers have a similar problem. Each call to tls_rx_rec_wait()
computes a fresh timeo from sock_rcvtimeo(). Could one empty record
arriving just before each timeout expires keep a splice with SO_RCVTIMEO
waiting forever?

Elsewhere in this series, tls_sw_read_sock() caps the same pattern:

	if (++nodata >= TLS_RX_NODATA_LIMIT) {

At the end of the series, tls_sw_splice_read() still has no cap. Would it
make sense to cap consecutive empty records here as well? One option is
to reuse TLS_RX_NODATA_LIMIT and return -EAGAIN to nonblocking callers.
Another is to carry a single remaining-timeout budget across retries.

tls_sw_recvmsg() has the same unbounded behaviour. The patch "tls:
Consume empty data records in tls_sw_recvmsg()" acknowledges this.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007-tls-follow-on-v3-0-6d52d98389b3%40kernel.org

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

[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] The concern: with MSG_PEEK, the new empty-record branch in
  tls_sw_recvmsg() (line 1934) consumes the empty record and sets…
- [Low] At this commit, the only zero-length-record selftest,
  TEST_F(zero_len, test) in tools/testing/selftests/net/tls.c, uses TLS…

Pre-existing issues:
- [Medium] tls_sw_recvmsg() (net/tls/tls_sw.c) can return 0, which
  userspace reads as EOF, on a connection that is still live.
- [Medium] In tls_sw_recvmsg() (net/tls/tls_sw.c), a reader keeps
  consuming empty application_data records with the socket lock held for…

--

tls: Consume empty data records in tls_sw_recvmsg()

This patch adds a branch to tls_sw_recvmsg() that consumes zero-length
application_data records as soon as they are decrypted. Before this, the
peek and async arms queued every empty record on rx_list. The new branch
sets MSG_EOR itself and continues the receive loop.

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index ff82e2f4e9e52..b7f3edf9828e0 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c

[ ... ]

> @@ -1927,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);

[Severity: Low]
This isn't a bug, but at this commit the only zero-length record selftest
is TEST_F(zero_len, test) in tools/testing/selftests/net/tls.c. It receives
on a zc-capable TLS 1.2 socket with a user buffer and without MSG_PEEK:

	EXPECT_EQ(__tls_recv_cmsg(_metadata, self->cfd, NULL,
				  buf, sizeof(buf), MSG_DONTWAIT), ret);

So every empty record takes the darg.zc arm, and this consume_skb() call
never runs in that test.

A later patch in the series, "selftests: tls: Add peek and splice coverage
for zero-length records", adds TEST_F(zero_len_peek, test). That test uses
MSG_DONTWAIT | MSG_PEEK, which covers this non-zc path. The async arm is
still not covered, because that needs an async crypto driver.

> +
> +			/* An empty record still marks a boundary. */
> +			msg->msg_flags |= MSG_EOR;
> +			continue;
> +		}

[Severity: Medium]
This is a pre-existing issue, but can tls_sw_recvmsg() return 0 on a live
connection? This would happen when an empty application_data record is
followed directly by a non-data record, such as an alert or a TLS 1.3
NewSessionTicket or KeyUpdate.

For the empty record, tls_record_content_type() sets control to
TLS_RECORD_TYPE_DATA and emits the TLS_GET_RECORD_TYPE cmsg. This branch
then consumes the record and continues.

For the next record, tls_record_content_type() finds a type mismatch:

	} else if (*control != tlm->control) {
		return 0;
	}

The err <= 0 arm in tls_sw_recvmsg() then queues that skb on rx_list and
jumps to recv_end with err == 0. Since copied == 0, this returns 0:

	return copied ? : err;

An application that does not check the cmsg would read that as EOF. One
example is OpenSSL's ktls_read_record path.

The zc and sync non-zc arms already behaved this way before this patch.
The zero_len 0data_0data_ctrl variant in tools/testing/selftests/net/tls.c
also expects this result:

	.recs = { &id0_data_l0, &id1_data_l0, &id2_ctrl_l11, },
	.recv_ret = { 0, 11, -EAGAIN, },

Is a 0 return with a DATA cmsg the intended behaviour here?

[Severity: Medium]
This is a pre-existing issue, but this branch always continues. An empty
record changes neither bound in the loop condition:

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

So with the socket lock held, a reader keeps consuming empty records for
as long as the peer streams them. Only the nodata && signal_pending()
check at the top of the loop ends the run.

Take a MSG_DONTWAIT reader that has copied nothing. tls_rx_rec_wait()
returns -EAGAIN only when no record is ready, so that reader can stay in
the kernel while the peer keeps records coming. Other users of the socket
wait in lock_sock() during that time. tls_read_flush_backlog() can reach
__release_sock() and cond_resched(), so this is a stall rather than a
lockup.

The commit message mentions this: "still reads empty records for as long
as they arrive, until a signal is pending". The sibling reader,
tls_sw_read_sock(), caps a run of empty records:

	if (++nodata >= TLS_RX_NODATA_LIMIT) {
		schedule_work(&ctx->data_ready_work);
		break;
	}

Would a similar cap make sense in tls_sw_recvmsg()?

At this commit the branch also keeps going after the rcvlowat target has
been met. Data already copied is then held back while empty records keep
arriving. The next patch in the series, "tls: Return copied data ahead of
a run of empty records", adds a "decrypted + copied >= target" break to
this branch, which fixes that part.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007-tls-follow-on-v3-0-6d52d98389b3%40kernel.org

Re: [PATCH net-next v3 6/9] tls: Return copied data ahead of a run of empty 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] The commit "tls: Return copied data ahead of a run of empty
  records" fixes a delay a remote peer can cause in tls_sw_recvmsg(),…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the new
  early exit in tls_sw_recvmsg() only fires when `decrypted + copied >=…

--

tls: Return copied data ahead of a run of empty records

This patch makes tls_sw_recvmsg() leave the receive loop at a zero-length
application_data record once the receive low-water mark has been met. Data
that has already been copied then goes back to the reader. Before this, it
stayed in the loop while a peer kept sending empty records.

Link: https://lore.kernel.org/r/179124254665.434549.12245680127262431659@kernel.org

> Suggested-by: sashiko-bot <netdev-bot+sashiko@kernel.org>
> Link: https://lore.kernel.org/r/179124254665.434549.12245680127262431659@kernel.org
> Signed-off-by: Chuck Lever <cel@kernel.org>

[Severity: Low]
Should this commit carry a Fixes: tag?

The commit message describes a delay that a remote peer can cause, but the
trailers only contain Suggested-by and Link.

Every other kernel patch in this series has a Fixes: tag. That includes
"tls: Check for a pending signal after an empty record", which is also
described as not reproduced and uses Fixes: c46234ebb4d1.

The behavior predates the series. Before "tls: Consume empty data records
in tls_sw_recvmsg()", an empty record went through the normal path with
decrypted += 0 and len -= 0. The || tls_strp_msg_ready(ctx) loop condition
then kept the reader in the loop.

That parent commit carries Fixes: 692d7b5d1f91. It also adds the
!chunk && control == TLS_RECORD_TYPE_DATA branch that this patch changes.

Without a tag here, would stable backports that take the parent commit
miss this follow-up?

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index b7f3edf9828e0..fd6f944ffd677 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -1937,6 +1937,12 @@ int tls_sw_recvmsg(struct sock *sk,
>  
>  			/* An empty record still marks a boundary. */
>  			msg->msg_flags |= MSG_EOR;
> +
> +			/* Return data already copied instead of holding it
> +			 * through a run of empty records.
> +			 */
> +			if (decrypted + copied >= target)
> +				break;
>  			continue;
>  		}

[Severity: Medium]
This is a pre-existing issue and was not introduced by this patch. What
happens to a reader that has copied some data but is still below target?

In tls_sw_recvmsg(), target comes from:

	target = sock_rcvlowat(sk, flags & MSG_WAITALL, len);

Under MSG_WAITALL, including MSG_DONTWAIT | MSG_WAITALL, that is the full
len. With SO_RCVLOWAT set, it can also be larger than what was copied.

Such a reader still takes the continue on every empty DATA record.
Neither decrypted nor len advances.

The timeout doesn't seem to limit this. tls_rx_rec_wait() recomputes the
timeout on every call, but only checks it inside the wait loop:

	timeo = sock_rcvtimeo(sk, nonblock);

	while (!tls_strp_msg_ready(ctx)) {
		...
		if (!timeo)
			return -EAGAIN;
		...
	}

When a record is already parsed, the loop body never runs. So neither the
zero MSG_DONTWAIT timeout nor SO_RCVTIMEO is checked.

The backlog also keeps refilling. tls_read_flush_backlog() calls
sk_flush_backlog() whenever tcp_inq() is small:

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

	*flushed_at = done;
	return sk_flush_backlog(sk);

Could a peer that keeps sending empty records hold a nonblocking
MSG_WAITALL reader with partial progress in tls_sw_recvmsg()
indefinitely?

It looks like the call only ends on one of these:

  - a pending signal (the nodata && signal_pending(current) check)
  - an error
  - the peer stops sending empty records

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007-tls-follow-on-v3-0-6d52d98389b3%40kernel.org