All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sabrina Dubroca <sd@queasysnail.net>
To: Chuck Lever <cel@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>,
	Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
	Dave Watson <davejwatson@fb.com>, Shuah Khan <shuah@kernel.org>,
	netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH net 5/9] net/tls: Consume empty data records in tls_sw_recvmsg()
Date: Thu, 30 Jul 2026 14:40:00 +0200	[thread overview]
Message-ID: <amtGINRBf2NTqt_J@krikkit> (raw)
In-Reply-To: <20260726-tls-follow-on-v1-5-99bf4cc1c729@kernel.org>

2026-07-26, 20:33:33 -0400, Chuck Lever wrote:
> TLS 1.2 and TLS 1.3 both permit zero-length application_data
> records as a traffic-analysis countermeasure (RFC 5246, Section
> 6.2.1; RFC 8446, Section 5.1). Such a record decrypts to
> full_len == 0, so every arm of the receive loop reaches
> "decrypted += chunk" and "len -= chunk" with chunk == 0: len
> never reaches zero, and tls_strp_msg_ready() holds the second
> loop term true while the peer keeps records arriving. The peek
> arm and the async arm also queue each record on rx_list, which
> then grows without bound. tls_rx_rec_wait() returns without
> waiting whenever a record is already parsed, so its signal check
> never runs

So we should just move the signal check to the top of
tls_rx_rec_wait()'s loop? (just after all the existing error handling
code)

Then we don't need to add custom code everywhere, and we don't need to
do that much special handling for 0-length records.

> and no other test in the loop consults
> signal_pending(). A peer streaming empty records therefore holds
> the caller in recvmsg(), unresponsive to SIGKILL, until it stops.
> 

[...]
> +		/* An empty record advances neither loop bound, so a flood
> +		 * of them can be interrupted only here. On the zero-copy
> +		 * path darg.skb is the strparser anchor, already released
> +		 * by tls_rx_rec_done().
> +		 */
> +		if (tls_rx_empty_data_rec(chunk, control)) {
> +			long timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> +
> +			if (!darg.zc)
> +				consume_skb(darg.skb);

I don't see why you need this special handling. Could you explain that?

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

(note: I know that ship has sailed)

I don't think the way ktls handles MSG_EOR on RX makes any sense,
outside of non-DATA records. For non-DATA records, we only ever
process one per call, so it works out.

For DATA records, We set MSG_EOR when we process the end of a record,
and then we process part of the next record. MSG_EOR is still set,
eventhough we're not finishing the read at a record
boundary. Userspace has no way of knowing where that record boundary
was, or how many there may have been.

-- 
Sabrina

  reply	other threads:[~2026-07-30 12:40 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-27  0:33 [PATCH net 0/9] net/tls: Receive-path fixes for zero-length data records Chuck Lever
2026-07-27  0:33 ` [PATCH net 1/9] net/tls: Bound time spent on no-data records in tls_sw_read_sock() Chuck Lever
2026-07-30  9:12   ` Sabrina Dubroca
2026-07-30 13:05     ` Chuck Lever
2026-07-27  0:33 ` [PATCH net 2/9] net/tls: Consume empty data records in tls_sw_splice_read() Chuck Lever
2026-07-30 10:38   ` Sabrina Dubroca
2026-07-27  0:33 ` [PATCH net 3/9] net/tls: Fail tls_sw_splice_read() after a failed async decrypt Chuck Lever
2026-07-27  0:33 ` [PATCH net 4/9] net/tls: Honor O_NONBLOCK in tls_sw_splice_read() Chuck Lever
2026-07-30 10:59   ` Sabrina Dubroca
2026-07-27  0:33 ` [PATCH net 5/9] net/tls: Consume empty data records in tls_sw_recvmsg() Chuck Lever
2026-07-30 12:40   ` Sabrina Dubroca [this message]
2026-07-27  0:33 ` [PATCH net 6/9] selftests: tls: add peek and splice coverage for zero-length records Chuck Lever
2026-07-27 14:07   ` Sabrina Dubroca
2026-07-27  0:33 ` [PATCH net 7/9] selftests: tls: skip the zero_len tests when TLS is unavailable Chuck Lever
2026-07-27  0:33 ` [PATCH net 8/9] selftests: tls: cover splice on a nonblocking socket Chuck Lever
2026-07-27  0:33 ` [PATCH net 9/9] selftests: tls: cover splice after a failed decrypt Chuck Lever
2026-07-27 15:19   ` Sabrina Dubroca

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=amtGINRBf2NTqt_J@krikkit \
    --to=sd@queasysnail.net \
    --cc=cel@kernel.org \
    --cc=davejwatson@fb.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.