Re: [PATCH net-next v9 2/9] net: Enable BIG TCP with partial GSO

[Simon Horman <horms@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [2]: [PATCH net-next v9 2/9] net: Enable BIG TCP with partial GSO ---
commit b8199f77774400021642eb53bd60ec1a062a54ef
Author: Alice Mikityanska <alice@isovalent.com>

net: Enable BIG TCP with partial GSO
    
This commit removes the 65534-byte clamp on segment length in skb_segment
for partial GSO. It stores the gso_by_frags flag early to allow BIG TCP
packets to use larger lengths without accidental resegmentation.

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 18dabb4e9cfa0..6ae4c2b205f2e 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c

[ ... ]

> @@ -4852,9 +4853,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>  		/* GSO partial only requires that we trim off any excess that
>  		 * doesn't fit into an MSS sized block, so take care of that
>  		 * now.
> -		 * Cap len to not accidentally hit GSO_BY_FRAGS.
>  		 */
> -		partial_segs = min(len, GSO_BY_FRAGS - 1) / mss;
> +		partial_segs = len / mss;

[Severity: High]
When removing the clamp on partial GSO segments, mss is scaled up without any
upper bound. For BIG TCP packets, could this construct segments that exceed
MAX_SKB_FRAGS?

If a 512KB packet is passed to skb_segment() for partial GSO, mss is scaled
up to the full packet length. When attempting to build this single large
segment by flattening the frag_list into a new frags array, it appears it
would hit the fragment limit and abort:

net/core/skbuff.c:skb_segment() {
    ...
    if (unlikely(skb_shinfo(nskb)->nr_frags >=
                 MAX_SKB_FRAGS)) {
        net_warn_ratelimited(
            "skb_segment: too many frags: %u %u\n",
            pos, mss);
        err = -EINVAL;
        goto err;
    }
    ...
}

Does this deterministically drop BIG TCP traffic over tunnels that require
partial GSO?

[Severity: Medium]
Can the partial_segs calculation exceed 65535 for malicious inputs and cause
a 16-bit truncation?

If a crafted packet with a very small gso_size (for example, mss=1 via an
untrusted packet socket) and a large payload is segmented, len / mss could
produce a value greater than 65535.

When this 32-bit value is later assigned to the 16-bit field in skb_segment():

net/core/skbuff.c:skb_segment() {
    ...
    skb_shinfo(iter)->gso_segs = partial_segs;
    ...
}

Could this silent truncation corrupt hardware engine programming or Byte Queue
Limits?

>  		if (partial_segs > 1)
>  			mss *= partial_segs;
>  		else

[ ... ]