net: skbuff: replace skb_page_unref() and __skb_frag_unref() with skb_netmem_unref()

Unreferencing a page_pool fragment with recycle=false decrements the
non-pp backing refcount instead of pp_ref_count, underflowing the page
refcount when the fragment was owned via page_pool (as on mlx4's RX
error path). Having both skb_page_unref() and __skb_frag_unref(frag,
recycle) made it easy for callers to pass hardcoded recycle=false on
SKB-owned fragments.

Replace skb_page_unref() and __skb_frag_unref() with
skb_netmem_unref(netmem, recycle), which dispatches to
napi_pp_put_page() when recycle is true and put_netmem() otherwise:

- Use skb_frag_unref(skb, ...) on SKB-owned fragments in mlx4, sky2,
  tls_strp, and skb_shift() so unrefs always match skb->pp_recycle.
- Use put_netmem() directly in tls_device's destroy_record(), which
  releases raw non-pp page references outside an SKB.

Found by code inspection and confirmed with the KUnit tests added later
in this series; not observed in production. The mlx4, sky2, and TLS
call-site changes are compile-tested only.

Assisted-by: LLM
Signed-off-by: Mina Almasry <almasrymina@google.com>
Link: https://patch.msgid.link/20261014173945.373295-4-almasrymina@google.com
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Mina Almasry 2026-10-14 17:37:17 +00:00 committed by Paolo Abeni
parent 729a47b803
commit 4d68808407
8 changed files with 29 additions and 28 deletions

View File

@ -2501,7 +2501,7 @@ static void skb_put_frags(struct sk_buff *skb, unsigned int hdr_space,
if (length == 0) {
/* don't need this page */
__skb_frag_unref(frag, false);
skb_frag_unref(skb, i);
--skb_shinfo(skb)->nr_frags;
} else {
size = min(length, (unsigned) PAGE_SIZE);

View File

@ -497,7 +497,7 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
fail:
while (nr > 0) {
nr--;
__skb_frag_unref(skb_shinfo(skb)->frags + nr, false);
skb_frag_unref(skb, nr);
}
return 0;
}

View File

@ -36,7 +36,16 @@ static __always_inline void skb_frag_ref(struct sk_buff *skb, int f)
bool napi_pp_put_page(netmem_ref netmem);
static __always_inline void skb_page_unref(netmem_ref netmem, bool recycle)
/**
* skb_netmem_unref - release a fragment reference on a netmem
* @netmem: netmem to unreference
* @recycle: whether page_pool recycling is enabled (e.g. skb->pp_recycle)
*
* Drops pp_ref_count via napi_pp_put_page() if @recycle is true and @netmem
* belongs to a page_pool; otherwise drops a non-pp backing reference via
* put_netmem(). Counterpart to skb_netmem_ref().
*/
static __always_inline void skb_netmem_unref(netmem_ref netmem, bool recycle)
{
#ifdef CONFIG_PAGE_POOL
if (recycle && napi_pp_put_page(netmem))
@ -46,31 +55,22 @@ static __always_inline void skb_page_unref(netmem_ref netmem, bool recycle)
}
/**
* __skb_frag_unref - release a reference on a paged fragment.
* @frag: the paged fragment
* @recycle: recycle the page if allocated via page_pool
*
* Releases a reference on the paged fragment @frag
* or recycles the page via the page_pool API.
*/
static __always_inline void __skb_frag_unref(skb_frag_t *frag, bool recycle)
{
skb_page_unref(skb_frag_netmem(frag), recycle);
}
/**
* skb_frag_unref - release a reference on a paged fragment of an skb.
* skb_frag_unref - release a reference on a paged fragment of an skb
* @skb: the buffer
* @f: the fragment offset
*
* Releases a reference on the @f'th paged fragment of @skb.
* Drops a reference on the @f'th fragment of @skb via skb_netmem_unref()
* (pp_ref_count if skb->pp_recycle and page_pool-backed, else non-pp
* backing refcount), skipping managed zerocopy fragments. Counterpart to
* skb_frag_ref().
*/
static __always_inline void skb_frag_unref(struct sk_buff *skb, int f)
{
struct skb_shared_info *shinfo = skb_shinfo(skb);
if (!skb_zcopy_managed(skb))
__skb_frag_unref(&shinfo->frags[f], skb->pp_recycle);
skb_netmem_unref(skb_frag_netmem(&shinfo->frags[f]),
skb->pp_recycle);
}
#endif /* _LINUX_SKBUFF_REF_H */

View File

@ -1144,7 +1144,8 @@ static void skb_release_data(struct sk_buff *skb, enum skb_drop_reason reason)
}
for (i = 0; i < shinfo->nr_frags; i++)
__skb_frag_unref(&shinfo->frags[i], skb->pp_recycle);
skb_netmem_unref(skb_frag_netmem(&shinfo->frags[i]),
skb->pp_recycle);
free_head:
if (shinfo->frag_list)
@ -1154,7 +1155,7 @@ static void skb_release_data(struct sk_buff *skb, enum skb_drop_reason reason)
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_frag_unref() we need
* 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.
@ -4465,7 +4466,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
fragto = &skb_shinfo(tgt)->frags[merge];
skb_frag_size_add(fragto, skb_frag_size(fragfrom));
__skb_frag_unref(fragfrom, skb->pp_recycle);
skb_frag_unref(skb, 0);
}
/* Reposition in the original skb */

View File

@ -117,8 +117,8 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb,
struct scatterlist *src = already_unref ? esp_req_sg(aead, req) : req->src;
for (sg = sg_next(src); sg; sg = sg_next(sg))
skb_page_unref(page_to_netmem(sg_page(sg)),
skb->pp_recycle);
skb_netmem_unref(page_to_netmem(sg_page(sg)),
skb->pp_recycle);
}
}

View File

@ -134,8 +134,8 @@ static void esp_ssg_unref(struct xfrm_state *x, void *tmp, struct sk_buff *skb,
struct scatterlist *src = already_unref ? esp_req_sg(aead, req) : req->src;
for (sg = sg_next(src); sg; sg = sg_next(sg))
skb_page_unref(page_to_netmem(sg_page(sg)),
skb->pp_recycle);
skb_netmem_unref(page_to_netmem(sg_page(sg)),
skb->pp_recycle);
}
}

View File

@ -143,7 +143,7 @@ static void destroy_record(struct tls_record_info *record)
int i;
for (i = 0; i < record->num_frags; i++)
__skb_frag_unref(&record->frags[i], false);
put_netmem(skb_frag_netmem(&record->frags[i]));
kfree(record);
}

View File

@ -197,7 +197,7 @@ static void tls_strp_flush_anchor_copy(struct tls_strparser *strp)
DEBUG_NET_WARN_ON_ONCE(atomic_read(&shinfo->dataref) != 1);
for (i = 0; i < shinfo->nr_frags; i++)
__skb_frag_unref(&shinfo->frags[i], false);
skb_frag_unref(strp->anchor, i);
shinfo->nr_frags = 0;
if (strp->copy_mode) {
kfree_skb_list(shinfo->frag_list);