Re: [PATCH net-next v10 3/7] tls: Re-present partially-consumed records in tls_sw_read_sock()

[Sabrina Dubroca <sd@queasysnail.net>]

2026-05-11, 19:25:54 -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> When read_actor() accepts only part of a record but desc->count
> is still non-zero, the receive loop currently falls through to
> the next iteration without freeing or requeuing the partially
> consumed skb. The next iteration overwrites skb, leaking the
> remainder of the current record and silently dropping stream
> data.
> 
> __tcp_read_sock() handles the same case by leaving the unread
> bytes available for the next iteration to re-present, though
> its mechanism (sequence-number re-lookup) differs from the TLS
> path's explicit queue management. Adopt the same loop-level
> behavior here: update rxm->offset and rxm->full_len, requeue
> the skb to the head of rx_list, and continue. The next
> iteration pops the same skb and re-presents the unread bytes
> to read_actor().
> 
> Fixes: 662fbcec32f4 ("net/tls: implement ->read_sock()")

Fixes typically go through "net", not "net-next".

> Cc: Sagi Grimberg <sagi@grimberg.me>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/tls/tls_sw.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 559bef05fee4..40cb0a92d88a 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -2411,13 +2411,13 @@ 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;
> -		} else {
> -			consume_skb(skb);
> -			if (!desc->count)
> -				break;
> +			__skb_queue_head(&ctx->rx_list, skb);
> +			skb = NULL;

Here you're NULLing a "stolen" skb...

> +			continue;

It doesn't make much sense to me to loop again if !desc->count, we'll
just dequeue the same skb again and go back to read_actor(). Do we
expect read_actor to do more work if we call it again once dest->count
has reached 0?

>  		}
> +		consume_skb(skb);

...but not here.
(which I don't find particularly useful in either case)

> +		if (!desc->count)
> +			break;

Maybe then change the loop to

    while (desc->count)


We'd end up with

	while (desc->count) {
		// ...
		if (used < rxm->full_len) {
			rxm->offset += used;
			rxm->full_len -= used;
			__skb_queue_head(&ctx->rx_list, skb);
		} else {
			consume_skb(skb);
		}
	}

-- 
Sabrina