[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