All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xsk: fix memory corruptions in net/core/xdp.c
@ 2026-06-24  8:41 Clement Lecigne
  2026-06-24  8:53 ` sashiko-bot
  2026-06-25 15:14 ` Alexander Lobakin
  0 siblings, 2 replies; 6+ messages in thread
From: Clement Lecigne @ 2026-06-24  8:41 UTC (permalink / raw)
  To: aleksander.lobakin, edumazet, netdev
  Cc: clecigne, bpf, linux-kernel, kuba, sdf, horms, john.fastabend,
	ast, daniel

From: Clément Lecigne <clecigne@google.com>

Commit 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
introduced a vulnerability in the handling of XDP_PASS for AF_XDP zero-copy
frames.

Note: Currently, this specific AF_XDP zero-copy conversion path is only
reachable from the drivers/net/ethernet/intel/ice driver.

When building an skb, xdp_build_skb_from_zc() uses the chunk size
(xdp->frame_sz) for the allocation. However, napi_build_skb() automatically
reserves space at the end of the allocation for the skb_shared_info
structure. 

Most high performance UMEM applications use 4K chunks, where the
corruption cannot happen. However, if the UMEM is configured with 2KB
chunks (a very common configuration to maximize packet density in memory),
a standard 1500 MTU packet will trigger the corruption because the required
space exceeds the 2048 byte chunk size:

Headroom (256) + Packet (1514) + skb_shared_info (320) = 2090 bytes

Because 2090 bytes > 2048 bytes and __skb_put() does not perform bounds
checking, the memcpy() writes past the available linear data area and
corrupts the skb_shared_info structure. This can lead to arbitrary code
execution if pointers like destructor_arg are overwritten.

Additionally, in xdp_copy_frags_from_zc(), the allocation size is set
strictly to the fragment size (len), but the subsequent memcpy() uses
LARGEST_ALIGN(len). This mismatch results in an out-of-bounds write of
up to 7 bytes, which triggers KASAN warnings and is unsafe despite typical
page pool allocator padding.

Fix the skb allocation in xdp_build_skb_from_zc() by dynamically
calculating the exact truesize required: the sum of the headroom, the
packet length, and the skb_shared_info overhead, properly aligned via
SKB_DATA_ALIGN.

Fix the out-of-bounds write in xdp_copy_frags_from_zc() by rounding up
the allocation request using LARGEST_ALIGN(len) to match the copy
operation.

