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

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

[Rishikesh Jethwani]

A socket offloaded through a bond has every skb validated twice, once
at the master and again at the slave. tls_validate_xmit_skb() passes
the master-level call through for that reason (netif_is_bond_master()),
but tls_validate_xmit_skb_sw(), the validator tls_device_down()
installs, calls tls_sw_fallback() unconditionally, so it encrypts the
master-level ciphertext a second time with the same rcd_sn and IV. This
cancels the CTR keystream and exposes plaintext on the wire. For a
complete record encrypted twice in one skb, the second pass also
generates a valid GCM tag, so the peer accepts corrupted data without
an authentication failure.

Guard on the content rather than the device. Offload plaintext always
carries the decrypted bit (tls_push_data() and
tls_device_write_space() send with MSG_SENDPAGE_DECRYPTED), so
tls_sw_fallback() can pass through any skb without it: such a skb needs
no offload encryption. complete_skb() now clears the bit that
skb_copy_header() copied from the plaintext original, so the fallback's
own output passes through any later validation pass untouched. This
also does not rely on every path validating a skb at both the master
and the slave.

Tested on two hosts back to back over a mlx5 port, the only slave of
a bond0 (balance-xor, layer3+4), so each transmit is validated at the
master and again at the slave. Run from each host as sender, a TLS 1.2
AES-GCM-128 TX socket kept open across an administrative bounce of the
slave leaked every later record as plaintext before the fix; after it
each record is encrypted exactly once and the kTLS peer reads the
stream intact.

Fixes: c55dcdd435aa ("net/tls: Fix use-after-free after the TLS device goes down and up")
Cc: stable@vger.kernel.org # needs commit fc6d80eb5044 ("tcp: prevent collapsing skbs across boundary in rtx queue")
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 net/tls/tls_device_fallback.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

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;
+
 	update_chksum(nskb, headln);
 
 	/* sock_efree means skb must gone through skb_orphan_partial() */
@@ -371,6 +379,17 @@ static struct sk_buff *tls_sw_fallback(struct sock *sk, struct sk_buff *skb)
 	s32 sync_size = 0;
 	u64 rcd_sn;
 
+	/* Only skbs carrying the decrypted bit hold plaintext that awaits
+	 * offload encryption; anything else needs no work here. That
+	 * includes this function's own output (complete_skb() clears the
+	 * bit skb_copy_header() inherited), which is seen again when a skb
+	 * is validated a second time, e.g. a bond master handing it to a
+	 * slave via bond_dev_queue_xmit(). fill_sg_in() would map it back
+	 * onto its own record by tcp_seq, reusing rcd_sn and IV.
+	 */
+	if (!skb_is_decrypted(skb))
+		return skb;
+
 	/* worst case is:
 	 * MAX_SKB_FRAGS in tls_record_info
 	 * MAX_SKB_FRAGS + 1 in SKB head and frags.
-- 
2.50.1

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

[Sabrina Dubroca]

2026-10-19, 15:26:51 -0600, Rishikesh Jethwani wrote:
> A socket offloaded through a bond has every skb validated twice, once
> at the master and again at the slave. tls_validate_xmit_skb() passes
> the master-level call through for that reason (netif_is_bond_master()),
> but tls_validate_xmit_skb_sw(), the validator tls_device_down()
> installs, calls tls_sw_fallback() unconditionally, so it encrypts the
> master-level ciphertext a second time with the same rcd_sn and IV. This
> cancels the CTR keystream and exposes plaintext on the wire. For a
> complete record encrypted twice in one skb, the second pass also
> generates a valid GCM tag, so the peer accepts corrupted data without
> an authentication failure.
> 
> Guard on the content rather than the device. Offload plaintext always
> carries the decrypted bit (tls_push_data() and
> tls_device_write_space() send with MSG_SENDPAGE_DECRYPTED), so
> tls_sw_fallback() can pass through any skb without it: such a skb needs
> no offload encryption. complete_skb() now clears the bit that
> skb_copy_header() copied from the plaintext original, so the fallback's
> own output passes through any later validation pass untouched. This
> also does not rely on every path validating a skb at both the master
> and the slave.
> 
> Tested on two hosts back to back over a mlx5 port, the only slave of
> a bond0 (balance-xor, layer3+4), so each transmit is validated at the

As in another patch, a couple of selftests for "offload over
bond->NIC" would be good.

> master and again at the slave. Run from each host as sender, a TLS 1.2
> AES-GCM-128 TX socket kept open across an administrative bounce of the
> slave leaked every later record as plaintext before the fix; after it
> each record is encrypted exactly once and the kTLS peer reads the
> stream intact.

The wording and verbosity of this section ("administrative bounce", 3
lines to say "before it didn't work and now it does" which IMO
shouldn't even have to be said) needs to be cleaned up. The rest of
the commit message as well.

> Fixes: c55dcdd435aa ("net/tls: Fix use-after-free after the TLS device goes down and up")
> Cc: stable@vger.kernel.org # needs commit fc6d80eb5044 ("tcp: prevent collapsing skbs across boundary in rtx queue")
> Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
> ---
>  net/tls/tls_device_fallback.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> 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 is encrypted now" is enough. the rest of this comment (why it
was set and what the bug would be) really doesn't belong here.

> +	nskb->decrypted = 0;
> +
>  	update_chksum(nskb, headln);
>  
>  	/* sock_efree means skb must gone through skb_orphan_partial() */
> @@ -371,6 +379,17 @@ static struct sk_buff *tls_sw_fallback(struct sock *sk, struct sk_buff *skb)
>  	s32 sync_size = 0;
>  	u64 rcd_sn;
>  
> +	/* Only skbs carrying the decrypted bit hold plaintext that awaits
> +	 * offload encryption; anything else needs no work here. That
> +	 * includes this function's own output (complete_skb() clears the
> +	 * bit skb_copy_header() inherited), which is seen again when a skb
> +	 * is validated a second time, e.g. a bond master handing it to a
> +	 * slave via bond_dev_queue_xmit(). fill_sg_in() would map it back
> +	 * onto its own record by tcp_seq, reusing rcd_sn and IV.
> +	 */

