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

[Josef Bacik <josef@xxxxxxxxxxxxxx>]

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?

net-next is closed, so I'll resend the whole series once it reopens.

Thanks,
Josef