[PATCH ipsec 0/7] xfrm: fix ESP IV generation and ESN authentication

[PATCH ipsec 0/7] xfrm: fix ESP IV generation and ESN authentication

[Jérémy Jean]

Hello,

This series fixes nonce reuse in AES-GCM and ESN authentication errors
in the IPv4 and IPv6 ESP offload paths:
* 1/7 and 2/7: Save the current sequence number before incrementing it,
               so that GCM IV construction prevents nonce reuse,
* 3/7: Use the current sequence for software AAD and hardware offload,
* 4/7: Give each early GSO segment a private secpath,
* 5/7: Use the packet sequence directly for mlx5 IV generation,
* 6/7: Segment untrusted GSO packets before allocating sequence numbers,
* 7/7: Keep the sequence counter unchanged after full ESN overflow.

Note that triggering the bug fixed by 7/7 is impractical: it requires
processing about 2^64 packets under a single key. Yet, I still believe
it's cleaner to fix it.

On the crypto side, as a reminder, reusing a nonce under the same GCM
key is catastrophic for security: it exposes the XOR of the plaintexts
and can reveal GCM authentication key, which opens the possibilty for
an adversary to forge ciphertexts without recovering the AES key.

This series combines and extends these two individual reports:
* https://lore.kernel.org/all/20260925095105.446269-2-Jeremy.Jean@oss.cyber.gouv.fr/
* https://lore.kernel.org/all/20260925095128.446450-2-Jeremy.Jean@oss.cyber.gouv.fr/

Recent discussion with Sabrina Dubroca may also be of interest:
https://lore.kernel.org/all/c443bad568f4e03d05b848b1b245707d@oss.cyber.gouv.fr/T/#u

Regards,
Jérémy

---

Jérémy Jean (7):
  xfrm: esp6: use the current sequence number for the IV
  xfrm: esp4: use the current sequence number for the IV
  xfrm: esp: use the current sequence number for AAD and offload
  xfrm: prevent AES-GCM nonce reuse after early GSO
  net/mlx5e: Use the packet sequence number for the IPsec IV
  xfrm: segment untrusted GSO packets before sequence allocation
  xfrm: leave the sequence counter unchanged on ESN overflow

 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c       | 12 +-----------
 net/ipv4/esp4.c                                    | 13 ++++---------
 net/ipv4/esp4_offload.c                            |  7 +++++--
 net/ipv6/esp6.c                                    | 13 ++++---------
 net/ipv6/esp6_offload.c                            |  7 +++++--
 net/xfrm/xfrm_output.c                             | 14 ++++++++++++--
 net/xfrm/xfrm_replay.c                             |  1 -
 7 files changed, 31 insertions(+), 36 deletions(-)


base-commit: 72d3fcf802c45d00b300f25b848a93c3a2bd7c7e
-- 
2.47.3

[PATCH ipsec 1/7] xfrm: esp6: use the current sequence number for the IV

[Jérémy Jean]

When a GSO packet is split in software for IPv6 ESP,
validate_xmit_xfrm() calls esp6_xmit() for each segment. These
segments have XFRM_GSO_SEGMENT set, but after the split, skb_is_gso()
returns false for each skb, so each call increments xo->seq.low by one
after writing the ESP sequence number.

When encryption is done in software, the AES-GCM IV is generated from
the incremented value: if the last segment has sequence number 100, it
uses 101 to generate its IV. A bug arises when the next ordinary
packet is processed: it also goes through esp6_xmit() with sequence
number 101, but since it does not have XFRM_GSO_SEGMENT, no increment
happens and the same IV is reused.

Save the full sequence number before incrementing xo->seq, and use
that value to generate the IV.

Fixes: 3dca3f38cfb8 ("xfrm: Separate ESP handling from segmentation for GRO packets.")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/ipv6/esp6_offload.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 22895521a57d..05d13cce22e4 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -346,6 +346,7 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features
 	}
 
 	seq = xo->seq.low;
+	esp.seqno = cpu_to_be64(seq + ((u64)xo->seq.hi << 32));
 
 	esp.esph = ip_esp_hdr(skb);
 	esp.esph->spi = x->id.spi;
@@ -364,8 +365,6 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features
 	if (xo->seq.low < seq)
 		xo->seq.hi++;
 
-	esp.seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
-
 	len = skb->len - sizeof(struct ipv6hdr);
 	if (len > IPV6_MAXPLEN)
 		len = 0;
-- 
2.47.3

[PATCH ipsec 2/7] xfrm: esp4: use the current sequence number for the IV

[Jérémy Jean]

When a GSO packet is split in software for IPv4 ESP,
validate_xmit_xfrm() calls esp_xmit() for each segment. These
segments have XFRM_GSO_SEGMENT set, but after the split, skb_is_gso()
returns false for each skb, so each call increments xo->seq.low by one
after writing the ESP sequence number.

The low bits are saved in seq before the increment, but esp.seqno is
set afterwards, using the saved low bits and the high bits from
xo->seq.hi.

When encryption is done in software with ESN enabled, the segment at
(H, 0xffffffff) uses (H+1, 0xffffffff) to generate the AES-GCM IV
because xo->seq.hi has already been incremented when the low bits
wrapped. One full 32-bit cycle later, if the packet at
(H+1, 0xffffffff) on the same SA is non-GSO, it generates the same IV.

Move the assignment to esp.seqno before xo->seq is advanced. This
saves H before the wrap increments xo->seq.hi to H+1, so the segment
at (H, 0xffffffff) generates its IV from (H, 0xffffffff).

Fixes: 4b549ccce941 ("xfrm: replay: Fix ESN wrap around for GSO")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/ipv4/esp4_offload.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index abd77162f5e7..3a0aafe991f4 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -316,6 +316,7 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
 	}
 
 	seq = xo->seq.low;
+	esp.seqno = cpu_to_be64(seq + ((u64)xo->seq.hi << 32));
 
 	esph = esp.esph;
 	esph->spi = x->id.spi;
@@ -334,8 +335,6 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
 	if (xo->seq.low < seq)
 		xo->seq.hi++;
 
