Re: [PATCH net v4] net: skbuff: propagate shared-frag marker through frag-transfer helpers

[Hyunwoo Kim <imv4bel@gmail.com>]

On Fri, May 15, 2026 at 11:41:21AM -0500, Aaron Esau wrote:
> skb_segment() propagates SKBFL_SHARED_FRAG from head_skb only.  When
> segments pull frags from frag_list members, the flag is never
> propagated from those members into the segment skb.
> 
> There are two miss sites:
> 
> 1. Line ~4986: a new nskb propagates only from head_skb, but frag_skb
>    may already point to a list_skb carried over from the previous
>    segment's iteration (i, nfrags, frag_skb persist across the outer
>    do/while).
> 
> 2. When the inner loop exhausts head_skb frags and switches to a
>    list_skb (line ~4999-5002), frag_skb is updated but its
>    SKBFL_SHARED_FRAG is not propagated into nskb.
> 
> Your v4 GRO fix means head_skb will normally carry the flag, so
> skb_segment() picks it up indirectly.  But skb_segment() itself should
> propagate from frag_list members directly --- otherwise any non-GRO
> frag_list producer re-exposes the gap.
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4986,7 +4986,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  
> -		skb_shinfo(nskb)->flags |= skb_shinfo(head_skb)->flags &
> -					   SKBFL_SHARED_FRAG;
> +		skb_shinfo(nskb)->flags |= (skb_shinfo(head_skb)->flags |
> +					    skb_shinfo(frag_skb)->flags) &
> +					   SKBFL_SHARED_FRAG;
>  
>  		if (skb_zerocopy_clone(nskb, frag_skb, GFP_ATOMIC))
> @@ -5000,6 +5001,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  				frag = skb_shinfo(list_skb)->frags;
>  				frag_skb = list_skb;
>  
> +				skb_shinfo(nskb)->flags |= skb_shinfo(frag_skb)->flags & SKBFL_SHARED_FRAG;
> +
>  				if (!skb_headlen(list_skb)) {
>  					BUG_ON(!nfrags);
>  				} else {
> 
> Site 1 covers segments that start mid-list_skb (frag_skb carried from
> the previous segment).  Site 2 covers segments that switch from
> head_skb frags to list_skb frags mid-construction.
> 
> Fixes: cef401de7be8 ("net: fix possible wrong checksum generation")

If I understand correctly, triggering this in practice requires both
an skb with SHARED_FRAG asymmetry and that skb reaching skb_segment()
with GSO set, is that right? Looking at mainline, I couldn't find any
code path that produces such a combination.

Do you happen to have a reproducer or a concrete trigger call path?
If so, please share, I'd appreciate it.

Anyway, since I consider this one of the "relatively" more concerning
items among the "potential issues", I'll wait a bit longer for
additional reviews and then include it in v5.

As a heads-up, after this one, I don't plan to fold further "potential"
fixes into this patch. This patch is intended as an urgent fix for an 
actually triggerable issue, and the remaining potential issues are
more likely to be addressed together as a separate batch later:

https://lore.kernel.org/all/20260514163802.1d49d7cb@kernel.org/

Thanks for the review.

Best regards,
Hyunwoo Kim