Sorry, I'm not reading all this. 0 or 1 line of comment here, nothing more.

> +	if (!skb_is_decrypted(skb))
> +		return skb;
> +
>  	/* worst case is:
>  	 * MAX_SKB_FRAGS in tls_record_info
>  	 * MAX_SKB_FRAGS + 1 in SKB head and frags.
> -- 
> 2.50.1
> 

-- 
Sabrina

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

[Rishikesh Jethwani]

On Wed, Oct 21, 2026 at 2:04 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
>
> 2026-10-19, 15:26:51 -0600, Rishikesh Jethwani wrote:
> > A socket offloaded through a bond has every skb validated twice, once
> > at the master and again at the slave. tls_validate_xmit_skb() passes
> > the master-level call through for that reason (netif_is_bond_master()),
> > but tls_validate_xmit_skb_sw(), the validator tls_device_down()
> > installs, calls tls_sw_fallback() unconditionally, so it encrypts the
> > master-level ciphertext a second time with the same rcd_sn and IV. This
> > cancels the CTR keystream and exposes plaintext on the wire. For a
> > complete record encrypted twice in one skb, the second pass also
> > generates a valid GCM tag, so the peer accepts corrupted data without
> > an authentication failure.
> >
> > Guard on the content rather than the device. Offload plaintext always
> > carries the decrypted bit (tls_push_data() and
> > tls_device_write_space() send with MSG_SENDPAGE_DECRYPTED), so
> > tls_sw_fallback() can pass through any skb without it: such a skb needs
> > no offload encryption. complete_skb() now clears the bit that
> > skb_copy_header() copied from the plaintext original, so the fallback's
> > own output passes through any later validation pass untouched. This
> > also does not rely on every path validating a skb at both the master
> > and the slave.
> >
> > Tested on two hosts back to back over a mlx5 port, the only slave of
> > a bond0 (balance-xor, layer3+4), so each transmit is validated at the
>
> As in another patch, a couple of selftests for "offload over
> bond->NIC" would be good.

Will do. The hw offload test these would go into is still under
review:

https://lore.kernel.org/netdev/20261020181147.939575-2-rjethwani@purestorage.com/

Would you prefer I fold this fix and the bond tests into the next
revision of that series, or wait until it's applied and send v2 of
this patch with the tests on top?

> > master and again at the slave. Run from each host as sender, a TLS 1.2
> > AES-GCM-128 TX socket kept open across an administrative bounce of the
> > slave leaked every later record as plaintext before the fix; after it
> > each record is encrypted exactly once and the kTLS peer reads the
> > stream intact.
>
> The wording and verbosity of this section ("administrative bounce", 3
> lines to say "before it didn't work and now it does" which IMO
> shouldn't even have to be said) needs to be cleaned up. The rest of
> the commit message as well.
>
> > Fixes: c55dcdd435aa ("net/tls: Fix use-after-free after the TLS device goes down and up")
> > Cc: stable@vger.kernel.org # needs commit fc6d80eb5044 ("tcp: prevent collapsing skbs across boundary in rtx queue")
> > Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
> > ---
> >  net/tls/tls_device_fallback.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > 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 is encrypted now" is enough. the rest of this comment (why it
> was set and what the bug would be) really doesn't belong here.
>
> > +     nskb->decrypted = 0;
> > +
> >       update_chksum(nskb, headln);
> >
> >       /* sock_efree means skb must gone through skb_orphan_partial() */
> > @@ -371,6 +379,17 @@ static struct sk_buff *tls_sw_fallback(struct sock *sk, struct sk_buff *skb)
> >       s32 sync_size = 0;
> >       u64 rcd_sn;
> >
> > +     /* Only skbs carrying the decrypted bit hold plaintext that awaits
> > +      * offload encryption; anything else needs no work here. That
> > +      * includes this function's own output (complete_skb() clears the
> > +      * bit skb_copy_header() inherited), which is seen again when a skb
> > +      * is validated a second time, e.g. a bond master handing it to a
> > +      * slave via bond_dev_queue_xmit(). fill_sg_in() would map it back
> > +      * onto its own record by tcp_seq, reusing rcd_sn and IV.
> > +      */
>
> Sorry, I'm not reading all this. 0 or 1 line of comment here, nothing more.
>
> > +     if (!skb_is_decrypted(skb))
> > +             return skb;
> > +
> >       /* worst case is:
> >        * MAX_SKB_FRAGS in tls_record_info
> >        * MAX_SKB_FRAGS + 1 in SKB head and frags.
> > --
> > 2.50.1
> >
>
> --
> Sabrina

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

[netdev-bot+sashiko]

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