[PATCH net-next v4 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.

Turning SG off doesn't avoid it.  The walk reads list_skb to decide
whether to clone the next member before it looks at SG.  With the checks
in place the payload is copied by skb_copy_bits(), which checks the
length itself and recurses into a nested frag_list.  So without SG a len
that is too big fails in the copy, and a nested frag_list is segmented.

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/
Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>
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 7ecf6701f68c..45cd77e153f1 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);
@@ -5296,6 +5306,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