Fixes: 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
CC: Alexander Lobakin <aleksander.lobakin@intel.com>
CC: Eric Dumazet <edumazet@google.com>
Signed-off-by: Clément Lecigne <clecigne@google.com>
---
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 9890a30584ba..f36d1fb875ab 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -699,7 +699,7 @@ static noinline bool xdp_copy_frags_from_zc(struct sk_buff *skb,
 	for (u32 i = 0; i < nr_frags; i++) {
 		const skb_frag_t *frag = &xinfo->frags[i];
 		u32 len = skb_frag_size(frag);
-		u32 offset, truesize = len;
+		u32 offset, truesize = LARGEST_ALIGN(len);
 		struct page *page;
 
 		page = page_pool_dev_alloc(pp, &offset, &truesize);
@@ -740,7 +740,9 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
 {
 	const struct xdp_rxq_info *rxq = xdp->rxq;
 	u32 len = xdp->data_end - xdp->data_meta;
-	u32 truesize = xdp->frame_sz;
+	u32 headroom = xdp->data_meta - xdp->data_hard_start;
+	u32 truesize = SKB_DATA_ALIGN(headroom + len) +
+		       SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 	struct sk_buff *skb = NULL;
 	struct page_pool *pp;
 	int metalen;
@@ -762,7 +764,7 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
 	}
 
 	skb_mark_for_recycle(skb);
-	skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
+	skb_reserve(skb, headroom);
 
 	memcpy(__skb_put(skb, len), xdp->data_meta, LARGEST_ALIGN(len));
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xsk: fix memory corruptions in net/core/xdp.c
  2026-06-24  8:41 [PATCH] xsk: fix memory corruptions in net/core/xdp.c Clement Lecigne
@ 2026-06-24  8:53 ` sashiko-bot
  2026-06-25 15:14 ` Alexander Lobakin
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-06-24  8:53 UTC (permalink / raw)
  To: Clement Lecigne; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The new dynamic `truesize` calculation tightly bounds the skb linear buffer size but fails to account for the `LARGEST_ALIGN(len)` padding used in the subsequent `memcpy()`. This leads to an out-of-bounds write that corrupts the `skb_shared_info` structure.
--

commit f8cfaa75cd9fcfb65c25981d740b6eedecb19f7b
Author: Clément Lecigne <clecigne@google.com>

xsk: fix memory corruptions in net/core/xdp.c

This commit attempts to fix a memory corruption in AF_XDP zero-copy handling
by dynamically calculating the truesize for skb allocation. It also fixes a
mismatch in xdp_copy_frags_from_zc() between allocation and copy sizes.

> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 9890a30584ba7..f36d1fb875ab8 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
[ ... ]
> @@ -740,7 +740,9 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
>  {
>  	const struct xdp_rxq_info *rxq = xdp->rxq;
>  	u32 len = xdp->data_end - xdp->data_meta;
> -	u32 truesize = xdp->frame_sz;
> +	u32 headroom = xdp->data_meta - xdp->data_hard_start;
> +	u32 truesize = SKB_DATA_ALIGN(headroom + len) +
> +		       SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

[Severity: Critical]
Does this tight allocation size account for the LARGEST_ALIGN(len) padding
used during the memcpy() below?

If an XDP program modifies the packet data making headroom unaligned, and 
headroom + len is close to a multiple of SMP_CACHE_BYTES, napi_build_skb()
will place the skb_shared_info structure exactly at 
SKB_DATA_ALIGN(headroom + len).

Could the LARGEST_ALIGN(len) size in the memcpy() below write past this
linear data area and corrupt the adjacent skb_shared_info structure?

>  	struct sk_buff *skb = NULL;
>  	struct page_pool *pp;
>  	int metalen;
[ ... ]
> @@ -762,7 +764,7 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
>  	}
>  
>  	skb_mark_for_recycle(skb);
> -	skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
> +	skb_reserve(skb, headroom);
>  
>  	memcpy(__skb_put(skb, len), xdp->data_meta, LARGEST_ALIGN(len));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624084130.2382335-1-clecigne@google.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xsk: fix memory corruptions in net/core/xdp.c
  2026-06-24  8:41 [PATCH] xsk: fix memory corruptions in net/core/xdp.c Clement Lecigne
  2026-06-24  8:53 ` sashiko-bot
@ 2026-06-25 15:14 ` Alexander Lobakin
  2026-06-26  8:52   ` [PATCH v2] " Clement Lecigne
  2026-06-26  9:12   ` [PATCH] " Clement Lecigne
  1 sibling, 2 replies; 6+ messages in thread
From: Alexander Lobakin @ 2026-06-25 15:14 UTC (permalink / raw)
  To: Clement Lecigne
  Cc: edumazet, netdev, bpf, linux-kernel, kuba, sdf, horms,
	john.fastabend, ast, daniel

From: Clement Lecigne <clecigne@google.com>
Date: Wed, 24 Jun 2026 08:41:28 +0000

> From: Clément Lecigne <clecigne@google.com>
> 
> Commit 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
> introduced a vulnerability in the handling of XDP_PASS for AF_XDP zero-copy
> frames.
> 
> Note: Currently, this specific AF_XDP zero-copy conversion path is only
> reachable from the drivers/net/ethernet/intel/ice driver.

idpf uses this, too (every driver based on libeth_xdp in general,
currently these two).

> 
> When building an skb, xdp_build_skb_from_zc() uses the chunk size
> (xdp->frame_sz) for the allocation. However, napi_build_skb() automatically
> reserves space at the end of the allocation for the skb_shared_info
> structure. 
> 
> Most high performance UMEM applications use 4K chunks, where the
> corruption cannot happen. However, if the UMEM is configured with 2KB
> chunks (a very common configuration to maximize packet density in memory),
> a standard 1500 MTU packet will trigger the corruption because the required
> space exceeds the 2048 byte chunk size:
> 
> Headroom (256) + Packet (1514) + skb_shared_info (320) = 2090 bytes
> 
> Because 2090 bytes > 2048 bytes and __skb_put() does not perform bounds
> checking, the memcpy() writes past the available linear data area and
> corrupts the skb_shared_info structure. This can lead to arbitrary code
> execution if pointers like destructor_arg are overwritten.
> 
> Additionally, in xdp_copy_frags_from_zc(), the allocation size is set
> strictly to the fragment size (len), but the subsequent memcpy() uses
> LARGEST_ALIGN(len). This mismatch results in an out-of-bounds write of
> up to 7 bytes, which triggers KASAN warnings and is unsafe despite typical
> page pool allocator padding.
> 
> Fix the skb allocation in xdp_build_skb_from_zc() by dynamically
> calculating the exact truesize required: the sum of the headroom, the
> packet length, and the skb_shared_info overhead, properly aligned via
> SKB_DATA_ALIGN.
> 
> Fix the out-of-bounds write in xdp_copy_frags_from_zc() by rounding up
> the allocation request using LARGEST_ALIGN(len) to match the copy
> operation.
> 
> Fixes: 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
> CC: Alexander Lobakin <aleksander.lobakin@intel.com>
> CC: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Clément Lecigne <clecigne@google.com>
> ---
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 9890a30584ba..f36d1fb875ab 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -699,7 +699,7 @@ static noinline bool xdp_copy_frags_from_zc(struct sk_buff *skb,
>  	for (u32 i = 0; i < nr_frags; i++) {
>  		const skb_frag_t *frag = &xinfo->frags[i];
>  		u32 len = skb_frag_size(frag);
> -		u32 offset, truesize = len;
> +		u32 offset, truesize = LARGEST_ALIGN(len);

I think you need to re-sort this to keep RCT, now that the truesize
initialization is way longer than it was.

		const skb_frag_t *frag = &xinfo->frags[i];
		u32 offset, len = skb_frag_size(frag);
		u32 truesize = LARGEST_ALIGN(len);
		struct page *page;

>  		struct page *page;
>  
>  		page = page_pool_dev_alloc(pp, &offset, &truesize);

BTW usually LARGEST_ALIGN() aligns to 16, I've never seen a bigger one.
IIRC Page Pool never returns a truesize aligned to a smaller value. But
if you're really able to trigger this, it probably does?

> @@ -740,7 +740,9 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
>  {
>  	const struct xdp_rxq_info *rxq = xdp->rxq;
>  	u32 len = xdp->data_end - xdp->data_meta;
> -	u32 truesize = xdp->frame_sz;
> +	u32 headroom = xdp->data_meta - xdp->data_hard_start;
> +	u32 truesize = SKB_DATA_ALIGN(headroom + len) +
> +		       SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

Ah now I get it: xdp->frame_sz doesn't account the shinfo for
single-buffer frames, only for multi-buffer ones. The fix looks correct,
but I'd use SKB_HEAD_ALIGN() since it does exactly what you're
open-coding here and sort the declarations:

{
	u32 hr = xdp->data_meta - xdp->data_hard_start;
	const struct xdp_rxq_info *rxq = xdp->rxq;
	u32 len = xdp->data_end - xdp->data_meta;
	u32 truesize = SKB_HEAD_ALIGN(hr + len);
	struct sk_buff *skb = NULL;
	struct page_pool *pp;
	int metalen;
	void *data;

	if (!IS_ENABLED(CONFIG_PAGE_POOL))
		return NULL;

	...

>  	struct sk_buff *skb = NULL;
>  	struct page_pool *pp;
>  	int metalen;
> @@ -762,7 +764,7 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
>  	}
>  
>  	skb_mark_for_recycle(skb);
> -	skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
> +	skb_reserve(skb, headroom);
>  
>  	memcpy(__skb_put(skb, len), xdp->data_meta, LARGEST_ALIGN(len));

Thanks,
Olek

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] xsk: fix memory corruptions in net/core/xdp.c
  2026-06-25 15:14 ` Alexander Lobakin
@ 2026-06-26  8:52   ` Clement Lecigne
  2026-06-26  9:08     ` sashiko-bot
  2026-06-26  9:12   ` [PATCH] " Clement Lecigne
  1 sibling, 1 reply; 6+ messages in thread
From: Clement Lecigne @ 2026-06-26  8:52 UTC (permalink / raw)
  To: aleksander.lobakin, edumazet, netdev
  Cc: clecigne, bpf, linux-kernel, kuba, sdf, horms, john.fastabend,
	ast, daniel

From: Clément Lecigne <clecigne@google.com>

Commit 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
introduced a vulnerability in the handling of XDP_PASS for AF_XDP zero-copy
frames.

Note: Currently, this specific AF_XDP zero-copy conversion path is only
reachable from the drivers/net/ethernet/intel/ice and
drivers/net/ethernet/intel/idpf drivers.

When building an skb, xdp_build_skb_from_zc() uses the chunk size
(xdp->frame_sz) for the allocation. However, napi_build_skb() automatically
reserves space at the end of the allocation for the skb_shared_info
structure. 

Most high performance UMEM applications use 4K chunks, where the
corruption cannot happen. However, if the UMEM is configured with 2KB
chunks (a very common configuration to maximize packet density in memory),
a standard 1500 MTU packet will trigger the corruption because the required
space exceeds the 2048 byte chunk size:

Headroom (256) + Packet (1514) + skb_shared_info (320) = 2090 bytes

Because 2090 bytes > 2048 bytes and __skb_put() does not perform bounds
checking, the memcpy() writes past the available linear data area and
corrupts the skb_shared_info structure. This can lead to arbitrary code
execution if pointers like destructor_arg are overwritten.

Additionally, in xdp_copy_frags_from_zc(), the allocation size is set
strictly to the fragment size (len), but the subsequent memcpy() uses
LARGEST_ALIGN(len). This mismatch results in an out-of-bounds write of
up to 7 bytes, which triggers KASAN warnings and is unsafe despite typical
page pool allocator padding.

Fix the skb allocation in xdp_build_skb_from_zc() by dynamically
calculating the exact truesize required using SKB_HEAD_ALIGN() to
properly account for the headroom, the packet length, and the
skb_shared_info overhead.

Fix the out-of-bounds write in xdp_copy_frags_from_zc() by rounding up
the allocation request using LARGEST_ALIGN(len) to match the copy
operation.

Fixes: 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
CC: Alexander Lobakin <aleksander.lobakin@intel.com>
CC: Eric Dumazet <edumazet@google.com>
Signed-off-by: Clément Lecigne <clecigne@google.com>
---
Changes since v1:
 - Used SKB_HEAD_ALIGN to properly calculate the required allocation size
   including the skb_shared_info overhead.
 - Re-ordered variable declarations.

---
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 9890a30584ba..52546746378a 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -698,8 +698,8 @@ static noinline bool xdp_copy_frags_from_zc(struct sk_buff *skb,
 
 	for (u32 i = 0; i < nr_frags; i++) {
 		const skb_frag_t *frag = &xinfo->frags[i];
-		u32 len = skb_frag_size(frag);
-		u32 offset, truesize = len;
+		u32 offset, len = skb_frag_size(frag);
+		u32 truesize = LARGEST_ALIGN(len);
 		struct page *page;
 
 		page = page_pool_dev_alloc(pp, &offset, &truesize);
@@ -738,9 +738,10 @@ static noinline bool xdp_copy_frags_from_zc(struct sk_buff *skb,
  */
 struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
 {
+	u32 headroom = xdp->data_meta - xdp->data_hard_start;
 	const struct xdp_rxq_info *rxq = xdp->rxq;
 	u32 len = xdp->data_end - xdp->data_meta;
-	u32 truesize = xdp->frame_sz;
+	u32 truesize = SKB_HEAD_ALIGN(headroom + len);
 	struct sk_buff *skb = NULL;
 	struct page_pool *pp;
 	int metalen;
@@ -762,7 +763,7 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
 	}
 
 	skb_mark_for_recycle(skb);
-	skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
+	skb_reserve(skb, headroom);
 
 	memcpy(__skb_put(skb, len), xdp->data_meta, LARGEST_ALIGN(len));
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] xsk: fix memory corruptions in net/core/xdp.c
  2026-06-26  8:52   ` [PATCH v2] " Clement Lecigne
@ 2026-06-26  9:08     ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-06-26  9:08 UTC (permalink / raw)
  To: Clement Lecigne; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The exact dynamic allocation sizing in xdp_build_skb_from_zc() combined with a rounded-up memcpy() introduces a reliable out-of-bounds write into skb_shared_info.
--

commit 332575c8834e99019b6ead4fd6570c4662e36762
Author: Clément Lecigne <clecigne@google.com>

xsk: fix memory corruptions in net/core/xdp.c

This commit attempts to fix out of bounds writes during AF_XDP zero-copy
frame conversions. It introduces dynamic truesize calculations using
SKB_HEAD_ALIGN for the skb allocation and LARGEST_ALIGN for fragment
allocations to properly account for required headroom and packet length.

> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 9890a30584ba7..52546746378a0 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
[ ... ]
> @@ -738,9 +738,10 @@ static noinline bool xdp_copy_frags_from_zc(struct sk_buff *skb,
>   */
>  struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
>  {
> +	u32 headroom = xdp->data_meta - xdp->data_hard_start;
>  	const struct xdp_rxq_info *rxq = xdp->rxq;
>  	u32 len = xdp->data_end - xdp->data_meta;
> -	u32 truesize = xdp->frame_sz;
> +	u32 truesize = SKB_HEAD_ALIGN(headroom + len);

[Severity: Critical]
Does this regression allocate enough linear space to handle a rounded-up
memcpy when the headroom is unaligned?

If an XDP program adjusts data_meta such that headroom is not a multiple of
64 (for example, 252 bytes), and a packet of 68 bytes is received, the
allocated linear size from SKB_HEAD_ALIGN(320) would be 320.

However, the later memcpy uses LARGEST_ALIGN(len) for the 68 bytes, which
could round up to 96 bytes.  This means 252 + 96 = 348 bytes could be
written into a 320 byte space.

[ ... ]
> @@ -762,7 +763,7 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
>  	}
>  
>  	skb_mark_for_recycle(skb);
> -	skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
> +	skb_reserve(skb, headroom);
>  
>  	memcpy(__skb_put(skb, len), xdp->data_meta, LARGEST_ALIGN(len));

[Severity: Critical]
Can this regression corrupt skb_shared_info?

Since the linear space allocation might only be 320 bytes as shown above,
but this memcpy could write up to 348 bytes, it appears this might write
past the end of the linear data region.

Could this lead to overwriting destructor_arg and cause issues upon SKB
free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626085226.1765180-1-clecigne@google.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xsk: fix memory corruptions in net/core/xdp.c
  2026-06-25 15:14 ` Alexander Lobakin
  2026-06-26  8:52   ` [PATCH v2] " Clement Lecigne
@ 2026-06-26  9:12   ` Clement Lecigne
  1 sibling, 0 replies; 6+ messages in thread
From: Clement Lecigne @ 2026-06-26  9:12 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: edumazet, netdev, bpf, linux-kernel, kuba, sdf, horms,
	john.fastabend, ast, daniel

[-- Attachment #1: Type: text/plain, Size: 5631 bytes --]

Thanks, Olek!

I submitted a v2 of the patch directly as a reply to this thread by
mistake, do you still want me to post this new version separately?

On Thu, Jun 25, 2026 at 5:14 PM Alexander Lobakin
<aleksander.lobakin@intel.com> wrote:
>
> From: Clement Lecigne <clecigne@google.com>
> Date: Wed, 24 Jun 2026 08:41:28 +0000
>
> > From: Clément Lecigne <clecigne@google.com>
> >
> > Commit 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
> > introduced a vulnerability in the handling of XDP_PASS for AF_XDP zero-copy
> > frames.
> >
> > Note: Currently, this specific AF_XDP zero-copy conversion path is only
> > reachable from the drivers/net/ethernet/intel/ice driver.
>
> idpf uses this, too (every driver based on libeth_xdp in general,
> currently these two).

Done.

>
> >
> > When building an skb, xdp_build_skb_from_zc() uses the chunk size
> > (xdp->frame_sz) for the allocation. However, napi_build_skb() automatically
> > reserves space at the end of the allocation for the skb_shared_info
> > structure.
> >
> > Most high performance UMEM applications use 4K chunks, where the
> > corruption cannot happen. However, if the UMEM is configured with 2KB
> > chunks (a very common configuration to maximize packet density in memory),
> > a standard 1500 MTU packet will trigger the corruption because the required
> > space exceeds the 2048 byte chunk size:
> >
> > Headroom (256) + Packet (1514) + skb_shared_info (320) = 2090 bytes
> >
> > Because 2090 bytes > 2048 bytes and __skb_put() does not perform bounds
> > checking, the memcpy() writes past the available linear data area and
> > corrupts the skb_shared_info structure. This can lead to arbitrary code
> > execution if pointers like destructor_arg are overwritten.
> >
> > Additionally, in xdp_copy_frags_from_zc(), the allocation size is set
> > strictly to the fragment size (len), but the subsequent memcpy() uses
> > LARGEST_ALIGN(len). This mismatch results in an out-of-bounds write of
> > up to 7 bytes, which triggers KASAN warnings and is unsafe despite typical
> > page pool allocator padding.
> >
> > Fix the skb allocation in xdp_build_skb_from_zc() by dynamically
> > calculating the exact truesize required: the sum of the headroom, the
> > packet length, and the skb_shared_info overhead, properly aligned via
> > SKB_DATA_ALIGN.
> >
> > Fix the out-of-bounds write in xdp_copy_frags_from_zc() by rounding up
> > the allocation request using LARGEST_ALIGN(len) to match the copy
> > operation.
> >
> > Fixes: 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
> > CC: Alexander Lobakin <aleksander.lobakin@intel.com>
> > CC: Eric Dumazet <edumazet@google.com>
> > Signed-off-by: Clément Lecigne <clecigne@google.com>
> > ---
> > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > index 9890a30584ba..f36d1fb875ab 100644
> > --- a/net/core/xdp.c
> > +++ b/net/core/xdp.c
> > @@ -699,7 +699,7 @@ static noinline bool xdp_copy_frags_from_zc(struct sk_buff *skb,
> >       for (u32 i = 0; i < nr_frags; i++) {
> >               const skb_frag_t *frag = &xinfo->frags[i];
> >               u32 len = skb_frag_size(frag);
> > -             u32 offset, truesize = len;
> > +             u32 offset, truesize = LARGEST_ALIGN(len);
>
> I think you need to re-sort this to keep RCT, now that the truesize
> initialization is way longer than it was.

Done.

>
>                 const skb_frag_t *frag = &xinfo->frags[i];
>                 u32 offset, len = skb_frag_size(frag);
>                 u32 truesize = LARGEST_ALIGN(len);
>                 struct page *page;
>
> >               struct page *page;
> >
> >               page = page_pool_dev_alloc(pp, &offset, &truesize);
>
> BTW usually LARGEST_ALIGN() aligns to 16, I've never seen a bigger one.
> IIRC Page Pool never returns a truesize aligned to a smaller value. But
> if you're really able to trigger this, it probably does?
>
> > @@ -740,7 +740,9 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
> >  {
> >       const struct xdp_rxq_info *rxq = xdp->rxq;
> >       u32 len = xdp->data_end - xdp->data_meta;
> > -     u32 truesize = xdp->frame_sz;
> > +     u32 headroom = xdp->data_meta - xdp->data_hard_start;
> > +     u32 truesize = SKB_DATA_ALIGN(headroom + len) +
> > +                    SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>
> Ah now I get it: xdp->frame_sz doesn't account the shinfo for
> single-buffer frames, only for multi-buffer ones. The fix looks correct,
> but I'd use SKB_HEAD_ALIGN() since it does exactly what you're
> open-coding here and sort the declarations:

Good idea, done.


>
> {
>         u32 hr = xdp->data_meta - xdp->data_hard_start;
>         const struct xdp_rxq_info *rxq = xdp->rxq;
>         u32 len = xdp->data_end - xdp->data_meta;
>         u32 truesize = SKB_HEAD_ALIGN(hr + len);
>         struct sk_buff *skb = NULL;
>         struct page_pool *pp;
>         int metalen;
>         void *data;
>
>         if (!IS_ENABLED(CONFIG_PAGE_POOL))
>                 return NULL;
>
>         ...
>
> >       struct sk_buff *skb = NULL;
> >       struct page_pool *pp;
> >       int metalen;
> > @@ -762,7 +764,7 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
> >       }
> >
> >       skb_mark_for_recycle(skb);
> > -     skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
> > +     skb_reserve(skb, headroom);
> >
> >       memcpy(__skb_put(skb, len), xdp->data_meta, LARGEST_ALIGN(len));
>
> Thanks,
> Olek

Thanks,
-clem

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-26  9:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-24  8:41 [PATCH] xsk: fix memory corruptions in net/core/xdp.c Clement Lecigne
2026-06-24  8:53 ` sashiko-bot
2026-06-25 15:14 ` Alexander Lobakin
2026-06-26  8:52   ` [PATCH v2] " Clement Lecigne
2026-06-26  9:08     ` sashiko-bot
2026-06-26  9:12   ` [PATCH] " Clement Lecigne

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.