Re: [PATCH net-next v9 2/9] net: Enable BIG TCP with partial GSO
From: Alice Mikityanska <hidden>
Date: 2026-07-16 16:29:10
On Thu, 16 Jul 2026 at 17:47, Simon Horman [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 [off-list ref] 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.quoted
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[ ... ]quoted
@@ -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?
I already addressed this exact concern here: https://lore.kernel.org/netdev/1cf2746f-f15b-406d-bb49-2c3f2dee1ec2@app.fastmail.com/ (local) Key points: 1. skb_segment can fail with "too many frags" before my changes, even without BIG TCP. It's not a new bug that my patch adds. 2. tcp_sendmsg_locked doesn't build frag_list SKBs, and non-frag_list SKBs never fail in skb_segment with "too many frags". 3. When sending a TCP stream, 32768-byte high order pages are allocated, so even a 512 KB SKB can be represented without frag_list (and if such high-order allocation fails, tcp_sendmsg_locked won't use frag_list anyway). 4. The only scenario I can think of to get "too many frags" with partial GSO is receiving traffic and forwarding it to a partial GSO netdev. But again, then it easily hits "too many frags" even without BIG TCP or my changes (see the repro by the link above). There is definitely no "deterministic drop of BIG TCP" here, because of #2, and the selftest shows that it works. So, I'm open to discussion on how to make skb_segment more reliable (I wrote one idea by the link above), but I don't think it should block this series.
[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;
...
}I'll clamp partial_segs to GSO_MAX_SEGS=65535 to make sure that gso_segs doesn't overflow. Worst case, in an extreme scenario of gso_size=1, partial GSO will produce more than 2 output SKBs, but the traffic should still flow — this is how BIG TCP SKBs were handled by skb_segment before this change. This overflow shouldn't be possible with TCP, because TCP_MIN_GSO_SIZE=8, but AI suggests that virtio_net_hdr_to_skb can accept any gso_size, even 1, which makes such overflow possible.
Could this silent truncation corrupt hardware engine programming or Byte Queue Limits?quoted
if (partial_segs > 1) mss *= partial_segs; else[ ... ]