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

[Josef Bacik <josef@xxxxxxxxxxxxxx>]

skb_segment() walks head_skb's frag_list, but for each member it only
looks at the linear part and frags[], never at the member's own
frag_list.  head_skb->len still counts those bytes, so with a nested
frag_list the walk runs out of members while it still owes data, sets
list_skb to NULL, and dereferences it on a later pass, before any of
the layout checks get a chance to catch it.  When the member is taken
whole with skb_clone(), the pskb_trim() that follows also drops its
nested data.

Nothing in the tree is known to build such an skb; GRO flushes instead
of chaining an skb that already has a frag_list.  Check each member as
the walk takes it, in the GSO_BY_FRAGS, clone and frag paths, and fail
the segmentation with -EINVAL if there's no member left or it has its
own frag_list, with a DEBUG_NET_WARN_ON_ONCE() for debug kernels, like
the other frag_list layout checks.  Without SG the payload is copied
with helpers that do recurse into nested frag_lists, so that path is
left alone.

Suggested-by: Willem de Bruijn <willemb@xxxxxxxxxx>
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
 net/core/skbuff.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 56cec58bccb7..c5e28a52114b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5087,6 +5087,15 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
                int size;
 
                if (unlikely(gso_by_frags)) {
+                       /* The walk only goes one level deep: running out of
+                        * members, or finding one with its own frag_list, means
+                        * a nested frag_list that head_skb->len counts.
+                        */
+                       if (unlikely(!list_skb || skb_has_frag_list(list_skb))) 
{
+                               DEBUG_NET_WARN_ON_ONCE(1);
+                               err = -EINVAL;
+                               goto err;
+                       }
                        len = list_skb->len;
                } else {
                        len = head_skb->len - offset;
@@ -5096,9 +5105,11 @@ 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)) {
+                       if (unlikely(skb_headlen(list_skb) > len ||
+                                    skb_has_frag_list(list_skb))) {
                                DEBUG_NET_WARN_ON_ONCE(1);
                                err = -EINVAL;
                                goto err;
@@ -5216,6 +5227,13 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 
                while (pos < offset + len) {
                        if (i >= nfrags) {
+                               if (unlikely(!list_skb ||
+                                            skb_has_frag_list(list_skb))) {
+                                       DEBUG_NET_WARN_ON_ONCE(1);
+                                       err = -EINVAL;
+                                       goto err;
+                               }
+
                                if (skb_orphan_frags(list_skb, GFP_ATOMIC) ||
                                    skb_zerocopy_clone(nskb, list_skb,
                                                       GFP_ATOMIC))

-- 
2.55.0