[PATCH net-next v1 4/6] net: skbuff: replace __skb_frag_ref() with symmetric skb_netmem_ref()
From: Mina Almasry
Date: Sat Oct 10 2026 - 04:47:17 EST
Previously, __skb_frag_ref() and skb_frag_ref() unconditionally
incremented the non-pp backing refcount via get_netmem(), while
skb_frag_unref() decremented pp_ref_count whenever skb->pp_recycle was
set. Ref-ing a page_pool fragment into a pp_recycle=1 SKB (or clearing
pp_recycle=0 on an uncloned SKB while clones still held pp_recycle=1)
leaked the backing non-pp refcount and underflowed pp_ref_count across
IP-TFS, skb_split(), skb_shift(), and cloned unclone paths.
Replace __skb_frag_ref() with skb_netmem_ref(netmem, recycle) as the
exact counterpart to skb_netmem_unref(netmem, recycle):
- Keep skb->pp_recycle intact in skb_release_data() so uncloning paths
(pskb_expand_head() and pskb_carve_inside_{header,nonlinear}()) keep
trading pp_ref_count references symmetrically across clones.
- Propagate pp_recycle and call skb_frag_ref() on the destination SKB in
__pskb_copy_fclone(), skb_zerocopy(), skb_split(), skb_shift(), and
skb_segment().
- Use skb_frag_ref() in xfrm_iptfs, chcr_ktls, and cassini, and call
get_netmem() directly in veth and tls_device_fallback where callers
explicitly take a raw non-pp reference.
Cc: Stanislav Fomichev <sdf@xxxxxxxxxxx>
Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
Cc: Jesper Dangaard Brouer <hawk@xxxxxxxxxx>
Cc: bpf@xxxxxxxxxxxxxxx
Signed-off-by: Mina Almasry <almasrymina@xxxxxxxxxx>
---
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 2 +-
drivers/net/ethernet/sun/cassini.c | 4 +-
drivers/net/veth.c | 2 +-
include/linux/skbuff_ref.h | 28 +++++++++-----
net/core/skbuff.c | 38 ++++++++-----------
net/tls/tls_device_fallback.c | 2 +-
net/xfrm/xfrm_iptfs.c | 3 +-
7 files changed, 41 insertions(+), 38 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index dbec95b497773..4d645771596a8 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -1660,7 +1660,7 @@ static void chcr_ktls_copy_record_in_skb(struct sk_buff *nskb,
for (i = 0; i < record->num_frags; i++) {
skb_shinfo(nskb)->frags[i] = record->frags[i];
/* increase the frag ref count */
- __skb_frag_ref(&skb_shinfo(nskb)->frags[i]);
+ skb_frag_ref(nskb, i);
}
skb_shinfo(nskb)->nr_frags = record->num_frags;
diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index 9a8b5cf2d6485..02158a1bb2f70 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -1998,7 +1998,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
skb->len += hlen - swivel;
skb_frag_fill_page_desc(frag, page->buffer, off, hlen - swivel);
- __skb_frag_ref(frag);
+ skb_frag_ref(skb, 0);
/* any more data? */
if ((words[0] & RX_COMP1_SPLIT_PKT) && ((dlen -= hlen) > 0)) {
@@ -2022,7 +2022,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
frag++;
skb_frag_fill_page_desc(frag, page->buffer, 0, hlen);
- __skb_frag_ref(frag);
+ skb_frag_ref(skb, 1);
RX_USED_ADD(page, hlen + cp->crc_size);
}
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 643b97dc52456..90a333ebab683 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -746,7 +746,7 @@ static void veth_xdp_get(struct xdp_buff *xdp)
return;
for (i = 0; i < sinfo->nr_frags; i++)
- __skb_frag_ref(&sinfo->frags[i]);
+ get_netmem(skb_frag_netmem(&sinfo->frags[i]));
}
static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
diff --git a/include/linux/skbuff_ref.h b/include/linux/skbuff_ref.h
index 04114c64dad99..f768a3fa1d606 100644
--- a/include/linux/skbuff_ref.h
+++ b/include/linux/skbuff_ref.h
@@ -12,26 +12,36 @@
bool napi_pp_get_page(netmem_ref netmem);
/**
- * __skb_frag_ref - take an addition reference on a paged fragment.
- * @frag: the paged fragment
+ * skb_netmem_ref - acquire a fragment reference on a netmem
+ * @netmem: netmem to reference
+ * @recycle: whether page_pool recycling is enabled (e.g. skb->pp_recycle)
*
- * Takes an additional reference on the paged fragment @frag.
+ * Acquires pp_ref_count via napi_pp_get_page() if @recycle is true and
+ * @netmem belongs to a page_pool; otherwise acquires a non-pp backing
+ * reference via get_netmem(). Counterpart to skb_netmem_unref().
*/
-static __always_inline void __skb_frag_ref(skb_frag_t *frag)
+static __always_inline void skb_netmem_ref(netmem_ref netmem, bool recycle)
{
- get_netmem(skb_frag_netmem(frag));
+#ifdef CONFIG_PAGE_POOL
+ if (recycle && napi_pp_get_page(netmem))
+ return;
+#endif
+ get_netmem(netmem);
}
/**
- * skb_frag_ref - take an addition reference on a paged fragment of an skb.
+ * skb_frag_ref - acquire a reference on a paged fragment of an skb
* @skb: the buffer
- * @f: the fragment offset.
+ * @f: the fragment offset
*
- * Takes an additional reference on the @f'th paged fragment of @skb.
+ * Acquires a reference on the @f'th fragment of @skb via skb_netmem_ref()
+ * (pp_ref_count if skb->pp_recycle and page_pool-backed, else non-pp
+ * backing refcount). Counterpart to skb_frag_unref().
*/
static __always_inline void skb_frag_ref(struct sk_buff *skb, int f)
{
- __skb_frag_ref(&skb_shinfo(skb)->frags[f]);
+ skb_netmem_ref(skb_frag_netmem(&skb_shinfo(skb)->frags[f]),
+ skb->pp_recycle);
}
bool napi_pp_put_page(netmem_ref netmem);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 46da61ad1f9eb..f03ce8d5ae585 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1133,7 +1133,7 @@ static void skb_release_data(struct sk_buff *skb, enum skb_drop_reason reason)
int i;
if (!skb_data_unref(skb, shinfo))
- goto exit;
+ return;
if (skb_zcopy(skb)) {
bool skip_unref = shinfo->flags & SKBFL_MANAGED_FRAG_REFS;
@@ -1152,17 +1152,6 @@ static void skb_release_data(struct sk_buff *skb, enum skb_drop_reason reason)
kfree_skb_list_reason(shinfo->frag_list, reason);
skb_free_head(skb);
-exit:
- /* When we clone an SKB we copy the reycling bit. The pp_recycle
- * bit is only set on the head though, so in order to avoid races
- * while trying to recycle fragments on skb_netmem_unref() we need
- * to make one SKB responsible for triggering the recycle path.
- * So disable the recycling bit if an SKB is cloned and we have
- * additional references to the fragmented part of the SKB.
- * Eventually the last SKB will have the recycling bit set and it's
- * dataref set to 0, which will trigger the recycling
- */
- skb->pp_recycle = 0;
}
/*
@@ -2287,9 +2276,10 @@ struct sk_buff *__pskb_copy_fclone(struct sk_buff *skb, int headroom,
n = NULL;
goto out;
}
+ n->pp_recycle = skb->pp_recycle;
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
skb_shinfo(n)->frags[i] = skb_shinfo(skb)->frags[i];
- skb_frag_ref(skb, i);
+ skb_frag_ref(n, i);
}
skb_shinfo(n)->nr_frags = i;
skb_shinfo(n)->flags |= skb_shinfo(skb)->flags & SKBFL_SHARED_FRAG;
@@ -3927,6 +3917,7 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
if (len <= skb_tailroom(to))
return skb_copy_bits(from, 0, skb_put(to, len), len);
+ to->pp_recycle = from->pp_recycle;
if (hlen) {
ret = skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
if (unlikely(ret))
@@ -3939,14 +3930,14 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
offset = from->data - (unsigned char *)page_address(page);
__skb_fill_netmem_desc(to, 0, page_to_netmem(page),
offset, plen);
- get_page(page);
+ skb_frag_ref(to, 0);
j = 1;
len -= plen;
}
}
if (!skb_frags_readable(from) && j > 0 && len) {
- put_page(page);
+ skb_frag_unref(to, 0);
return -EFAULT;
}
@@ -3954,7 +3945,7 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
if (unlikely(skb_orphan_frags(from, GFP_ATOMIC))) {
if (j > 0)
- put_page(page);
+ skb_frag_unref(to, 0);
return -ENOMEM;
}
skb_zerocopy_clone(to, from, GFP_ATOMIC);
@@ -4255,7 +4246,7 @@ static inline void skb_split_no_header(struct sk_buff *skb,
* where splitting is expensive.
* 2. Split is accurately. We make this.
*/
- skb_frag_ref(skb, i);
+ skb_frag_ref(skb1, k);
skb_frag_off_add(&skb_shinfo(skb1)->frags[0], len - pos);
skb_frag_size_sub(&skb_shinfo(skb1)->frags[0], len - pos);
skb_frag_size_set(&skb_shinfo(skb)->frags[i], len - pos);
@@ -4286,6 +4277,7 @@ void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len)
skb_shinfo(skb1)->flags |= skb_shinfo(skb)->flags & zc_flags;
skb_zerocopy_clone(skb1, skb, 0);
+ skb1->pp_recycle = skb->pp_recycle;
if (len < pos) /* Split line is inside header. */
skb_split_inside_header(skb, skb1, len, pos);
else /* Second chunk has no header, nothing to copy. */
@@ -4343,8 +4335,8 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
/* Actual merge is delayed until the point when we know we can
* commit all, so that we don't have to undo partial changes
*/
- if (!skb_can_coalesce(tgt, to, skb_frag_page(fragfrom),
- skb_frag_off(fragfrom))) {
+ if (!skb_can_coalesce_netmem(tgt, to, skb_frag_netmem(fragfrom),
+ skb_frag_off(fragfrom))) {
merge = -1;
} else {
merge = to - 1;
@@ -4391,8 +4383,8 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
to++;
} else {
- __skb_frag_ref(fragfrom);
skb_frag_page_copy(fragto, fragfrom);
+ skb_frag_ref(tgt, to);
skb_frag_off_copy(fragto, fragfrom);
skb_frag_size_set(fragto, todo);
@@ -5087,8 +5079,10 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
goto err;
}
+ if (!skb_shinfo(nskb)->nr_frags)
+ nskb->pp_recycle = frag_skb->pp_recycle;
*nskb_frag = (i < 0) ? skb_head_frag_to_page_desc(frag_skb) : *frag;
- __skb_frag_ref(nskb_frag);
+ skb_frag_ref(nskb, skb_shinfo(nskb)->nr_frags);
size = skb_frag_size(nskb_frag);
if (pos < offset) {
@@ -6303,7 +6297,7 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
*/
if (skb_pp_frag_ref(from)) {
for (i = 0; i < from_shinfo->nr_frags; i++)
- __skb_frag_ref(&from_shinfo->frags[i]);
+ get_netmem(skb_frag_netmem(&from_shinfo->frags[i]));
}
to->truesize += delta;
diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
index 3b7d0ab2bcf17..35cc510db4321 100644
--- a/net/tls/tls_device_fallback.c
+++ b/net/tls/tls_device_fallback.c
@@ -256,7 +256,7 @@ static int fill_sg_in(struct scatterlist *sg_in,
for (i = 0; remaining > 0; i++) {
skb_frag_t *frag = &record->frags[i];
- __skb_frag_ref(frag);
+ get_netmem(skb_frag_netmem(frag));
sg_set_page(sg_in + i, skb_frag_page(frag),
skb_frag_size(frag), skb_frag_off(frag));
diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c
index 6920940a35b49..103cf7c5dbfb9 100644
--- a/net/xfrm/xfrm_iptfs.c
+++ b/net/xfrm/xfrm_iptfs.c
@@ -486,8 +486,7 @@ static int iptfs_skb_add_frags(struct sk_buff *skb,
tofrag->len -= offset;
offset = 0;
}
- __skb_frag_ref(tofrag);
- shinfo->nr_frags++;
+ skb_frag_ref(skb, shinfo->nr_frags++);
shinfo->flags |= SKBFL_SHARED_FRAG;
/* see if we are done */
--
2.56.0.385.gd3acb90ef8-goog