-	esp.seqno = cpu_to_be64(seq + ((u64)xo->seq.hi << 32));
-
 	if (hw_offload && encap_type == UDP_ENCAP_ESPINUDP) {
 		/* In the XFRM stack, the encapsulation protocol is set to iphdr->protocol by
 		 * setting *skb_mac_header(skb) (see esp_output_udp_encap()) where skb->mac_header
-- 
2.47.3

[PATCH ipsec 3/7] xfrm: esp: use the current sequence number for AAD and offload

[Jérémy Jean]

When a GSO packet is split in software, esp_xmit() and esp6_xmit()
advance xo->seq before software encryption or the handoff to the NIC.
If the 64-bit sequence number of the segment is (H, 0xffffffff), the
counter becomes (H+1, 0). Both the software ESN helpers and drivers
such as Chelsio then use H+1 instead of H as part of the associated
data that is authenticated.

However, the ESN 32 high bits are not present in the ESP header: the
receiver reconstructs the 64-bit value from the received 32 low bits
and its own replay state, and uses it to check cryptographic
integrity. It uses H for this packet at the boundary, so the GCM tag
computed by the sender with H+1 fails verification, as it is different
from the one obtained by the receiver using H.

Use the high bits saved in esp->seqno for the software AAD. For
hardware encryption, restore the current sequence after skb_ext_add()
makes the secpath private, leaving the shared counter advanced for
the remaining segments. Restore both halves because drivers can also
use xo->seq to generate the IV.

Fixes: 3dca3f38cfb8 ("xfrm: Separate ESP handling from segmentation for GRO packets.")
Fixes: 4b549ccce941 ("xfrm: replay: Fix ESN wrap around for GSO")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/ipv4/esp4.c         | 13 ++++---------
 net/ipv4/esp4_offload.c |  4 ++++
 net/ipv6/esp6.c         | 13 ++++---------
 net/ipv6/esp6_offload.c |  4 ++++
 4 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index e76db5817e78..04f27c41ea50 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -271,20 +271,15 @@ static void esp_output_restore_header(struct sk_buff *skb)
 static struct ip_esp_hdr *esp_output_set_extra(struct sk_buff *skb,
 					       struct xfrm_state *x,
 					       struct ip_esp_hdr *esph,
-					       struct esp_output_extra *extra)
+					       struct esp_output_extra *extra,
+					       __be64 seqno)
 {
 	/* For ESN we move the header forward by 4 bytes to
 	 * accommodate the high bits.  We will move it back after
 	 * encryption.
 	 */
 	if ((x->props.flags & XFRM_STATE_ESN)) {
-		__u32 seqhi;
-		struct xfrm_offload *xo = xfrm_offload(skb);
-
-		if (xo)
-			seqhi = xo->seq.hi;
-		else
-			seqhi = XFRM_SKB_CB(skb)->seq.output.hi;
+		__u32 seqhi = upper_32_bits(be64_to_cpu(seqno));
 
 		extra->esphoff = (unsigned char *)esph -
 				 skb_transport_header(skb);
@@ -543,7 +538,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 	else
 		dsg = &sg[esp->nfrags];
 
-	esph = esp_output_set_extra(skb, x, esp->esph, extra);
+	esph = esp_output_set_extra(skb, x, esp->esph, extra, esp->seqno);
 	esp->esph = esph;
 
 	sg_init_table(sg, esp->nfrags);
diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index 3a0aafe991f4..6f6ce9ea5c49 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -358,6 +358,10 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
 		if (!xo)
 			return -EINVAL;
 
+		/* Keep the current sequence number in the private copy. */
+		xo->seq.low = seq;
+		xo->seq.hi = upper_32_bits(be64_to_cpu(esp.seqno));
+
 		xo->flags |= XFRM_XMIT;
 		return 0;
 	}
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index b1c9b36f76dc..7d3e9c5b3992 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -308,20 +308,15 @@ static void esp_output_restore_header(struct sk_buff *skb)
 static struct ip_esp_hdr *esp_output_set_esn(struct sk_buff *skb,
 					     struct xfrm_state *x,
 					     struct ip_esp_hdr *esph,
-					     struct esp_output_extra *extra)
+					     struct esp_output_extra *extra,
+					     __be64 seqno)
 {
 	/* For ESN we move the header forward by 4 bytes to
 	 * accommodate the high bits.  We will move it back after
 	 * encryption.
 	 */
 	if ((x->props.flags & XFRM_STATE_ESN)) {
-		__u32 seqhi;
-		struct xfrm_offload *xo = xfrm_offload(skb);
-
-		if (xo)
-			seqhi = xo->seq.hi;
-		else
-			seqhi = XFRM_SKB_CB(skb)->seq.output.hi;
+		__u32 seqhi = upper_32_bits(be64_to_cpu(seqno));
 
 		extra->esphoff = (unsigned char *)esph -
 				 skb_transport_header(skb);
@@ -575,7 +570,7 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
 	else
 		dsg = &sg[esp->nfrags];
 
-	esph = esp_output_set_esn(skb, x, esp->esph, extra);
+	esph = esp_output_set_esn(skb, x, esp->esph, extra, esp->seqno);
 	esp->esph = esph;
 
 	sg_init_table(sg, esp->nfrags);
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 05d13cce22e4..682063bfe685 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -379,6 +379,10 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features
 		if (!xo)
 			return -EINVAL;
 
+		/* Keep the current sequence number in the private copy. */
+		xo->seq.low = seq;
+		xo->seq.hi = upper_32_bits(be64_to_cpu(esp.seqno));
+
 		xo->flags |= XFRM_XMIT;
 		return 0;
 	}
-- 
2.47.3

[PATCH ipsec 4/7] xfrm: prevent AES-GCM nonce reuse after early GSO

[Jérémy Jean]

In the software ESP offload path, xfrm_output_gso() leaves its segments
sharing a secpath extension. Each segment gets its own ESP sequence
number, but stores it in the same xo->seq field for IV generation.

If encryption is delayed, later segments overwrite the value needed by
earlier ones: esp*_xmit() can then encrypt several packets with the same
AES-GCM nonce, despite their distinct ESP sequence numbers.

Give each segment a private secpath before allocating its sequence
number.

Fixes: a204aef9fd77 ("xfrm: call xfrm_output_gso when inner_protocol is set in xfrm_output")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/xfrm/xfrm_output.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index e305ba32e356..f6644daa68ba 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -672,6 +672,13 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb
 	skb_list_walk_safe(segs, segs, nskb) {
 		int err;
 
+		/* Each segment reserves its own sequence number below. */
+		if (xfrm_offload(segs) && !secpath_set(segs)) {
+			XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
+			kfree_skb_list(segs);
+			return -ENOMEM;
+		}
+
 		skb_mark_not_on_list(segs);
 		err = xfrm_output2(net, sk, segs);
 
-- 
2.47.3

Re: [PATCH ipsec 4/7] xfrm: prevent AES-GCM nonce reuse after early GSO

[Sabrina Dubroca]

2026-09-30, 14:45:21 +0000, Jérémy Jean wrote:
> In the software ESP offload path, xfrm_output_gso() leaves its segments
> sharing a secpath extension. Each segment gets its own ESP sequence
> number, but stores it in the same xo->seq field for IV generation.
> 
> If encryption is delayed, later segments overwrite the value needed by
> earlier ones: esp*_xmit() can then encrypt several packets with the same
> AES-GCM nonce, despite their distinct ESP sequence numbers.

Here again, your commit message could describe much more precisely
what actually happens.

I'm guessing you mean something that starts like

xfrm_output_gso
  segs = skb0,skb1... all with the same xo
  ...
  xfrm_output_one(skb0)
    xfrm_replay_overflow(skb0) xo->seq = N
  xfrm_output_one(skb1)
    xfrm_replay_overflow(skb1) xo->seq = N+1
  ...

[and then some more stuff happens that gives them the right
esph->seq_no but wrong 64b seqno used for the IV]

But please help reviewers trace the codepath you've already gone down,
without having to guess what you mean. You don't need to give the full
call graph with the state of each variable, but there needs to be more
than just the very beginning and the very end.

-- 
Sabrina

Re: [PATCH ipsec 4/7] xfrm: prevent AES-GCM nonce reuse after early GSO

[Jérémy Jean]

On 2026-10-01 15:31, Sabrina Dubroca wrote:
> 2026-09-30, 14:45:21 +0000, Jérémy Jean wrote:
>> In the software ESP offload path, xfrm_output_gso() leaves its 
>> segments
>> sharing a secpath extension. Each segment gets its own ESP sequence
>> number, but stores it in the same xo->seq field for IV generation.
>> 
>> If encryption is delayed, later segments overwrite the value needed by
>> earlier ones: esp*_xmit() can then encrypt several packets with the 
>> same
>> AES-GCM nonce, despite their distinct ESP sequence numbers.
> 
> Here again, your commit message could describe much more precisely
> what actually happens.
> 
> I'm guessing you mean something that starts like
> 
> xfrm_output_gso
>   segs = skb0,skb1... all with the same xo
>   ...
>   xfrm_output_one(skb0)
>     xfrm_replay_overflow(skb0) xo->seq = N
>   xfrm_output_one(skb1)
>     xfrm_replay_overflow(skb1) xo->seq = N+1
>   ...
> 
> [and then some more stuff happens that gives them the right
> esph->seq_no but wrong 64b seqno used for the IV]
> 
> But please help reviewers trace the codepath you've already gone down,
> without having to guess what you mean. You don't need to give the full
> call graph with the state of each variable, but there needs to be more
> than just the very beginning and the very end.

Here is a more verbose changelog I wrote for that patch:

---
With software ESP offload, xfrm_output() attaches a secpath before
calling xfrm_output_gso() for an encapsulated GSO packet, but
skb_gso_segment() leaves the splitted skbs sharing the same secpath,
including xo->seq.

For the i-th splitted segment, xfrm_output_gso() reaches
xfrm_replay_overflow() through xfrm_output_one(), which allocates
sequence number N+i to that segment, and then incorrectly overwrites
the shared xo->seq with N+i. If encryption is delayed until all t
segments have been processed, xo->seq ends up with value N+t-1 for all
segments. When validate_xmit_xfrm() calls esp*_xmit() for each
segment, the IV is constructed from the same value N+t-1, causing
nonce reuse in AES-GCM. Yet, each ESP header correctly keeps using
value N+i from its private XFRM_SKB_CB.

Call secpath_set() for each offloaded segment before xfrm_output2(),
so sequence allocation writes to private metadata that cannot be
overwritten by later segments.
---

Regards,
Jérémy

[PATCH ipsec 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV

[Jérémy Jean]

mlx5e_ipsec_set_iv_esn() may decrement xo->seq.hi for a GSO skb based
on the SA's current output counter. The metadata already contains the
high word for the first packet, so this can produce the wrong IV.

For a three-segment GSO skb starting at (H, 0), oseq is 2 and
oseq - gso_segs wraps to 0xffffffff. The helper then uses (H-1, 0)
for the IV instead of (H, 0). This can create a situation where GCM
reuses a nonce.

Remove the high-word adjustment and use xo->seq directly to generate
the IV.

Fixes: cb01008390bb ("net/mlx5: IPSec, Add support for ESN")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c         | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
index 6056106edcc6..4aa9f9c52f57 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
@@ -153,21 +153,11 @@ static void mlx5e_ipsec_set_swp(struct sk_buff *skb,
 void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
 			    struct xfrm_offload *xo)
 {
-	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
-	__u32 oseq = replay_esn->oseq;
 	int iv_offset;
 	__be64 seqno;
-	u32 seq_hi;
-
-	if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID &&
-		     MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - skb_shinfo(skb)->gso_segs))) {
-		seq_hi = xo->seq.hi - 1;
-	} else {
-		seq_hi = xo->seq.hi;
-	}
 
 	/* Place the SN in the IV field */
-	seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
+	seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
 	iv_offset = skb_transport_offset(skb) + sizeof(struct ip_esp_hdr);
 	skb_store_bits(skb, iv_offset, &seqno, 8);
 }
