Re: [PATCH net-next v2 11/15] net: skbuff: refuse to segment a nested frag_list

[Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx>]

On Thu, Oct 22, 2026 at 8:44 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> On Wed, Oct 21, 2026 at 09:47:15PM -0400, Willem de Bruijn wrote:
> > This patch became quite a bit more complex because of my request.
> > More than I anticipated.
> >
> > Are you sure this first block is needed?
>
> No, it isn't.  I took it out and ran three KUnit cases with a nested
> frag_list under GSO_BY_FRAGS: a linear first member with the nested
> bytes counted in its len, the same without them counted, and a paged
> first member.  They all still get -EINVAL, from the check in the clone
> branch or the one in the frag walk.
>
> > this test for list_skb != NULL is new. Previously this code already
> > tested skb_headlen(list_skb) without it. Likely not needed for
> > anything new in this patch?
>
> Same answer, the cases above pass without it too.  With every member
> checked as it's taken, list_skb can only run out if head_skb->len is
> wrong for some other reason, which isn't what this patch is about.
>
> So done this way the patch comes down to the check you suggested in the
> frag walk, plus skb_has_frag_list(list_skb) added to the existing
> headlen check in the clone branch.  The second one is needed: with only
> the frag walk check, the frag_list_nested KUnit case still hits a NULL
> pointer dereference in skb_segment(), because the linear member is
> cloned, list_skb becomes NULL, and the next pass reads its headlen.
>
> The other option is to go back to the single walk up front from v1.
> It's an extra pass over the frag_list, but it's one check in one place.
> The difference in behaviour is without SG: there a nested frag_list
> whose member doesn't match gso_size goes through the copy path, which
> recurses, and a KUnit case for that gets correctly sized segments
> today.  The in-walk checks leave that alone and the up front walk would
> refuse it.  Nothing in the tree builds a nested frag_list either way.
>
> I lean towards the walk up front for v3 because it's simpler to read.
> Would you be ok with that, or do you prefer the two in-walk checks?

Given the complexity of skb_segment already, and how non-trivial
the added inline checks are, I'm also okay with the up front walk.

It only adds cost for GSO skbs that use a frag_list. Though that
may include some relatively common cases, such as BIG TCP.