[PATCH ipsec] esp: Force skb_cow_data() on RX when the skb is non-linear

[PATCH ipsec] esp: Force skb_cow_data() on RX when the skb is non-linear

[Hyunwoo Kim]

esp_input() and esp6_input() skip skb_cow_data() on uncloned skbs
through two arms: one for fully linear skbs (nfrags = 1) and one for
skbs carrying paged fragments without a frag_list, which sets
nfrags = skb_shinfo(skb)->nr_frags + 1.  In both arms the skb is mapped
into a scatterlist via skb_to_sgvec() and passed as both src and dst of
aead_request_set_crypt(), so the AEAD operates in place over the
existing frag pages.

Drop the paged-fragment arm so any non-linear inbound skb falls through
to skb_cow_data().  The fully linear fast path is unchanged and the
existing skb_cow_data() error handling that follows the gate is reused.

Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible")
Fixes: 03e2a30f6a27 ("esp6: Avoid skb_cow_data whenever possible")
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
 net/ipv4/esp4.c | 13 +++----------
 net/ipv6/esp6.c | 13 +++----------
 2 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 6dfc0bcdef65..a6fbdec139dc 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -868,17 +868,10 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
 		assoclen += seqhilen;
 	}
 
-	if (!skb_cloned(skb)) {
-		if (!skb_is_nonlinear(skb)) {
-			nfrags = 1;
-
-			goto skip_cow;
-		} else if (!skb_has_frag_list(skb)) {
-			nfrags = skb_shinfo(skb)->nr_frags;
-			nfrags++;
+	if (!skb_cloned(skb) && !skb_is_nonlinear(skb)) {
+		nfrags = 1;
 
-			goto skip_cow;
-		}
+		goto skip_cow;
 	}
 
 	err = skb_cow_data(skb, 0, &trailer);
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 9f75313734f8..e5504cc0ecf9 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -910,17 +910,10 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
 		assoclen += seqhilen;
 	}
 
-	if (!skb_cloned(skb)) {
-		if (!skb_is_nonlinear(skb)) {
-			nfrags = 1;
-
-			goto skip_cow;
-		} else if (!skb_has_frag_list(skb)) {
-			nfrags = skb_shinfo(skb)->nr_frags;
-			nfrags++;
+	if (!skb_cloned(skb) && !skb_is_nonlinear(skb)) {
+		nfrags = 1;
 
-			goto skip_cow;
-		}
+		goto skip_cow;
 	}
 
 	nfrags = skb_cow_data(skb, 0, &trailer);
-- 
2.43.0

Re: [PATCH ipsec] esp: Force skb_cow_data() on RX when the skb is non-linear

[Herbert Xu]

On Thu, Apr 30, 2026 at 11:49:13AM +0900, Hyunwoo Kim wrote:
> esp_input() and esp6_input() skip skb_cow_data() on uncloned skbs
> through two arms: one for fully linear skbs (nfrags = 1) and one for
> skbs carrying paged fragments without a frag_list, which sets
> nfrags = skb_shinfo(skb)->nr_frags + 1.  In both arms the skb is mapped
> into a scatterlist via skb_to_sgvec() and passed as both src and dst of
> aead_request_set_crypt(), so the AEAD operates in place over the
> existing frag pages.
> 
> Drop the paged-fragment arm so any non-linear inbound skb falls through
> to skb_cow_data().  The fully linear fast path is unchanged and the
> existing skb_cow_data() error handling that follows the gate is reused.
> 
> Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible")
> Fixes: 03e2a30f6a27 ("esp6: Avoid skb_cow_data whenever possible")
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> ---
>  net/ipv4/esp4.c | 13 +++----------
>  net/ipv6/esp6.c | 13 +++----------
>  2 files changed, 6 insertions(+), 20 deletions(-)

Good catch!

When a packet comes from a device driver, it's usually safe to
write to the fragments since they would have been allocated by
the driver.

But when a packet originates from our own stack, then it's not
safe to write to the fragments.

Unfortunately the two paths cross with the loopback driver (and
probably other means of creating loopback).

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH ipsec] esp: Force skb_cow_data() on RX when the skb is non-linear