-- 
2.47.3

Re: [PATCH ipsec 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV

[Sabrina Dubroca]

2026-09-30, 14:45:22 +0000, Jérémy Jean wrote:
> mlx5e_ipsec_set_iv_esn() may decrement xo->seq.hi for a GSO skb based
> on the SA's current output counter. The metadata already contains the

And doing a check based on x->replay_esn->oseq, which may not match
the value for this packet, seems really weird.


> -	if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID &&
> -		     MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - skb_shinfo(skb)->gso_segs))) {
> -		seq_hi = xo->seq.hi - 1;
> -	} else {
> -		seq_hi = xo->seq.hi;
> -	}

I guess this is trying to solve the same thing as 4b549ccce941 ("xfrm:
replay: Fix ESN wrap around for GSO")?


>  	/* Place the SN in the IV field */
> -	seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
> +	seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));

This is starting to feel like we should have a helper to compose the
64b PN, and maybe give a name to the (low,hi) pair struct to pass it
to that helper.

-- 
Sabrina

Re: [PATCH ipsec 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV

[Jérémy Jean]

On 2026-10-01 13:42, Sabrina Dubroca wrote:
> 2026-09-30, 14:45:22 +0000, Jérémy Jean wrote:
>> mlx5e_ipsec_set_iv_esn() may decrement xo->seq.hi for a GSO skb based
>> on the SA's current output counter. The metadata already contains the
> 
> And doing a check based on x->replay_esn->oseq, which may not match
> the value for this packet, seems really weird.
> 
> 
>> -	if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID &&
>> -		     MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - 
>> skb_shinfo(skb)->gso_segs))) {
>> -		seq_hi = xo->seq.hi - 1;
>> -	} else {
>> -		seq_hi = xo->seq.hi;
>> -	}
> 
> I guess this is trying to solve the same thing as 4b549ccce941 ("xfrm:
> replay: Fix ESN wrap around for GSO")?

Yes, it seems so.

FWIW, another related commit to this fix is c05c5e5aa163 ("xfrm: replay:
Fix the update of replay_esn->oseq_hi for GSO").

>>  	/* Place the SN in the IV field */
>> -	seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
>> +	seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
> 
> This is starting to feel like we should have a helper to compose the
> 64b PN, and maybe give a name to the (low,hi) pair struct to pass it
> to that helper.

I don't have a strong opinion about this, but I'd say introducung a 
helper
to replace a oneliner is maybe not required?

Jérémy

Re: [PATCH ipsec 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV

[Tariq Toukan]

On 30/09/2026 17:45, Jérémy Jean wrote:
> mlx5e_ipsec_set_iv_esn() may decrement xo->seq.hi for a GSO skb based
> on the SA's current output counter. The metadata already contains the
> high word for the first packet, so this can produce the wrong IV.
> 
> For a three-segment GSO skb starting at (H, 0), oseq is 2 and
> oseq - gso_segs wraps to 0xffffffff. The helper then uses (H-1, 0)
> for the IV instead of (H, 0). This can create a situation where GCM
> reuses a nonce.
> 
> Remove the high-word adjustment and use xo->seq directly to generate
> the IV.
> 
> Fixes: cb01008390bb ("net/mlx5: IPSec, Add support for ESN")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>   .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c         | 12 +-----------
>   1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
> index 6056106edcc6..4aa9f9c52f57 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
> @@ -153,21 +153,11 @@ static void mlx5e_ipsec_set_swp(struct sk_buff *skb,
>   void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
>   			    struct xfrm_offload *xo)
>   {
> -	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
> -	__u32 oseq = replay_esn->oseq;
>   	int iv_offset;
>   	__be64 seqno;
> -	u32 seq_hi;
> -
> -	if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID &&
> -		     MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - skb_shinfo(skb)->gso_segs))) {
> -		seq_hi = xo->seq.hi - 1;
> -	} else {
> -		seq_hi = xo->seq.hi;
> -	}
>   
>   	/* Place the SN in the IV field */
> -	seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
> +	seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
>   	iv_offset = skb_transport_offset(skb) + sizeof(struct ip_esp_hdr);
>   	skb_store_bits(skb, iv_offset, &seqno, 8);
>   }

