Re: [PATCH net-next v17 04/15] tls: add TLS 1.3 hardware offload support

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] tls: add TLS 1.3 hardware offload support drops the
  TLS_1_2_VERSION guards in tls_set_device_offload() and…
- [Low] Documentation/networking/tls-offload.rst (Rekey section, lines
  404-408 at the parent commit) states that offload does not support TLS…

--

tls: add TLS 1.3 hardware offload support

This patch enables TLS 1.3 on the kTLS device offload path by dropping the
TLS_1_2_VERSION checks in tls_set_device_offload() and
tls_set_device_offload_rx(), appending the 1.3 content_type byte with the
tag in tls_device_record_close(), and teaching the device fallback
encryption path about the 1.3 IV, AAD and prepend layouts.

> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index f5e1b6b61ce38..ada66c0bd0756 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c

[ ... ]

> @@ -1101,11 +1110,6 @@ int tls_set_device_offload(struct sock *sk)
>  	}
>  
>  	crypto_info = &ctx->crypto_send.info;
> -	if (crypto_info->version != TLS_1_2_VERSION) {
> -		rc = -EOPNOTSUPP;
> -		goto release_netdev;
> -	}
> -
>  	cipher_desc = get_cipher_desc(crypto_info->cipher_type);
>  	if (!cipher_desc || !cipher_desc->offloadable) {
>  		rc = -EINVAL;
> @@ -1208,9 +1212,6 @@ int tls_set_device_offload_rx(struct sock *sk, struct tls_context *ctx)
>  	struct net_device *netdev;
>  	int rc = 0;
>  
> -	if (ctx->crypto_recv.info.version != TLS_1_2_VERSION)
> -		return -EOPNOTSUPP;
> -
>  	/* A rekey (setsockopt on an already-configured socket) is not
>  	 * supported on the device offload path yet; reject it here so the
>  	 * caller can decide (propagate the error for a HW connection, or

[Severity: Medium]

At this point in the series, TLS 1.3 becomes offloadable in both directions
while the rekey rejection right below in tls_set_device_offload_rx() is
still in place.  Can a HW-offloaded TLS 1.3 RX socket get permanently
stuck if the peer sends a KeyUpdate?

The peer KeyUpdate makes tls_check_pending_rekey() in net/tls/tls_sw.c set
sw_ctx->key_update_pending, and tls_rx_rec_wait() then bails out
unconditionally:

	/* a rekey is pending, let userspace deal with it */
	if (unlikely(ctx->key_update_pending))
		return -EKEYEXPIRED;

tls_sk_poll() also masks EPOLLIN in that state.  The only place that clears
the flag is tls_finish_key_update(), reached from the SW setup path, but
the setsockopt(TLS_RX) that would install the new key is rejected in
tls_set_device_offload_rx() for rx_conf != TLS_BASE and the error is
propagated by do_tls_setsockopt_conf() without a SW fallback.  So recvmsg()
keeps returning -EKEYEXPIRED with no way for userspace to recover.

TX side, a TLS 1.3 KeyUpdate that used to work via software now turns into
a hard setsockopt failure on an offload-capable NIC.

The later patches in this series ("tls: device: add TX KeyUpdate support"
and "tls: device: add RX KeyUpdate support") implement the rekey and make
tls_set_device_offload_rx() end in tls_sw_ctx_finalize(), which clears
key_update_pending, so the end state of the series looks fine.  Would it
make sense to reorder so this patch does not leave a bisectable window
where a remote peer can wedge an offloaded 1.3 socket?

[Severity: Low]

Should the Rekey section of Documentation/networking/tls-offload.rst be
updated in this patch?  It currently says:

	Rekey
	=====

	Offload does not currently support TLS 1.3, therefore key rotation
	is not a concern for offloaded connections at this point.

which no longer holds once the version guards above are gone and 1.3
crypto_info is programmed into the NIC for both directions.  The "TLS 1.3
Key Updates" section of Documentation/networking/tls.rst also describes
installing a new key with setsockopt(TLS_TX/TLS_RX) without mentioning the
hardware offload limitation that this patch temporarily leaves behind.

The later patch "tls: document TLS 1.3 hardware offload rekey handling"
does rewrite that section with the TX and RX rekey description, so this is
only a mismatch in the intermediate state of the series.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917224355.2288021-1-rjethwani%40purestorage.com