From 4da7925c124a356fa2dcf45ff4defce19da9e56c Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Jun 2026 13:48:24 -0400 Subject: [PATCH 1/6] tls: Avoid evaluating freed skb in tls_sw_read_sock() loop tls_sw_read_sock() ends its receive loop with while (skb), but the else branch in the body calls consume_skb(skb) before the predicate is re-evaluated. A pointer becomes indeterminate when the object it points to reaches end-of-lifetime (C2011 6.2.4p2), and using an indeterminate value is undefined behavior (Annex J.2). The pointer is not dereferenced today -- the predicate either exits the loop or skb is overwritten at the top of the next iteration -- but any future change that adds a dereference between consume_skb() and the predicate would silently introduce a use-after-free. Replace the do/while form with an explicit for(;;) loop so termination happens through a break statement rather than predicate evaluation of a freed pointer. Cc: Sagi Grimberg Signed-off-by: Chuck Lever Reviewed-by: Hannes Reinecke Reviewed-by: Sabrina Dubroca Link: https://patch.msgid.link/20260604-tls-read-sock-v12-1-b114efa6e3e2@oracle.com Signed-off-by: Jakub Kicinski --- net/tls/tls_sw.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 964ebc268ee4..8e4e57721335 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -2383,7 +2383,7 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc, goto read_sock_end; decrypted = 0; - do { + for (;;) { if (!skb_queue_empty(&ctx->rx_list)) { skb = __skb_dequeue(&ctx->rx_list); rxm = strp_msg(skb); @@ -2435,9 +2435,9 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc, } else { consume_skb(skb); if (!desc->count) - skb = NULL; + break; } - } while (skb); + } read_sock_end: tls_rx_reader_release(sk, ctx); From 3f05d3bf9422ad33a467933067e7662ea67c001c Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Jun 2026 13:48:25 -0400 Subject: [PATCH 2/6] tls: Re-present partially-consumed records in tls_sw_read_sock() The tls_sw_read_sock() loop releases the current skb whether read_actor() consumed the full record or only a prefix. When the actor takes only part of the record and leaves desc->count non-zero, the remainder is lost: skb is neither requeued nor freed, and the next iteration overwrites it during dequeue or tls_rx_rec_wait(). No mainline consumer reaches this path today. The only in-tree TLS read_sock user is nvme/tcp, whose actor nvme_tcp_recv_skb() loops until the input length is exhausted and returns either the full length or a negative error. The path becomes reachable with the upcoming NFSD svcsock receive built on read_sock_cmsg. Its data actor, svc_tcp_recv_actor(), parses an RPC fragment stream incrementally and returns at fragment boundaries. When a TLS record carries the tail of one RPC fragment plus the head of the next, the actor returns fewer bytes than offered while leaving desc->count non-zero, and without re-presentation the trailing fragment header vanishes. __tcp_read_sock() handles the equivalent case for plain TCP by leaving the unread bytes available for the next iteration to re-present, via sequence-number re-lookup. Adopt the same loop-level behavior: when read_actor() consumes only part of the record, update rxm->offset and rxm->full_len and requeue the skb to the head of rx_list so the next iteration re-presents the unread bytes. Switch the open-ended for-loop to "while (desc->count)" so the partial- and full-consume arms share a single exit check and read_actor() is not re-invoked once desc->count is exhausted. Cc: Sagi Grimberg Signed-off-by: Chuck Lever Reviewed-by: Hannes Reinecke Reviewed-by: Sabrina Dubroca Link: https://patch.msgid.link/20260604-tls-read-sock-v12-2-b114efa6e3e2@oracle.com Signed-off-by: Jakub Kicinski --- net/tls/tls_sw.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 8e4e57721335..fd8d3c979368 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -2383,7 +2383,7 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc, goto read_sock_end; decrypted = 0; - for (;;) { + while (desc->count) { if (!skb_queue_empty(&ctx->rx_list)) { skb = __skb_dequeue(&ctx->rx_list); rxm = strp_msg(skb); @@ -2430,12 +2430,9 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc, if (used < rxm->full_len) { rxm->offset += used; rxm->full_len -= used; - if (!desc->count) - goto read_sock_requeue; + __skb_queue_head(&ctx->rx_list, skb); } else { consume_skb(skb); - if (!desc->count) - break; } } From 524bc67509ff3ee9731a02d553ab7e5bc4202e9d Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Jun 2026 13:48:26 -0400 Subject: [PATCH 3/6] tls: Move decrypt-failure abort into tls_rx_one_record() Three receive paths -- recvmsg, read_sock, and splice_read -- each follow tls_rx_one_record() with the same tls_err_abort() call. Consolidate the abort into tls_rx_one_record() so the decrypt-and-abort sequence lives in one place. A tls_check_pending_rekey() failure after successful decryption no longer triggers tls_err_abort(). That path fires only when skb_copy_bits() fails on a valid skb, which is not a realistic scenario. Suggested-by: Sabrina Dubroca Reviewed-by: Hannes Reinecke Reviewed-by: Sabrina Dubroca Signed-off-by: Chuck Lever Link: https://patch.msgid.link/20260604-tls-read-sock-v12-3-b114efa6e3e2@oracle.com Signed-off-by: Jakub Kicinski --- net/tls/tls_sw.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index fd8d3c979368..798f2535ddf7 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1827,6 +1827,9 @@ static int tls_check_pending_rekey(struct sock *sk, struct tls_context *ctx, return 0; } +/* On decrypt failure the connection is aborted (sk_err set) before + * returning a negative errno. + */ static int tls_rx_one_record(struct sock *sk, struct msghdr *msg, struct tls_decrypt_arg *darg) { @@ -1838,8 +1841,10 @@ static int tls_rx_one_record(struct sock *sk, struct msghdr *msg, err = tls_decrypt_device(sk, msg, tls_ctx, darg); if (!err) err = tls_decrypt_sw(sk, tls_ctx, msg, darg); - if (err < 0) + if (err < 0) { + tls_err_abort(sk, -EBADMSG); return err; + } rxm = strp_msg(darg->skb); rxm->offset += prot->prepend_size; @@ -2150,10 +2155,8 @@ int tls_sw_recvmsg(struct sock *sk, darg.async = false; err = tls_rx_one_record(sk, msg, &darg); - if (err < 0) { - tls_err_abort(sk, -EBADMSG); + if (err < 0) goto recv_end; - } async |= darg.async; @@ -2312,10 +2315,8 @@ ssize_t tls_sw_splice_read(struct socket *sock, loff_t *ppos, memset(&darg.inargs, 0, sizeof(darg.inargs)); err = tls_rx_one_record(sk, NULL, &darg); - if (err < 0) { - tls_err_abort(sk, -EBADMSG); + if (err < 0) goto splice_read_end; - } tls_rx_rec_done(ctx); skb = darg.skb; @@ -2398,10 +2399,8 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc, memset(&darg.inargs, 0, sizeof(darg.inargs)); err = tls_rx_one_record(sk, NULL, &darg); - if (err < 0) { - tls_err_abort(sk, -EBADMSG); + if (err < 0) goto read_sock_end; - } released = tls_read_flush_backlog(sk, prot, INT_MAX, 0, decrypted, From 8cf0c70ec84de78f8eb21070929a02ee2d98158e Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Jun 2026 13:48:27 -0400 Subject: [PATCH 4/6] tls: Factor tls_strp_msg_consume() from tls_strp_msg_done() tls_strp_msg_done() conflates releasing the current record with checking for the next one via tls_strp_check_rcv(). A subsequent patch needs to release a record without immediately triggering that check, so the release step is separated into tls_strp_msg_consume(). tls_strp_msg_done() is preserved as a wrapper for existing callers. Reviewed-by: Hannes Reinecke Reviewed-by: Alistair Francis Signed-off-by: Chuck Lever Reviewed-by: Sabrina Dubroca Link: https://patch.msgid.link/20260604-tls-read-sock-v12-4-b114efa6e3e2@oracle.com Signed-off-by: Jakub Kicinski --- net/tls/tls.h | 1 + net/tls/tls_strp.c | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/net/tls/tls.h b/net/tls/tls.h index 12f44cb649c9..cb0091e03f41 100644 --- a/net/tls/tls.h +++ b/net/tls/tls.h @@ -194,6 +194,7 @@ int tls_strp_init(struct tls_strparser *strp, struct sock *sk); void tls_strp_data_ready(struct tls_strparser *strp); void tls_strp_check_rcv(struct tls_strparser *strp); +void tls_strp_msg_consume(struct tls_strparser *strp); void tls_strp_msg_done(struct tls_strparser *strp); int tls_rx_msg_size(struct tls_strparser *strp, struct sk_buff *skb); diff --git a/net/tls/tls_strp.c b/net/tls/tls_strp.c index c72e88317627..e7aaee6efe6e 100644 --- a/net/tls/tls_strp.c +++ b/net/tls/tls_strp.c @@ -581,7 +581,12 @@ static void tls_strp_work(struct work_struct *w) release_sock(strp->sk); } -void tls_strp_msg_done(struct tls_strparser *strp) +/* Release the current record without triggering a check for the + * next record. Callers must invoke tls_strp_check_rcv() before + * releasing the socket lock, or queued data will stall until the + * next tls_strp_data_ready() event. + */ +void tls_strp_msg_consume(struct tls_strparser *strp) { WARN_ON(!strp->stm.full_len); @@ -592,7 +597,11 @@ void tls_strp_msg_done(struct tls_strparser *strp) WRITE_ONCE(strp->msg_ready, 0); memset(&strp->stm, 0, sizeof(strp->stm)); +} +void tls_strp_msg_done(struct tls_strparser *strp) +{ + tls_strp_msg_consume(strp); tls_strp_check_rcv(strp); } From 22f8bf8808dc80a44f88a3ab1776ddb1baaa12d8 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Jun 2026 13:48:28 -0400 Subject: [PATCH 5/6] tls: Suppress spurious saved_data_ready on all receive paths Each record release via tls_strp_msg_done() triggered tls_strp_check_rcv(), which called tls_rx_msg_ready() and fired saved_data_ready(). During a multi-record receive, the first N-1 wakeups are pure overhead: the caller is already running and will pick up subsequent records on the next loop iteration. The recvmsg and splice_read paths share this waste. Suppress per-record notifications and emit a single one on reader exit. tls_rx_rec_done() releases the current record and parses the next without announcing; tls_strp_check_rcv() gains a bool announce parameter so callers can request the quiet form. tls_rx_reader_release() fires the deferred announce on exit through tls_rx_msg_maybe_announce(), an idempotent helper that calls saved_data_ready() only when a record is parsed and has not yet been announced. To keep the final notification idempotent against records that the BH or the worker has already announced, tls_strparser gains a msg_announced bit. tls_rx_msg_maybe_announce() sets the bit when firing saved_data_ready(); the bit is cleared whenever the parsed record is wiped, by tls_strp_msg_consume() on consumption or by tls_strp_msg_load() when the lower socket loses bytes from under the parse. A second call for the same parsed record -- as when recvmsg() satisfies the request from ctx->rx_list without touching the strparser -- becomes a no-op. With no remaining callers, tls_strp_msg_done() is removed. Signed-off-by: Chuck Lever Reviewed-by: Hannes Reinecke Reviewed-by: Sabrina Dubroca Link: https://patch.msgid.link/20260604-tls-read-sock-v12-5-b114efa6e3e2@oracle.com Signed-off-by: Jakub Kicinski --- include/net/tls.h | 5 +++++ net/tls/tls.h | 5 ++--- net/tls/tls_main.c | 2 +- net/tls/tls_strp.c | 23 ++++++++++++----------- net/tls/tls_sw.c | 27 ++++++++++++++++++++++++--- 5 files changed, 44 insertions(+), 18 deletions(-) diff --git a/include/net/tls.h b/include/net/tls.h index ebd2550280ae..3811943288b3 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -111,11 +111,16 @@ struct tls_sw_context_tx { struct tls_strparser { struct sock *sk; + /* Bitfield word and msg_ready are serialized by the lower + * socket lock; BH and worker contexts both acquire it. + */ u32 mark : 8; u32 stopped : 1; u32 copy_mode : 1; u32 mixed_decrypted : 1; + u32 msg_announced : 1; + bool msg_ready; struct strp_msg stm; diff --git a/net/tls/tls.h b/net/tls/tls.h index cb0091e03f41..60a37bdaaa25 100644 --- a/net/tls/tls.h +++ b/net/tls/tls.h @@ -193,12 +193,11 @@ void tls_strp_stop(struct tls_strparser *strp); int tls_strp_init(struct tls_strparser *strp, struct sock *sk); void tls_strp_data_ready(struct tls_strparser *strp); -void tls_strp_check_rcv(struct tls_strparser *strp); +void tls_strp_check_rcv(struct tls_strparser *strp, bool announce); void tls_strp_msg_consume(struct tls_strparser *strp); -void tls_strp_msg_done(struct tls_strparser *strp); int tls_rx_msg_size(struct tls_strparser *strp, struct sk_buff *skb); -void tls_rx_msg_ready(struct tls_strparser *strp); +void tls_rx_msg_maybe_announce(struct tls_strparser *strp); bool tls_strp_msg_load(struct tls_strparser *strp, bool force_refresh); int tls_strp_msg_cow(struct tls_sw_context_rx *ctx); diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index fd39acf41a61..c10a3fd7fc17 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -769,7 +769,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval, } else { struct tls_sw_context_rx *rx_ctx = tls_sw_ctx_rx(ctx); - tls_strp_check_rcv(&rx_ctx->strp); + tls_strp_check_rcv(&rx_ctx->strp, true); } return 0; diff --git a/net/tls/tls_strp.c b/net/tls/tls_strp.c index e7aaee6efe6e..61b10c697ecc 100644 --- a/net/tls/tls_strp.c +++ b/net/tls/tls_strp.c @@ -368,7 +368,6 @@ static int tls_strp_copyin(read_descriptor_t *desc, struct sk_buff *in_skb, desc->count = 0; WRITE_ONCE(strp->msg_ready, 1); - tls_rx_msg_ready(strp); } return ret; @@ -492,6 +491,7 @@ bool tls_strp_msg_load(struct tls_strparser *strp, bool force_refresh) if (!strp->copy_mode && force_refresh) { if (unlikely(tcp_inq(strp->sk) < strp->stm.full_len)) { WRITE_ONCE(strp->msg_ready, 0); + strp->msg_announced = 0; memset(&strp->stm, 0, sizeof(strp->stm)); return false; } @@ -539,18 +539,24 @@ static int tls_strp_read_sock(struct tls_strparser *strp) return tls_strp_read_copy(strp, false); WRITE_ONCE(strp->msg_ready, 1); - tls_rx_msg_ready(strp); return 0; } -void tls_strp_check_rcv(struct tls_strparser *strp) +/* Parse queued data. When @announce is true and parsing produces a + * newly-ready record, fire the consumer notification. Callers that + * need to notify a waiter about a record parsed by another path + * should invoke tls_rx_msg_maybe_announce() directly. + */ +void tls_strp_check_rcv(struct tls_strparser *strp, bool announce) { if (unlikely(strp->stopped) || strp->msg_ready) return; if (tls_strp_read_sock(strp) == -ENOMEM) queue_work(tls_strp_wq, &strp->work); + else if (announce && strp->msg_ready) + tls_rx_msg_maybe_announce(strp); } /* Lower sock lock held */ @@ -568,7 +574,7 @@ void tls_strp_data_ready(struct tls_strparser *strp) return; } - tls_strp_check_rcv(strp); + tls_strp_check_rcv(strp, true); } static void tls_strp_work(struct work_struct *w) @@ -577,7 +583,7 @@ static void tls_strp_work(struct work_struct *w) container_of(w, struct tls_strparser, work); lock_sock(strp->sk); - tls_strp_check_rcv(strp); + tls_strp_check_rcv(strp, true); release_sock(strp->sk); } @@ -596,15 +602,10 @@ void tls_strp_msg_consume(struct tls_strparser *strp) tls_strp_flush_anchor_copy(strp); WRITE_ONCE(strp->msg_ready, 0); + strp->msg_announced = 0; memset(&strp->stm, 0, sizeof(strp->stm)); } -void tls_strp_msg_done(struct tls_strparser *strp) -{ - tls_strp_msg_consume(strp); - tls_strp_check_rcv(strp); -} - void tls_strp_stop(struct tls_strparser *strp) { strp->stopped = 1; diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 798f2535ddf7..df4cdf11f784 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1401,7 +1401,10 @@ tls_rx_rec_wait(struct sock *sk, struct sk_psock *psock, bool nonblock, return ret; if (!skb_queue_empty(&sk->sk_receive_queue)) { - tls_strp_check_rcv(&ctx->strp); + /* Defer notification to the exit point; this thread + * will consume the record directly. + */ + tls_strp_check_rcv(&ctx->strp, false); if (tls_strp_msg_ready(ctx)) break; } @@ -1887,9 +1890,13 @@ static int tls_record_content_type(struct msghdr *msg, struct tls_msg *tlm, return 1; } +/* The deferred announce is fired once on reader exit by + * tls_rx_reader_release(). + */ static void tls_rx_rec_done(struct tls_sw_context_rx *ctx) { - tls_strp_msg_done(&ctx->strp); + tls_strp_msg_consume(&ctx->strp); + tls_strp_check_rcv(&ctx->strp, false); } /* This function traverses the rx_list in tls receive context to copies the @@ -2044,6 +2051,12 @@ static int tls_rx_reader_lock(struct sock *sk, struct tls_sw_context_rx *ctx, static void tls_rx_reader_release(struct sock *sk, struct tls_sw_context_rx *ctx) { + /* Fire any deferred announce once per reader so that a record + * parsed but not yet announced becomes visible to the next + * reader. The call is idempotent through msg_announced. + */ + tls_rx_msg_maybe_announce(&ctx->strp); + if (unlikely(ctx->reader_contended)) { if (wq_has_sleeper(&ctx->wq)) wake_up(&ctx->wq); @@ -2520,10 +2533,18 @@ int tls_rx_msg_size(struct tls_strparser *strp, struct sk_buff *skb) return ret; } -void tls_rx_msg_ready(struct tls_strparser *strp) +/* Fire saved_data_ready() at most once per parsed record. The + * msg_announced bit is cleared by tls_strp_msg_consume() when the + * record is consumed, arming the next announcement. + */ +void tls_rx_msg_maybe_announce(struct tls_strparser *strp) { struct tls_sw_context_rx *ctx; + if (!READ_ONCE(strp->msg_ready) || strp->msg_announced) + return; + strp->msg_announced = 1; + ctx = container_of(strp, struct tls_sw_context_rx, strp); ctx->saved_data_ready(strp->sk); } From edcf32b8a48f5882b5b7a91b21c89d233d4aecf2 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Jun 2026 13:48:29 -0400 Subject: [PATCH 6/6] tls: Flush backlog before waiting for a new record While lock_sock is held, incoming TCP segments land on sk->sk_backlog rather than sk->sk_receive_queue. tls_rx_rec_wait() inspects only sk_receive_queue, so backlog data remains invisible. For non-blocking callers (read_sock, and recvmsg or splice_read with MSG_DONTWAIT) this causes a spurious -EAGAIN. For blocking callers it forces an unnecessary sleep/wakeup cycle. Flush the backlog inside tls_rx_rec_wait() before checking sk_receive_queue so the strparser can parse newly-arrived segments immediately. On the next loop iteration tls_read_flush_backlog() may redundantly flush, but this path is cold and the cost is negligible. Backlog processing can run tcp_reset(), which calls tcp_done_with_error() to set sk->sk_err = ECONNRESET and then tcp_done() to set sk->sk_shutdown = SHUTDOWN_MASK. The pre-existing top-of-loop sk_err check already ran before the flush, so the freshly-set error would be masked by the next-line sk_shutdown test returning 0 (EOF). Re-check sk_err immediately before the sk_shutdown test so a connection abort surfaces as -ECONNRESET rather than a clean EOF. Commit f508262ae9f2 ("tls: Preserve sk_err across recvmsg() when data has been copied") gave the top-of-loop sk_err check a has_copied split. The recheck applies the same handling: when the caller has already copied bytes, sk_err is reported but preserved so the error surfaces on the next call; otherwise sock_error() consumes it so the error is reported exactly once. Suggested-by: Sabrina Dubroca Link: https://lore.kernel.org/netdev/ahgHgQ84RCc8uYrG@krikkit/ Reviewed-by: Hannes Reinecke Signed-off-by: Chuck Lever Reviewed-by: Sabrina Dubroca Link: https://patch.msgid.link/20260604-tls-read-sock-v12-6-b114efa6e3e2@oracle.com Signed-off-by: Jakub Kicinski --- 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 df4cdf11f784..5a4300c943a1 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1400,6 +1400,8 @@ tls_rx_rec_wait(struct sock *sk, struct sk_psock *psock, bool nonblock, if (ret < 0) return ret; + if (sk_flush_backlog(sk)) + released = true; if (!skb_queue_empty(&sk->sk_receive_queue)) { /* Defer notification to the exit point; this thread * will consume the record directly. @@ -1409,6 +1411,16 @@ tls_rx_rec_wait(struct sock *sk, struct sk_psock *psock, bool nonblock, break; } + /* sk_flush_backlog() can run tcp_reset(), which sets + * sk_err and then sk_shutdown via tcp_done(). Recheck + * sk_err here so a connection abort surfaces as the + * actual error rather than a clean EOF. + */ + if (sk->sk_err) { + if (has_copied) + return -READ_ONCE(sk->sk_err); + return sock_error(sk); + } if (sk->sk_shutdown & RCV_SHUTDOWN) return 0;