Thanks for your patch.

Doesn't this make mlx5e_ipsec_set_iv_esn() identical to 
mlx5e_ipsec_set_iv()?

I wouldn't keep both copies then..

Re: [PATCH ipsec 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV

[Jérémy Jean]

Hello Tariq,

On 2026-10-05 15:06, Tariq Toukan wrote:
> On 30/09/2026 17:45, Jérémy Jean wrote:
>> mlx5e_ipsec_set_iv_esn() may decrement xo->seq.hi for a GSO skb based
>> on the SA's current output counter. The metadata already contains the
>> high word for the first packet, so this can produce the wrong IV.
>> 
>> For a three-segment GSO skb starting at (H, 0), oseq is 2 and
>> oseq - gso_segs wraps to 0xffffffff. The helper then uses (H-1, 0)
>> for the IV instead of (H, 0). This can create a situation where GCM
>> reuses a nonce.
>> 
>> Remove the high-word adjustment and use xo->seq directly to generate
>> the IV.
>> 
>> Fixes: cb01008390bb ("net/mlx5: IPSec, Add support for ESN")
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM
>> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>> ---
>>   .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c         | 12 
>> +-----------
>>   1 file changed, 1 insertion(+), 11 deletions(-)
>> 
>> diff --git 
>> a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c 
>> b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>> index 6056106edcc6..4aa9f9c52f57 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>> @@ -153,21 +153,11 @@ static void mlx5e_ipsec_set_swp(struct sk_buff 
>> *skb,
>>   void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state 
>> *x,
>>   			    struct xfrm_offload *xo)
>>   {
>> -	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
>> -	__u32 oseq = replay_esn->oseq;
>>   	int iv_offset;
>>   	__be64 seqno;
>> -	u32 seq_hi;
>> -
>> -	if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID &&
>> -		     MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - 
>> skb_shinfo(skb)->gso_segs))) {
>> -		seq_hi = xo->seq.hi - 1;
>> -	} else {
>> -		seq_hi = xo->seq.hi;
>> -	}
>>     	/* Place the SN in the IV field */
>> -	seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
>> +	seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
>>   	iv_offset = skb_transport_offset(skb) + sizeof(struct ip_esp_hdr);
>>   	skb_store_bits(skb, iv_offset, &seqno, 8);
>>   }
> 
> Thanks for your patch.
> 
> Doesn't this make mlx5e_ipsec_set_iv_esn() identical to 
> mlx5e_ipsec_set_iv()?
> 
> I wouldn't keep both copies then..

Good point: after a quick check, indeed you are probably right.
I will look to simplify this in a new version of the patch series.

Jérémy

Re: [PATCH ipsec 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV

[Jérémy Jean]

On 2026-10-05 15:50, Jérémy Jean wrote:
> Hello Tariq,
> 
> On 2026-10-05 15:06, Tariq Toukan wrote:
>> On 30/09/2026 17:45, Jérémy Jean wrote:
>>> mlx5e_ipsec_set_iv_esn() may decrement xo->seq.hi for a GSO skb based
>>> on the SA's current output counter. The metadata already contains the
>>> high word for the first packet, so this can produce the wrong IV.
>>> 
>>> For a three-segment GSO skb starting at (H, 0), oseq is 2 and
>>> oseq - gso_segs wraps to 0xffffffff. The helper then uses (H-1, 0)
>>> for the IV instead of (H, 0). This can create a situation where GCM
>>> reuses a nonce.
>>> 
>>> Remove the high-word adjustment and use xo->seq directly to generate
>>> the IV.
>>> 
>>> Fixes: cb01008390bb ("net/mlx5: IPSec, Add support for ESN")
>>> Cc: stable@vger.kernel.org
>>> Assisted-by: LLM
>>> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>>> ---
>>>   .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c         | 12 
>>> +-----------
>>>   1 file changed, 1 insertion(+), 11 deletions(-)
>>> 
>>> diff --git 
>>> a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c 
>>> b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>>> index 6056106edcc6..4aa9f9c52f57 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>>> @@ -153,21 +153,11 @@ static void mlx5e_ipsec_set_swp(struct sk_buff 
>>> *skb,
>>>   void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state 
>>> *x,
>>>   			    struct xfrm_offload *xo)
>>>   {
>>> -	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
>>> -	__u32 oseq = replay_esn->oseq;
>>>   	int iv_offset;
>>>   	__be64 seqno;
>>> -	u32 seq_hi;
>>> -
>>> -	if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID &&
>>> -		     MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - 
>>> skb_shinfo(skb)->gso_segs))) {
>>> -		seq_hi = xo->seq.hi - 1;
>>> -	} else {
>>> -		seq_hi = xo->seq.hi;
>>> -	}
>>>     	/* Place the SN in the IV field */
>>> -	seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
>>> +	seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
>>>   	iv_offset = skb_transport_offset(skb) + sizeof(struct ip_esp_hdr);
>>>   	skb_store_bits(skb, iv_offset, &seqno, 8);
>>>   }
>> 
>> Thanks for your patch.
>> 
>> Doesn't this make mlx5e_ipsec_set_iv_esn() identical to 
>> mlx5e_ipsec_set_iv()?
>> 
>> I wouldn't keep both copies then..
> 
> Good point: after a quick check, indeed you are probably right.
> I will look to simplify this in a new version of the patch series.
> 
> Jérémy

