Re: [PATCH net-next v12 3/6] tls: Move decrypt-failure abort into tls_rx_one_record()

On Thu, Jun 4, 2026, at 1:48 PM, Chuck Lever wrote:
> From: Chuck Lever <[email protected]>
>
> Three receive paths -- recvmsg, read_sock, and splice_read --
> each follow tls_rx_one_record() with the same tls_err_abort()
> call. Consolidate the abort into tls_rx_one_record() so the
> decrypt-and-abort sequence lives in one place.
>
> A tls_check_pending_rekey() failure after successful
> decryption no longer triggers tls_err_abort(). That path
> fires only when skb_copy_bits() fails on a valid skb,
> which is not a realistic scenario.
>
> Suggested-by: Sabrina Dubroca <[email protected]>
> Reviewed-by: Hannes Reinecke <[email protected]>
> Reviewed-by: Sabrina Dubroca <[email protected]>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>  net/tls/tls_sw.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index fd8d3c979368..798f2535ddf7 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -1827,6 +1827,9 @@ static int tls_check_pending_rekey(struct sock 
> *sk, struct tls_context *ctx,
>  	return 0;
>  }
> 
> +/* On decrypt failure the connection is aborted (sk_err set) before
> + * returning a negative errno.
> + */
>  static int tls_rx_one_record(struct sock *sk, struct msghdr *msg,
>  			     struct tls_decrypt_arg *darg)
>  {
> @@ -1838,8 +1841,10 @@ static int tls_rx_one_record(struct sock *sk, 
> struct msghdr *msg,
>  	err = tls_decrypt_device(sk, msg, tls_ctx, darg);
>  	if (!err)
>  		err = tls_decrypt_sw(sk, tls_ctx, msg, darg);
> -	if (err < 0)
> +	if (err < 0) {
> +		tls_err_abort(sk, -EBADMSG);
>  		return err;
> +	}
> 
>  	rxm = strp_msg(darg->skb);
>  	rxm->offset += prot->prepend_size;
> @@ -2150,10 +2155,8 @@ int tls_sw_recvmsg(struct sock *sk,
>  			darg.async = false;
> 
>  		err = tls_rx_one_record(sk, msg, &darg);
> -		if (err < 0) {
> -			tls_err_abort(sk, -EBADMSG);
> +		if (err < 0)
>  			goto recv_end;
> -		}
> 
>  		async |= darg.async;
> 
> @@ -2312,10 +2315,8 @@ ssize_t tls_sw_splice_read(struct socket *sock,  
> loff_t *ppos,
>  		memset(&darg.inargs, 0, sizeof(darg.inargs));
> 
>  		err = tls_rx_one_record(sk, NULL, &darg);
> -		if (err < 0) {
> -			tls_err_abort(sk, -EBADMSG);
> +		if (err < 0)
>  			goto splice_read_end;
> -		}
> 
>  		tls_rx_rec_done(ctx);
>  		skb = darg.skb;
> @@ -2398,10 +2399,8 @@ int tls_sw_read_sock(struct sock *sk, 
> read_descriptor_t *desc,
>  			memset(&darg.inargs, 0, sizeof(darg.inargs));
> 
>  			err = tls_rx_one_record(sk, NULL, &darg);
> -			if (err < 0) {
> -				tls_err_abort(sk, -EBADMSG);
> +			if (err < 0)
>  				goto read_sock_end;
> -			}
> 
>  			released = tls_read_flush_backlog(sk, prot, INT_MAX,
>  							  0, decrypted,
>
> -- 
> 2.54.0

I reviewed the Sashiko findings for this patch.

Finding 1 is a false positive. The multi-step scenario described in the
finding (unrealistic skb_copy_bits failure -> no-abort -> retry ->
double-decrement underflow) requires a precondition the commit message
explicitly calls "not a realistic scenario."

Finding 2 is a pre-existing issue. The ZC fallback path lives entirely
in tls_decrypt_sw(), which this patch does not touch.


-- 
Chuck Lever