Re: [PATCH net] tls: device: keep tls_sw_fallback() from encrypting its own output

[netdev-bot+sashiko@kernel.org]

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

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

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] The patch makes complete_skb() in
  net/tls/tls_device_fallback.c run `nskb->decrypted = 0;` on every skb…

--

tls: device: keep tls_sw_fallback() from encrypting its own output

This patch makes tls_sw_fallback() pass through any skb that lacks the
decrypted bit. It also makes complete_skb() clear the decrypted bit on
the fallback's output. Together these stop a skb from being encrypted a
second time when it is validated at both a bond master and its slave.

> diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
> index 3b7d0ab2bcf1..24fba12febd1 100644
> --- a/net/tls/tls_device_fallback.c
> +++ b/net/tls/tls_device_fallback.c
> @@ -192,6 +192,14 @@ static void complete_skb(struct sk_buff *nskb, struct sk_buff *skb, int headln)
>  	skb->destructor = NULL;
>  	skb->sk = NULL;
>
> +	/* nskb carries ciphertext now, but skb_copy_header() inherited the
> +	 * plaintext original's decrypted bit. Clear it so the bit keeps meaning
> +	 * "still-plaintext, needs an encryptor": a requeued nskb would otherwise
> +	 * be re-validated and encrypted a second time on any re-entry into
> +	 * tls_sw_fallback().
> +	 */
> +	nskb->decrypted = 0;

[Severity: Medium]
Can clearing skb->decrypted here strip PSP encapsulation from fallback
output?

PSP also uses skb->decrypted on transmit, but it means "this skb belongs
to a PSP TX association". The bit is set at enqueue time:

include/net/psp/functions.h:psp_enqueue_set_decrypted() {
	...
	pas = psp_sk_assoc(sk);
	if (pas && pas->tx.spi)
		skb->decrypted = 1;
}

Drivers then use it to decide on PSP TX offload:

include/net/psp/functions.h:psp_skb_get_assoc_rcu() {
	if (!skb->decrypted || !skb->sk)
		return NULL;

	return psp_sk_get_assoc_rcu(skb->sk);
}

mlx5 reaches this through mlx5e_accel_tx_begin()->mlx5e_psp_is_offload(),
and netdevsim through nsim_do_psp(). mlx5e_accel_tx_begin() applies kTLS
and then PSP to the same skb.

Nothing seems to prevent one socket from having both features.
psp_sock_assoc_set_tx() installs psp_validate_xmit without checking for a
TLS ULP. tls_set_device_offload() overwrites sk->sk_validate_xmit_skb
unconditionally. Documentation/networking/psp.rst also describes starting
with TLS and then switching to PSP.

On such a socket the TLS SW fallback can engage in two ways.
tls_device_down() can install tls_validate_xmit_skb_sw() after a port
bounce, or tls_validate_xmit_skb() can fall back after a reroute. The
path is then:

tls_sw_fallback()
  tls_enc_skb()
    complete_skb()
      nskb->decrypted = 0;
mlx5e_accel_tx_begin()
  mlx5e_psp_is_offload()
    psp_skb_get_assoc_rcu() returns NULL

The result is that the segment goes out as plain TCP with no PSP header.

Before this patch, skb_copy_header() carried decrypted=1 over to nskb, so
PSP encapsulation still happened. Once the fallback is active,
retransmits go through it as well. The peer's __psp_sk_rx_policy_check()
would then drop the non-PSP data segments, so the connection could stall.
PSP protection would also be skipped silently on the wire, although the
payload stays TLS-encrypted.

Could this be fixed without clearing skb->decrypted? Some options:

- Use a separate marker for fallback output.
- Skip the bond-master pass in tls_validate_xmit_skb_sw() the same way
  tls_validate_xmit_skb() already does with netif_is_bond_master().
- Keep the bit when the socket has a PSP TX association.

> +
>  	update_chksum(nskb, headln);
>
>  	/* sock_efree means skb must gone through skb_orphan_partial() */

[ ... ]

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