It seems to me that a delete only patch would be okay then? (see
patch below).
The removal of the check in mlx5e_ipsec_set_esn_ops() makes
mlx5e_ipsec_set_iv() the only callable function. Then it seems
one can simply remove lines. Do you confirm?


  .../mellanox/mlx5/core/en_accel/ipsec.c       |  5 -----
  .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c  | 22 -------------------
  .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  |  2 --
  3 files changed, 29 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c 
b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
index db260e3..43ef5b3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
@@ -659,11 +659,6 @@ static void mlx5e_ipsec_set_esn_ops(struct 
mlx5e_ipsec_sa_entry *sa_entry)
             x->xso.dir != XFRM_DEV_OFFLOAD_OUT)
                 return;

-       if (x->props.flags & XFRM_STATE_ESN) {
-               sa_entry->set_iv_op = mlx5e_ipsec_set_iv_esn;
-               return;
-       }
-
         sa_entry->set_iv_op = mlx5e_ipsec_set_iv;
  }

diff --git 
a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c 
b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
index 6056106..9ffe094 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
@@ -150,28 +150,6 @@ static void mlx5e_ipsec_set_swp(struct sk_buff 
*skb,

  }

-void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
-                           struct xfrm_offload *xo)
-{
-       struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
-       __u32 oseq = replay_esn->oseq;
-       int iv_offset;
-       __be64 seqno;
-       u32 seq_hi;
-
-       if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID 
&&
-                    MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - 
skb_shinfo(skb)->gso_segs))) {
-               seq_hi = xo->seq.hi - 1;
-       } else {
-               seq_hi = xo->seq.hi;
-       }
-
-       /* Place the SN in the IV field */
-       seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
-       iv_offset = skb_transport_offset(skb) + sizeof(struct 
ip_esp_hdr);
-       skb_store_bits(skb, iv_offset, &seqno, 8);
-}
-
  void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_state *x,
                         struct xfrm_offload *xo)
  {
diff --git 
a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h 
b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
index 45b0d19..a3d92c9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
@@ -53,8 +53,6 @@ struct mlx5e_accel_tx_ipsec_state {

  #ifdef CONFIG_MLX5_EN_IPSEC

-void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
-                           struct xfrm_offload *xo);
  void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_state *x,
                         struct xfrm_offload *xo);
  bool mlx5e_ipsec_handle_tx_skb(struct net_device *netdev,

Re: [PATCH ipsec 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV

[Tariq Toukan]

On 05/10/2026 21:28, Jérémy Jean wrote:
> On 2026-10-05 15:50, Jérémy Jean wrote:
>> Hello Tariq,
>>
>> On 2026-10-05 15:06, Tariq Toukan wrote:
>>> On 30/09/2026 17:45, Jérémy Jean wrote:
>>>> mlx5e_ipsec_set_iv_esn() may decrement xo->seq.hi for a GSO skb based
>>>> on the SA's current output counter. The metadata already contains the
>>>> high word for the first packet, so this can produce the wrong IV.
>>>>
>>>> For a three-segment GSO skb starting at (H, 0), oseq is 2 and
>>>> oseq - gso_segs wraps to 0xffffffff. The helper then uses (H-1, 0)
>>>> for the IV instead of (H, 0). This can create a situation where GCM
>>>> reuses a nonce.
>>>>
>>>> Remove the high-word adjustment and use xo->seq directly to generate
>>>> the IV.
>>>>
>>>> Fixes: cb01008390bb ("net/mlx5: IPSec, Add support for ESN")
>>>> Cc: stable@vger.kernel.org
>>>> Assisted-by: LLM
>>>> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>>>> ---
>>>>   .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c         | 12 
>>>> +-----------
>>>>   1 file changed, 1 insertion(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ 
>>>> ipsec_rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ 
>>>> ipsec_rxtx.c
>>>> index 6056106edcc6..4aa9f9c52f57 100644
>>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>>>> @@ -153,21 +153,11 @@ static void mlx5e_ipsec_set_swp(struct sk_buff 
>>>> *skb,
>>>>   void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state 
>>>> *x,
>>>>                   struct xfrm_offload *xo)
>>>>   {
>>>> -    struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
>>>> -    __u32 oseq = replay_esn->oseq;
>>>>       int iv_offset;
>>>>       __be64 seqno;
>>>> -    u32 seq_hi;
>>>> -
>>>> -    if (unlikely(skb_is_gso(skb) && oseq < 
>>>> MLX5E_IPSEC_ESN_SCOPE_MID &&
>>>> -             MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - skb_shinfo(skb)- 
>>>> >gso_segs))) {
>>>> -        seq_hi = xo->seq.hi - 1;
>>>> -    } else {
>>>> -        seq_hi = xo->seq.hi;
>>>> -    }
>>>>         /* Place the SN in the IV field */
>>>> -    seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
>>>> +    seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
>>>>       iv_offset = skb_transport_offset(skb) + sizeof(struct 
>>>> ip_esp_hdr);
>>>>       skb_store_bits(skb, iv_offset, &seqno, 8);
>>>>   }
>>>
>>> Thanks for your patch.
>>>
>>> Doesn't this make mlx5e_ipsec_set_iv_esn() identical to 
>>> mlx5e_ipsec_set_iv()?
>>>
>>> I wouldn't keep both copies then..
>>
>> Good point: after a quick check, indeed you are probably right.
>> I will look to simplify this in a new version of the patch series.
>>
>> Jérémy
> 
> It seems to me that a delete only patch would be okay then? (see
> patch below).
> The removal of the check in mlx5e_ipsec_set_esn_ops() makes
> mlx5e_ipsec_set_iv() the only callable function. Then it seems
> one can simply remove lines. Do you confirm?
> 
> 

 From a quick look, it seems we can do even more by totally removing the 
set_iv_op function pointer.

>   .../mellanox/mlx5/core/en_accel/ipsec.c       |  5 -----
>   .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c  | 22 -------------------
>   .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  |  2 --
>   3 files changed, 29 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/ 
> drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> index db260e3..43ef5b3 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
> @@ -659,11 +659,6 @@ static void mlx5e_ipsec_set_esn_ops(struct 
> mlx5e_ipsec_sa_entry *sa_entry)
>              x->xso.dir != XFRM_DEV_OFFLOAD_OUT)
>                  return;
> 
> -       if (x->props.flags & XFRM_STATE_ESN) {
> -               sa_entry->set_iv_op = mlx5e_ipsec_set_iv_esn;
> -               return;
> -       }
> -
>          sa_entry->set_iv_op = mlx5e_ipsec_set_iv;
>   }
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ 
> ipsec_rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ 
> ipsec_rxtx.c
> index 6056106..9ffe094 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
> @@ -150,28 +150,6 @@ static void mlx5e_ipsec_set_swp(struct sk_buff *skb,
> 
>   }
> 
> -void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
> -                           struct xfrm_offload *xo)
> -{
> -       struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
> -       __u32 oseq = replay_esn->oseq;
> -       int iv_offset;
> -       __be64 seqno;
> -       u32 seq_hi;
> -
> -       if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID &&
> -                    MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - 
> skb_shinfo(skb)->gso_segs))) {
> -               seq_hi = xo->seq.hi - 1;
> -       } else {
> -               seq_hi = xo->seq.hi;
> -       }
> -
> -       /* Place the SN in the IV field */
> -       seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
> -       iv_offset = skb_transport_offset(skb) + sizeof(struct ip_esp_hdr);
> -       skb_store_bits(skb, iv_offset, &seqno, 8);
> -}
> -
>   void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_state *x,
>                          struct xfrm_offload *xo)
>   {
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ 
> ipsec_rxtx.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ 
> ipsec_rxtx.h
> index 45b0d19..a3d92c9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
> @@ -53,8 +53,6 @@ struct mlx5e_accel_tx_ipsec_state {
> 
>   #ifdef CONFIG_MLX5_EN_IPSEC
> 
> -void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
> -                           struct xfrm_offload *xo);
>   void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_state *x,
>                          struct xfrm_offload *xo);
>   bool mlx5e_ipsec_handle_tx_skb(struct net_device *netdev,

Re: [PATCH ipsec 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV

[Jérémy Jean]

On 2026-10-06 08:49, Tariq Toukan wrote:
> On 05/10/2026 21:28, Jérémy Jean wrote:
>> On 2026-10-05 15:50, Jérémy Jean wrote:
>>> Hello Tariq,
>>> 
>>> On 2026-10-05 15:06, Tariq Toukan wrote:
>>>> On 30/09/2026 17:45, Jérémy Jean wrote:
>>>>> mlx5e_ipsec_set_iv_esn() may decrement xo->seq.hi for a GSO skb 
>>>>> based
>>>>> on the SA's current output counter. The metadata already contains 
>>>>> the
>>>>> high word for the first packet, so this can produce the wrong IV.
>>>>> 
>>>>> For a three-segment GSO skb starting at (H, 0), oseq is 2 and
>>>>> oseq - gso_segs wraps to 0xffffffff. The helper then uses (H-1, 0)
>>>>> for the IV instead of (H, 0). This can create a situation where GCM
>>>>> reuses a nonce.
>>>>> 
>>>>> Remove the high-word adjustment and use xo->seq directly to 
>>>>> generate
>>>>> the IV.
>>>>> 
>>>>> Fixes: cb01008390bb ("net/mlx5: IPSec, Add support for ESN")
>>>>> Cc: stable@vger.kernel.org
>>>>> Assisted-by: LLM
>>>>> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>>>>> ---
>>>>>   .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c         | 12 
>>>>> +-----------
>>>>>   1 file changed, 1 insertion(+), 11 deletions(-)
>>>>> 
>>>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ 
>>>>> ipsec_rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ 
>>>>> ipsec_rxtx.c
>>>>> index 6056106edcc6..4aa9f9c52f57 100644
>>>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>>>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
>>>>> @@ -153,21 +153,11 @@ static void mlx5e_ipsec_set_swp(struct 
>>>>> sk_buff *skb,
>>>>>   void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct 
>>>>> xfrm_state *x,
>>>>>                   struct xfrm_offload *xo)
>>>>>   {
>>>>> -    struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
>>>>> -    __u32 oseq = replay_esn->oseq;
>>>>>       int iv_offset;
>>>>>       __be64 seqno;
>>>>> -    u32 seq_hi;
>>>>> -
>>>>> -    if (unlikely(skb_is_gso(skb) && oseq < 
>>>>> MLX5E_IPSEC_ESN_SCOPE_MID &&
>>>>> -             MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - skb_shinfo(skb)- 
>>>>> >gso_segs))) {
>>>>> -        seq_hi = xo->seq.hi - 1;
>>>>> -    } else {
>>>>> -        seq_hi = xo->seq.hi;
>>>>> -    }
>>>>>         /* Place the SN in the IV field */
>>>>> -    seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
>>>>> +    seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
>>>>>       iv_offset = skb_transport_offset(skb) + sizeof(struct 
>>>>> ip_esp_hdr);
>>>>>       skb_store_bits(skb, iv_offset, &seqno, 8);
>>>>>   }
>>>> 
>>>> Thanks for your patch.
>>>> 
>>>> Doesn't this make mlx5e_ipsec_set_iv_esn() identical to 
>>>> mlx5e_ipsec_set_iv()?
>>>> 
>>>> I wouldn't keep both copies then..
>>> 
>>> Good point: after a quick check, indeed you are probably right.
>>> I will look to simplify this in a new version of the patch series.
>>> 
>>> Jérémy
>> 
>> It seems to me that a delete only patch would be okay then? (see
>> patch below).
>> The removal of the check in mlx5e_ipsec_set_esn_ops() makes
>> mlx5e_ipsec_set_iv() the only callable function. Then it seems
>> one can simply remove lines. Do you confirm?
>> 
>> 
> 
> From a quick look, it seems we can do even more by totally removing the 
> set_iv_op function pointer.

Indeed, thanks for the suggestion.
Below is a diff to do this.
Does that you look right to you?

  .../mellanox/mlx5/core/en_accel/ipsec.c       | 18 -----------
  .../mellanox/mlx5/core/en_accel/ipsec.h       |  2 --
  .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c  | 31 +++----------------
  .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  |  4 ---
  4 files changed, 4 insertions(+), 51 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c 
b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
index db260e3..de2e740 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
@@ -651,22 +651,6 @@ static void mlx5e_ipsec_modify_state(struct 
work_struct *_work)
  	mlx5_accel_esp_modify_xfrm(sa_entry, attrs);
  }