[Hyunwoo Kim]

On Thu, Apr 30, 2026 at 12:26:16PM +0800, Herbert Xu wrote:
> On Thu, Apr 30, 2026 at 11:49:13AM +0900, Hyunwoo Kim wrote:
> > esp_input() and esp6_input() skip skb_cow_data() on uncloned skbs
> > through two arms: one for fully linear skbs (nfrags = 1) and one for
> > skbs carrying paged fragments without a frag_list, which sets
> > nfrags = skb_shinfo(skb)->nr_frags + 1.  In both arms the skb is mapped
> > into a scatterlist via skb_to_sgvec() and passed as both src and dst of
> > aead_request_set_crypt(), so the AEAD operates in place over the
> > existing frag pages.
> > 
> > Drop the paged-fragment arm so any non-linear inbound skb falls through
> > to skb_cow_data().  The fully linear fast path is unchanged and the
> > existing skb_cow_data() error handling that follows the gate is reused.
> > 
> > Fixes: cac2661c53f3 ("esp4: Avoid skb_cow_data whenever possible")
> > Fixes: 03e2a30f6a27 ("esp6: Avoid skb_cow_data whenever possible")
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > ---
> >  net/ipv4/esp4.c | 13 +++----------
> >  net/ipv6/esp6.c | 13 +++----------
> >  2 files changed, 6 insertions(+), 20 deletions(-)
> 
> Good catch!
> 
> When a packet comes from a device driver, it's usually safe to
> write to the fragments since they would have been allocated by
> the driver.
> 
> But when a packet originates from our own stack, then it's not
> safe to write to the fragments.
> 
> Unfortunately the two paths cross with the loopback driver (and
> probably other means of creating loopback).
> 
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thank you for the review.

If it's not too much trouble, I would be grateful if you could also 
take a look at this rxrpc patch, which addresses a similar bug: 
https://lore.kernel.org/all/afKV2zGR6rrelPC7@v4bel/


Best regards,
Hyunwoo Kim

> 
> Thanks,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH ipsec] esp: Force skb_cow_data() on RX when the skb is non-linear

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
esp: Force skb_cow_data() on RX when the skb is non-linear

This commit modifies esp_input() and esp6_input() to unconditionally call
skb_cow_data() for all non-linear skbs. This removes the fast path that
previously allowed in-place AEAD decryption for skbs carrying paged
fragments without a frag_list.

> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index 6dfc0bcdef65..a6fbdec139dc 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -868,17 +868,10 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
>  		assoclen += seqhilen;
>  	}
>  
> -	if (!skb_cloned(skb)) {
> -		if (!skb_is_nonlinear(skb)) {
> -			nfrags = 1;
> -
> -			goto skip_cow;
> -		} else if (!skb_has_frag_list(skb)) {
> -			nfrags = skb_shinfo(skb)->nr_frags;
> -			nfrags++;
> +	if (!skb_cloned(skb) && !skb_is_nonlinear(skb)) {
> +		nfrags = 1;
>  
> -			goto skip_cow;
> -		}
> +		goto skip_cow;
>  	}

By removing this fast path and forcing all non-linear skbs to fall through
to skb_cow_data(), does this break the IPsec GRO/GSO fast path?

Looking at skb_cow_data() in net/core/skbuff.c:

int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer)
{
	...
	if ((skb_cloned(skb) || skb_shinfo(skb)->nr_frags) &&
	    !__pskb_pull_tail(skb, __skb_pagelen(skb)))
		return -ENOMEM;
	...

For a large GRO or GSO packet (which can be up to 64KB) with paged fragments,
this attempts to pull the entire paged payload into the linear data area
via __pskb_pull_tail().

Since this runs in the RX softirq path, pulling the entire payload requires
a high-order contiguous memory allocation using GFP_ATOMIC. 

Could this lead to widespread packet drops when memory is fragmented and
these high-order atomic allocations fail?

Additionally, even when the allocations succeed, does unconditionally copying
up to 64KB of data per packet introduce a severe performance regression by
entirely negating the software crypto fast path?

[ ... ]