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

[Josef Bacik <josef@xxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxx/
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
 net/core/skbuff.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e3fc6291d223..b2c44a58db23 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5164,7 +5164,16 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
                int hsize;
                int size;
 
+               /* list_skb runs out early if head_skb->len counts data that
+                * this walk can't find: bytes that aren't there, or a member's
+                * own frag_list, which the walk never looks inside.
+                */
                if (unlikely(gso_by_frags)) {
+                       if (unlikely(!list_skb)) {
+                               DEBUG_NET_WARN_ON_ONCE(1);
+                               err = -EINVAL;
+                               goto err;
+                       }
                        len = list_skb->len;
                } else {
                        len = head_skb->len - offset;
@@ -5174,7 +5183,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 
                hsize = skb_headlen(head_skb) - offset;
 
-               if (hsize <= 0 && i >= nfrags && skb_headlen(list_skb) &&
+               if (hsize <= 0 && i >= nfrags && list_skb &&
+                   skb_headlen(list_skb) &&
                    (skb_headlen(list_skb) == len || sg)) {
                        if (unlikely(skb_headlen(list_skb) > len)) {
                                DEBUG_NET_WARN_ON_ONCE(1);
@@ -5294,6 +5304,12 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 
                while (pos < offset + len) {
                        if (i >= nfrags) {
+                               if (unlikely(!list_skb)) {
+                                       DEBUG_NET_WARN_ON_ONCE(1);
+                                       err = -EINVAL;
+                                       goto err;
+                               }
+
                                if (skb_shinfo(nskb)->nr_frags &&
                                    nskb->pp_recycle != list_skb->pp_recycle) {
                                        nskb->data_len = pos - offset - hsize;

-- 
2.55.0