[PATCH] net: skbuff: keep the page_pool fragment offset aligned in skb_pp_cow_data()

[PATCH] net: skbuff: keep the page_pool fragment offset aligned in skb_pp_cow_data()

[Bruno Xavier]

skb_pp_cow_data() takes two kinds of memory from the same page_pool. The
head buffer goes to napi_build_skb() and becomes skb->head, so
skb_shinfo() lands at skb->head + skb->end and has to be aligned. The
payload fragments in the loop below are plain data, and they are requested
with the raw remaining packet length.

page_pool_alloc_frag_netmem() advances pool->frag_offset by

	ALIGN(size, dma_get_cache_alignment())

and dma_get_cache_alignment() returns 1 wherever ARCH_HAS_DMA_MINALIGN is
undefined, x86 included. An unaligned fragment request therefore leaves
pool->frag_offset unaligned, and every later head allocation from that
pool comes back unaligned. skb->head is then misaligned and
refcount_inc(&skb_shinfo(skb)->dataref) in skb_clone() is an unaligned
lock incl. Where that 4-byte access crosses a cache line it is a split
lock, and on a CPU with split lock detection the kernel dies in softirq:

  Kernel panic - not syncing: Fatal exception in interrupt
  RIP: 0010:skb_clone+0x159/0x1e0

This hit four times in three days on a Core Ultra 7 258V running a generic
XDP program on lo with a raw IPv4 socket open, so raw_v4_input() cloned
every matching skb. Observed skb->head misalignments were 5, 6, 10, 11 and
13 bytes. Without split lock detection the unaligned atomic just runs, a
few microseconds each time, and nothing is logged.

Align the fragment request so the pool's fragment offset stays usable for
the head allocations this function also makes.

Fixes: e6d5dbdd20aa ("xdp: add multi-buff support for xdp running in generic mode")
Signed-off-by: Bruno Xavier <bfxavier@gmail.com>
---