-static void mlx5e_ipsec_set_esn_ops(struct mlx5e_ipsec_sa_entry 
*sa_entry)
-{
-	struct xfrm_state *x = sa_entry->x;
-
-	if (x->xso.type != XFRM_DEV_OFFLOAD_CRYPTO ||
-	    x->xso.dir != XFRM_DEV_OFFLOAD_OUT)
-		return;
-
-	if (x->props.flags & XFRM_STATE_ESN) {
-		sa_entry->set_iv_op = mlx5e_ipsec_set_iv_esn;
-		return;
-	}
-
-	sa_entry->set_iv_op = mlx5e_ipsec_set_iv;
-}
-
  static void mlx5e_ipsec_handle_netdev_event(struct work_struct *_work)
  {
  	struct mlx5e_ipsec_work *work =
@@ -859,8 +843,6 @@ static int mlx5e_xfrm_add_state(struct net_device 
*dev,
  	if (err)
  		goto err_add_rule;

-	mlx5e_ipsec_set_esn_ops(sa_entry);
-
  	if (sa_entry->dwork)
  		queue_delayed_work(ipsec->wq, &sa_entry->dwork->dwork,
  				   MLX5_IPSEC_RESCHED);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h 
b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
index abcbd38..172fce9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
@@ -278,8 +278,6 @@ struct mlx5e_ipsec_sa_entry {
  	struct net_device *dev;
  	struct mlx5e_ipsec *ipsec;
  	struct mlx5_accel_esp_xfrm_attrs attrs;
-	void (*set_iv_op)(struct sk_buff *skb, struct xfrm_state *x,
-			  struct xfrm_offload *xo);
  	u32 ipsec_obj_id;
  	u32 enc_key_id;
  	struct mlx5e_ipsec_rule ipsec_rule;
diff --git 
a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c 
b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
index 6056106..4a9c334 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
@@ -150,30 +150,7 @@ static void mlx5e_ipsec_set_swp(struct sk_buff 
*skb,

  }

-void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
-			    struct xfrm_offload *xo)
-{
-	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
-	__u32 oseq = replay_esn->oseq;
-	int iv_offset;
-	__be64 seqno;
-	u32 seq_hi;
-
-	if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID &&
-		     MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - skb_shinfo(skb)->gso_segs))) 
{
-		seq_hi = xo->seq.hi - 1;
-	} else {
-		seq_hi = xo->seq.hi;
-	}
-
-	/* Place the SN in the IV field */
-	seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
-	iv_offset = skb_transport_offset(skb) + sizeof(struct ip_esp_hdr);
-	skb_store_bits(skb, iv_offset, &seqno, 8);
-}
-
-void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_state *x,
-			struct xfrm_offload *xo)
+static void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_offload 
*xo)
  {
  	int iv_offset;
  	__be64 seqno;
@@ -264,7 +241,6 @@ bool mlx5e_ipsec_handle_tx_skb(struct net_device 
*netdev,
  {
  	struct mlx5e_priv *priv = netdev_priv(netdev);
  	struct xfrm_offload *xo = xfrm_offload(skb);
-	struct mlx5e_ipsec_sa_entry *sa_entry;
  	struct xfrm_state *x;
  	struct sec_path *sp;

@@ -293,8 +269,9 @@ bool mlx5e_ipsec_handle_tx_skb(struct net_device 
*netdev,
  			goto drop;
  		}

-	sa_entry = (struct mlx5e_ipsec_sa_entry *)x->xso.offload_handle;
-	sa_entry->set_iv_op(skb, x, xo);
+	if (x->xso.type == XFRM_DEV_OFFLOAD_CRYPTO &&
+	    x->xso.dir == XFRM_DEV_OFFLOAD_OUT)
+		mlx5e_ipsec_set_iv(skb, xo);
  	mlx5e_ipsec_set_state(priv, skb, x, xo, ipsec_st);

  	return true;
diff --git 
a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h 
b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
index 45b0d19..9a7e032 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
@@ -53,10 +53,6 @@ struct mlx5e_accel_tx_ipsec_state {

  #ifdef CONFIG_MLX5_EN_IPSEC

-void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
-			    struct xfrm_offload *xo);
-void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_state *x,
-			struct xfrm_offload *xo);
  bool mlx5e_ipsec_handle_tx_skb(struct net_device *netdev,
  			       struct sk_buff *skb,
  			       struct mlx5e_accel_tx_ipsec_state *ipsec_st);

[PATCH ipsec 6/7] xfrm: segment untrusted GSO packets before sequence allocation

[Jérémy Jean]

Packets received through TUN or virtio-net can have gso_segs == 0,
with SKB_GSO_DODGY set until the segmentation metadata is checked.
When ESP defers segmentation, gso_segs is still zero, so the SA
sequence counter is not advanced. The segments receive sequence
numbers starting at oseq + 1, which subsequent packets can reuse,
causing AES-GCM nonce reuse.

This affects IPv4 and IPv6, with or without ESN, when the ESP offload
type is registered. Hardware encryption is not required.

Segment SKB_GSO_DODGY packets in xfrm_output() before allocating
sequence numbers, so each segment gets its own number. Set the ESP
encapsulation flag in xfrm_output_one(), after early segmentation, so
UFO fragment offsets and flags are filled in correctly.

Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/xfrm/xfrm_output.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index f6644daa68ba..30b95aba10f0 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -505,6 +505,9 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 	if (err <= 0 || x->xso.type == XFRM_DEV_OFFLOAD_PACKET)
 		goto resume;
 
+	if (xfrm_offload(skb))
+		skb->encapsulation = 1;
+
 	do {
 		err = xfrm_skb_check_space(skb);
 		if (err) {
@@ -812,10 +815,10 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
 		xfrm_state_hold(x);
 
 		xfrm_get_inner_ipproto(skb, x);
-		skb->encapsulation = 1;
 
 		if (skb_is_gso(skb)) {
-			if (skb->inner_protocol && x->props.mode == XFRM_MODE_TUNNEL)
+			if ((skb_shinfo(skb)->gso_type & SKB_GSO_DODGY) ||
+			    (skb->inner_protocol && x->props.mode == XFRM_MODE_TUNNEL))
 				return xfrm_output_gso(net, sk, skb);
 
 			skb_shinfo(skb)->gso_type |= SKB_GSO_ESP;
-- 
2.47.3

[PATCH ipsec 7/7] xfrm: leave the sequence counter unchanged on ESN overflow

[Jérémy Jean]

When the 64-bit ESN counter overflows, xfrm_replay_overflow_offload_esn()
rejects the packet and rolls back the stored counter. It decrements
replay_esn->oseq even though only the local oseq has advanced, which
can later induce a reuse of the last sequence number and the
corresponding AES-GCM nonce. Both IPv4 and IPv6 are affected, yet,
processing about 2^64 packets under a single key is required to
trigger this bug, which is highly unlikely in regular use cases.

Leave the stored low word unchanged on overflow. The high word still
needs to be restored because it was already incremented.

Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/xfrm/xfrm_replay.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/xfrm/xfrm_replay.c b/net/xfrm/xfrm_replay.c
index dbdf8a39dffe..12457d97c819 100644
--- a/net/xfrm/xfrm_replay.c
+++ b/net/xfrm/xfrm_replay.c
@@ -721,7 +721,6 @@ static int xfrm_replay_overflow_offload_esn(struct xfrm_state *x, struct sk_buff
 				xo->seq.hi = oseq_hi;
 			}
 			if (replay_esn->oseq_hi == 0) {
-				replay_esn->oseq--;
 				replay_esn->oseq_hi--;
 				xfrm_audit_state_replay_overflow(x, skb);
 				err = -EOVERFLOW;
-- 
2.47.3

Re: [PATCH ipsec 7/7] xfrm: leave the sequence counter unchanged on ESN overflow

[Sabrina Dubroca]

2026-09-30, 14:45:24 +0000, Jérémy Jean wrote:
> When the 64-bit ESN counter overflows, xfrm_replay_overflow_offload_esn()
> rejects the packet and rolls back the stored counter. It decrements
> replay_esn->oseq even though only the local oseq has advanced, which
> can later induce a reuse of the last sequence number and the
> corresponding AES-GCM nonce. Both IPv4 and IPv6 are affected, yet,
> processing about 2^64 packets under a single key is required to
> trigger this bug, which is highly unlikely in regular use cases.
> 
> Leave the stored low word unchanged on overflow. The high word still
> needs to be restored because it was already incremented.

nit: using "low word" and "high word" instead of the actual variable
names doesn't help the readability of your commit messages

> Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>  net/xfrm/xfrm_replay.c | 1 -
>  1 file changed, 1 deletion(-)

I ran into that one as well while I was reviewing your previous
patches, but didn't get around to posting a patch.

Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

-- 
Sabrina

Re: [PATCH ipsec 7/7] xfrm: leave the sequence counter unchanged on ESN overflow

[Jérémy Jean]

Hello Sabrina,

On 2026-10-01 13:24, Sabrina Dubroca wrote:
> 2026-09-30, 14:45:24 +0000, Jérémy Jean wrote:
>> When the 64-bit ESN counter overflows, 
>> xfrm_replay_overflow_offload_esn()
>> rejects the packet and rolls back the stored counter. It decrements
>> replay_esn->oseq even though only the local oseq has advanced, which
>> can later induce a reuse of the last sequence number and the
>> corresponding AES-GCM nonce. Both IPv4 and IPv6 are affected, yet,
>> processing about 2^64 packets under a single key is required to
>> trigger this bug, which is highly unlikely in regular use cases.
>> 
>> Leave the stored low word unchanged on overflow. The high word still
>> needs to be restored because it was already incremented.
> 
> nit: using "low word" and "high word" instead of the actual variable
> names doesn't help the readability of your commit messages

Right, I could specify that. How do you suggest I do that? Should I send
a v2 for this single 7/7 patch, resend a v2 for the full 7-patch series
(possibly in a couple of days in case there are some other public
comments), or another option? Thanks.

>> Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for 
>> offloading")
>> Cc: stable@vger.kernel.org
>> Assisted-by: LLM
>> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>> ---
>>  net/xfrm/xfrm_replay.c | 1 -
>>  1 file changed, 1 deletion(-)
> 
> I ran into that one as well while I was reviewing your previous
> patches, but didn't get around to posting a patch.

Good, thanks for letting me know.

> Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

Re: [PATCH ipsec 7/7] xfrm: leave the sequence counter unchanged on ESN overflow

[Sabrina Dubroca]

2026-10-01, 13:37:54 +0200, Jérémy Jean wrote:
> Hello Sabrina,
> 
> On 2026-10-01 13:24, Sabrina Dubroca wrote:
> > 2026-09-30, 14:45:24 +0000, Jérémy Jean wrote:
> > > When the 64-bit ESN counter overflows,
> > > xfrm_replay_overflow_offload_esn()
> > > rejects the packet and rolls back the stored counter. It decrements
> > > replay_esn->oseq even though only the local oseq has advanced, which
> > > can later induce a reuse of the last sequence number and the
> > > corresponding AES-GCM nonce. Both IPv4 and IPv6 are affected, yet,
> > > processing about 2^64 packets under a single key is required to
> > > trigger this bug, which is highly unlikely in regular use cases.
> > > 
> > > Leave the stored low word unchanged on overflow. The high word still
> > > needs to be restored because it was already incremented.
> > 
> > nit: using "low word" and "high word" instead of the actual variable
> > names doesn't help the readability of your commit messages
> 
> Right, I could specify that. How do you suggest I do that? Should I send
> a v2 for this single 7/7 patch, resend a v2 for the full 7-patch series
> (possibly in a couple of days in case there are some other public
> comments), or another option? Thanks.

Unless there are other reasons to send a v2 (ie some more important
issues in a patch/in the series), or if someone requests it, then no,
it's not necessary. That's just a preference in wording (and possibly
just mine, maybe it doesn't bother anyone else).

In general, do not resend a single patch, always the full series.

-- 
Sabrina

Re: [PATCH ipsec 7/7] xfrm: leave the sequence counter unchanged on ESN overflow

[Jérémy Jean]

On 2026-10-01 13:45, Sabrina Dubroca wrote:
> 2026-10-01, 13:37:54 +0200, Jérémy Jean wrote:
>> Hello Sabrina,
>> 
>> On 2026-10-01 13:24, Sabrina Dubroca wrote:
>> > 2026-09-30, 14:45:24 +0000, Jérémy Jean wrote:
>> > > When the 64-bit ESN counter overflows,
>> > > xfrm_replay_overflow_offload_esn()
>> > > rejects the packet and rolls back the stored counter. It decrements
>> > > replay_esn->oseq even though only the local oseq has advanced, which
>> > > can later induce a reuse of the last sequence number and the
>> > > corresponding AES-GCM nonce. Both IPv4 and IPv6 are affected, yet,
>> > > processing about 2^64 packets under a single key is required to
>> > > trigger this bug, which is highly unlikely in regular use cases.
>> > >
>> > > Leave the stored low word unchanged on overflow. The high word still
>> > > needs to be restored because it was already incremented.
>> >
>> > nit: using "low word" and "high word" instead of the actual variable
>> > names doesn't help the readability of your commit messages
>> 
>> Right, I could specify that. How do you suggest I do that? Should I 
>> send
>> a v2 for this single 7/7 patch, resend a v2 for the full 7-patch 
>> series
>> (possibly in a couple of days in case there are some other public
>> comments), or another option? Thanks.
> 
> Unless there are other reasons to send a v2 (ie some more important
> issues in a patch/in the series), or if someone requests it, then no,
> it's not necessary. That's just a preference in wording (and possibly
> just mine, maybe it doesn't bother anyone else).

Okay, I'm not doing anything just yet then.

> In general, do not resend a single patch, always the full series.

Noted, thanks for the advice.

Cheers,
Jérémy

Re: [PATCH ipsec 0/7] xfrm: fix ESP IV generation and ESN authentication

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.