Re: [PATCH net 5/9] net/tls: Consume empty data records in tls_sw_recvmsg()
"Chuck Lever" <[email protected]>
| Newsgroups | gmane.linux.network |
|---|---|
| Message-ID | <[email protected]> |
On Thu, Jul 30, 2026, at 8:40 AM, Sabrina Dubroca wrote: > 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) It appears that only the signal check can move. sk_err and sk_shutdown are tested under !tls_strp_msg_ready() so that an already-parsed record is delivered before an error or EOF is reported. Hoisting those would let an error preempt deliverable data. However, hoisting signal_pending() alone is safe, since the record stays parsed for the next call. It is also just two call sites rather than everywhere: recvmsg and splice. Let's target that consolidation against net-next rather than net. > > + if (!darg.zc) > > + consume_skb(darg.skb); > > I don't see why you need this special handling. Could you explain that? On the zero-copy path darg.skb is the strparser anchor, which tls_rx_rec_done() has already released. Freeing it again would be a double free. On the other path it is a freshly allocated clear_skb that nothing else owns. Or, were you requesting the placement of a documenting comment? > I don't think the way ktls handles MSG_EOR on RX makes any sense, > outside of non-DATA records. Agreed. -- Chuck Lever