Re: [PATCH net-next v3 11/15] net: skbuff: don't run off frag_list in skb_segment()

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Josef Bacik wrote:
> skb_segment() keeps going until it has segmented head_skb->len bytes.
> It takes the data from head_skb and then from each member of the
> frag_list.  If head_skb->len counts data that this walk can't find, the
> walk runs out of members while it still owes data and dereferences a
> NULL list_skb.  There are two ways to get there:
> 
>  - head_skb->len is bigger than what head_skb and its frag_list hold.
>  - A member has a frag_list of its own.  head_skb->len counts those
>    bytes, but for each member the walk only looks at the linear part and
>    frags[].
> 
> No path in the tree is known to build a GSO skb like either.  GRO
> flushes instead of chaining an skb that already has a frag_list.  IP
> reassembly does nest frag_lists, but a reassembled skb isn't GSO.
> 
> Check list_skb in the three places where the walk moves on to it, and
> fail the segmentation with -EINVAL.  The KUnit cases in the next patch
> hit the NULL pointer dereference without this patch, and at least one of
> them hits it again with any one of the three checks taken out.
> 
> Without SG the payload is copied by helpers that check the length
> themselves and recurse into a nested frag_list.  That path still fails
> for a len that is too big, and still segments a nested frag_list.
> 
> Found by the Sashiko AI review of the BUG_ON() series.
> 
> Link: https://lore.kernel.org/all/20261007-b4-skbuff-bug-on-v2-0-b9a5f732895b@toxicpanda.com/
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

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