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

["Chuck Lever" <cel@kernel.org>]

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