[PATCH net 1/1] net: skbuff: fix pskb_carve leaking zcopy pages

[PATCH net 1/1] net: skbuff: fix pskb_carve leaking zcopy pages

[Pavel Begunkov]

When SKBFL_MANAGED_FRAG_REFS is set, frag pages are not refcounted but
their lifetime is controlled by the attached ubuf_info. To make a copy
of the skb_shared_info, we either should clear the flag and reference
the frags, or keep the flag and have frags unreferenced.

pskb_carve_inside_header() and pskb_carve_inside_nonlinear() don't
follow the rule and thus can leak page references. Let's clear
SKBFL_MANAGED_FRAG_REFS from the original skb to fix it. It's the
simplest way to address it, but there are more performant ways to do
that if it ever becomes a problem.

Link: https://lore.kernel.org/all/20260523085809.26331-1-nvminh232@clc.fitus.edu.vn/
Fixes: 753f1ca4e1e50 ("net: introduce managed frags infrastructure")
Reported-by: Minh Nguyen <minhnguyen.080505@gmail.com>
Reported-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 net/core/skbuff.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0d3cc115f2e7..c02f0a507ba8 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -6823,6 +6823,11 @@ static int pskb_carve_inside_header(struct sk_buff *skb, const u32 off,
 	skb_copy_from_linear_data_offset(skb, off, data, new_hlen);
 	skb->len -= off;
 
+	/* Remove SKBFL_MANAGED_FRAG_REFS instead of trying to honour it
+	 * while refcounting frags below.
+	 */
+	skb_zcopy_downgrade_managed(skb);
+
 	memcpy((struct skb_shared_info *)(data + size),
 	       skb_shinfo(skb),
 	       offsetof(struct skb_shared_info,
@@ -6936,6 +6941,11 @@ static int pskb_carve_inside_nonlinear(struct sk_buff *skb, const u32 off,
 		return -ENOMEM;
 	size = SKB_WITH_OVERHEAD(size);
 
+	/* Remove SKBFL_MANAGED_FRAG_REFS instead of trying to honour it
+	 * while refcounting frags below.
+	 */
+	skb_zcopy_downgrade_managed(skb);
+
 	memcpy((struct skb_shared_info *)(data + size),
 	       skb_shinfo(skb), offsetof(struct skb_shared_info, frags[0]));
 	if (skb_orphan_frags(skb, gfp_mask)) {
-- 
2.54.0

Re: [PATCH net 1/1] net: skbuff: fix pskb_carve leaking zcopy pages

[Willem de Bruijn]

Pavel Begunkov wrote:
> When SKBFL_MANAGED_FRAG_REFS is set, frag pages are not refcounted but
> their lifetime is controlled by the attached ubuf_info. To make a copy
> of the skb_shared_info, we either should clear the flag and reference
> the frags, or keep the flag and have frags unreferenced.
> 
> pskb_carve_inside_header() and pskb_carve_inside_nonlinear() don't
> follow the rule and thus can leak page references. Let's clear
> SKBFL_MANAGED_FRAG_REFS from the original skb to fix it. It's the
> simplest way to address it, but there are more performant ways to do
> that if it ever becomes a problem.
> 
> Link: https://lore.kernel.org/all/20260523085809.26331-1-nvminh232@clc.fitus.edu.vn/
> Fixes: 753f1ca4e1e50 ("net: introduce managed frags infrastructure")
> Reported-by: Minh Nguyen <minhnguyen.080505@gmail.com>
> Reported-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

> ---
>  net/core/skbuff.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 0d3cc115f2e7..c02f0a507ba8 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -6823,6 +6823,11 @@ static int pskb_carve_inside_header(struct sk_buff *skb, const u32 off,
>  	skb_copy_from_linear_data_offset(skb, off, data, new_hlen);
>  	skb->len -= off;
>  
> +	/* Remove SKBFL_MANAGED_FRAG_REFS instead of trying to honour it
> +	 * while refcounting frags below.
> +	 */

FWIW the multi-line comments are not really needed. The function of
skb_zcopy_downgrade_managed is quite clear.