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/[email protected]/ 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, 940 tests with no skips. --- Changes in v6: - Drop the empty-record cap from tls_sw_splice_read() (sashiko). - Drop the zero_len_splice variant that reached the cap. - State why the read_sock consumer cannot bound the run (sashiko). - Name the later patches the O_NONBLOCK description relies on (sashiko). - Note the MSG_EOR change under MSG_PEEK in the recvmsg patch (sashiko). - Link to v5: https://patch.msgid.link/[email protected] Changes in v5: - Restore the v3 data_ready work item in tls_sw_read_sock() (sashiko). - Queue the work item on tls_strp_wq, not system_wq (sashiko). - Queue the work item only when a record is left parsed at the break. - Skip the work item's callback once the strparser is stopped. - Reword the signal patch: -EAGAIN needs a pending signal (sashiko). - Cap consecutive empty records in tls_sw_splice_read() (sashiko). - Define TLS_RX_NODATA_LIMIT above tls_sw_splice_read(). - Add a zero_len_splice variant that reaches the cap. - Link to v4: https://patch.msgid.link/[email protected] Changes in v4: - Drop the data_ready work item; the reader release announces (sashiko). - Return -EAGAIN to a nonblocking reader with a signal pending (sashiko). - Add a Fixes: tag to the low-water mark patch (sashiko). - Add a zero_len variant that pins the low-water mark patch's behavior. - Link to v3: https://patch.msgid.link/[email protected] 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/[email protected] 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/[email protected] - Link to v1: https://patch.msgid.link/[email protected] --- 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: Extend coverage for zero-length records selftests: tls: Cover splice on a nonblocking socket include/net/tls.h | 1 + net/tls/tls.h | 2 + net/tls/tls_strp.c | 2 +- net/tls/tls_sw.c | 98 +++++++++++++- tools/testing/selftests/net/tls.c | 261 +++++++++++++++++++++++++++++++++++--- 5 files changed, 340 insertions(+), 24 deletions(-) --- base-commit: 05b5019b01fb9443b09ae4ed34af1135a80a84c8 change-id: 20260726-tls-follow-on-486f1ba8bbb0 Best regards, -- Chuck Lever <[email protected]>