Notes:
    Tested on a Core Ultra 7 258V, Fedora 44 userspace, with netbird attaching a
    generic XDP program to lo and holding a raw IPv4 socket, so raw_v4_input()
    clones matching skbs. A bpftrace kprobe on napi_build_skb() and
    __build_skb_around() counting misaligned data pointers, plus a kprobe on
    skb_pp_cow_data() as a positive control:
    
      unpatched 7.1.9, 7m22s : 16884 calls, 72 misaligned builds, 10 misaligned clones
      patched   7.2.0, 7m    : 30479 calls,  0 misaligned builds,  0 misaligned clones
    
    Misalignments seen before the patch were 1, 2, 3, 5, 6, 9, 10, 11, 12 and 14
    bytes, so not even 2-byte alignment held. A kretprobe on
    page_pool_alloc_frag_netmem() watching pool->frag_offset independently showed
    zero misaligned offsets after the patch.
    
    The alternative is to have the page_pool frag allocator guarantee a minimum
    alignment itself:
    
    	-	size = ALIGN(size, dma_get_cache_alignment());
    	+	size = ALIGN(size, max_t(unsigned int, dma_get_cache_alignment(),
    	+				 __alignof__(long)));
    
    That covers every caller of page_pool_dev_alloc*() instead of just this one,
    but it changes a generic allocator and would carry a 2021 Fixes tag, so I kept
    the fix in the caller. Happy to send that version instead if you prefer it.

 net/core/skbuff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index cbbd60455abb..7f8953b48026 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -985,7 +985,7 @@ int skb_pp_cow_data(struct page_pool *pool, struct sk_buff **pskb,
 		u32 page_off;
 
 		size = min_t(u32, len, PAGE_SIZE);
-		truesize = size;
+		truesize = ALIGN(size, sizeof(long));
 
 		page = page_pool_dev_alloc(pool, &page_off, &truesize);
 		if (!page) {
-- 
2.55.0

Re: [PATCH] net: skbuff: keep the page_pool fragment offset aligned in skb_pp_cow_data()

[Fabricio Gava]

On Thu, Aug 27, 2026 at 02:29:17PM +0200, Bruno Xavier wrote:
>     The alternative is to have the page_pool frag allocator guarantee a
>     minimum alignment itself:
> [...]
>     That covers every caller of page_pool_dev_alloc*() instead of just
>     this one, but it changes a generic allocator and would carry a 2021
>     Fixes tag, so I kept the fix in the caller. Happy to send that version
>     instead if you prefer it.

That version has already been sent, by Florian Schauer, the day before
this patch -- "page_pool: keep frag_offset aligned for odd-sized requests",
now at v2:

https://lore.kernel.org/netdev/20260828060822.2628276-1-florian@schauer.to/

It is the same change you sketch, already at its second revision: the v2
changelog credits Eric Dumazet for __alignof__(struct skb_shared_info) in
place of sizeof(long).  Neither thread references the other and both are
still in state "new", so I am replying to both to connect them.

One thing that may bear on the choice between the two: skb_pp_cow_data() is
not the only user of the per-cpu system_page_pool that requests a raw
length.
xdp_copy_frags_from_zc() does the same, net/core/xdp.c:700-705:

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

	page = page_pool_dev_alloc(pp, &offset, &truesize);

Its caller xdp_build_skb_from_zc() takes pp from this_cpu_read(
system_page_pool.pool) at xdp.c:753, and then feeds
page_pool_dev_alloc_va() at xdp.c:754 into napi_build_skb() at xdp.c:758 --
so that path both leaves odd frag_offsets behind and consumes the head
allocations that follow them, on the very pool your patch is protecting.
With the fix in the caller, that door stays open.

I also measured the call-site attribution here.  Over ~75 s of that
workload and some 8.6 million fragment requests reaching
page_pool_alloc_frag_netmem(), the probe saw none originating outside
skb_pp_cow_data() -- but that machine drives neither the zero-copy path nor
a page_pool-backed NIC driver, so no other producer was exercised.  So the
measurement says your patch would be sufficient for that workload.  It does
not show that no other producer exists, and xdp_copy_frags_from_zc() draws
from the same per-cpu pool your patch protects.

One more thing about the scope your Fixes: tag implies.  skb_pp_cow_data()
is also used by veth, at drivers/net/veth.c:762, so the fragment loop is
reachable outside the generic-XDP path that e6d5dbdd20aa added.  That does
not make the tag wrong for where the code came from, but the blast radius
is wider than "xdp running in generic mode" reads.

For what it is worth, I hit exactly your symptom independently, on
different hardware: Fedora 44 with kernels 7.1.8 / 7.1.9 / 7.1.10 on an
i5-13420H, NetBird attaching a generic XDP program to lo, eight panics at
skb_clone+0x159 -- six from raw_v4_input(), two from ipv6_raw_deliver().
Measurements of the three links in the chain are in my reply on Florian's
v2 thread rather than repeated here.

Thanks,
Fabricio Gava

Re: [PATCH] net: skbuff: keep the page_pool fragment offset aligned in skb_pp_cow_data()

[Bruno Xavier]

On Sun, Aug 30, 2026 at 09:23:27PM +0000, Fabricio Gava wrote:
> That version has already been sent, by Florian Schauer, the day before
> this patch -- "page_pool: keep frag_offset aligned for odd-sized requests",
> now at v2:

Thanks for linking the threads, I had not seen Florian's v1.

> xdp_copy_frags_from_zc() does the same, net/core/xdp.c:700-705:

I mispoke in my own notes on this one. I checked system_page_pool for
other consumers before sending and read it as having exactly one,
net/core/dev.c:5619. xdp.c:753 is the second. So fixing skb_pp_cow_data()
alone leaves the defect reachable through the zero-copy path.

Florian's patch is the right fix. Please drop this one.

On veth, drivers/net/veth.c:762 passes its own rq->page_pool rather than
the per-cpu one, so I don't think it widens the sharing. It widens where
the misaligning loop runs, which is the part your reading of the Fixes tag
gets at.

I have the reproducer and an A/B kernel harness here. Building v2 now,
numbers on Florian's thread.

Thanks,
Bruno