All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hyunwoo Kim <imv4bel@gmail.com>
To: Aaron Esau <aaron1esau@gmail.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, sultan@kerneltoast.com,
	sd@queasysnail.net, steffen.klassert@secunet.com,
	herbert@gondor.apana.org.au, dsahern@kernel.org,
	netdev@vger.kernel.org, stable@vger.kernel.org,
	gregkh@linuxfoundation.org, imv4bel@gmail.com
Subject: Re: [PATCH net v4] net: skbuff: propagate shared-frag marker through frag-transfer helpers
Date: Sat, 16 May 2026 06:36:26 +0900	[thread overview]
Message-ID: <ageR2qzTRtpH8JGY@v4bel> (raw)
In-Reply-To: <ageAmZcEMu4Yjyyl@v4bel>

On Sat, May 16, 2026 at 05:22:49AM +0900, Hyunwoo Kim wrote:
> 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

Ah, I see. you've released another exploit publicly.

https://github.com/v12-security/pocs/tree/main/fragnesia-5db89c99566fc

It looks like the exploit doesn't work because the GRO propagation
in the already-published v4 patch prevents the SHARED_FRAG asymmetry
from being created in the first place.

In any case, v5 will be submitted shortly.

  reply	other threads:[~2026-05-15 21:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15  5:55 [PATCH net v4] net: skbuff: propagate shared-frag marker through frag-transfer helpers Hyunwoo Kim
2026-05-15  6:07 ` Hyunwoo Kim
2026-05-15  6:26 ` Sultan Alsawaf
2026-05-15  6:36   ` Hyunwoo Kim
2026-05-15 15:11     ` Hyunwoo Kim
2026-05-15 16:41 ` Aaron Esau
2026-05-15 20:22   ` Hyunwoo Kim
2026-05-15 21:36     ` Hyunwoo Kim [this message]
2026-05-16  0:08       ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ageR2qzTRtpH8JGY@v4bel \
    --to=imv4bel@gmail.com \
    --cc=aaron1esau@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sd@queasysnail.net \
    --cc=stable@vger.kernel.org \
    --cc=steffen.klassert@secunet.com \
    --cc=sultan@kerneltoast.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.