Thread (32 messages) 32 messages, 8 authors, 26m ago

Re: [PATCH net-next v9 1/9] net: Use helpers to get/set UDP len tree-wide

From: Simon Horman <horms@kernel.org>
Date: 2026-07-17 12:40:52

On Thu, Jul 16, 2026 at 06:18:59PM +0300, Alice Mikityanska wrote:
On Thu, 16 Jul 2026 at 17:47, Simon Horman [off-list ref] wrote:
quoted
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 [1]: [PATCH net-next v9 1/9] net: Use helpers to get/set UDP len tree-wide ---
commit 113d28557bcedccc492f330fafdaaf1377eb2e79
Author: Alice Mikityanska [off-list ref]

net: Use helpers to get/set UDP len tree-wide

This commit introduces helpers for setting and getting the UDP header length
tree-wide. It prepares the codebase to support BIG TCP for UDP tunnels by
explicitly handling cases where UDP length might be 0 for packets larger
than 65535 bytes.
quoted
diff --git a/net/6lowpan/nhc_udp.c b/net/6lowpan/nhc_udp.c
index 0a506c77283dd..ed4227e6db744 100644
--- a/net/6lowpan/nhc_udp.c
+++ b/net/6lowpan/nhc_udp.c
@@ -88,16 +88,16 @@ static int udp_uncompress(struct sk_buff *skb, size_t needed)
      switch (lowpan_dev(skb->dev)->lltype) {
      case LOWPAN_LLTYPE_IEEE802154:
              if (lowpan_802154_cb(skb)->d_size)
-                     uh.len = htons(lowpan_802154_cb(skb)->d_size -
-                                    sizeof(struct ipv6hdr));
+                     udp_set_len_short(&uh, lowpan_802154_cb(skb)->d_size -
+                                       sizeof(struct ipv6hdr));
[Severity: High]
If a crafted packet specifies a datagram size smaller than
sizeof(struct ipv6hdr), won't this subtraction underflow?
First, I'd like to emphasize that I'm merely converting direct uh.len
access to the new helper. If there is an existing bug with underflow,
my new helper will expose it with a WARN, which is good. But should
such bugs exist before this refactoring, I don't think that fixing all
of them is in scope of this series.
Yes, point taken.

...
quoted
[Severity: High]
If gro_max_size is configured larger than 65535 (which is supported and used
for BIG TCP), can't UDP GRO aggregate packets such that newlen > 65535?
This is the first patch that just converted direct access to helpers.
Patch 4 "udp: Support gro_ipv4_max_size > 65536" explicitly enables
len > 64k and replaces this line with udp_set_len. This comment on
patch 1 is irrelevant because it assumes that functionality added in a
later patch should work in a prior patch.
Agreed, the AI-generated review seems wide of the mark here.

...
quoted
[Severity: High]
When transmitting a TCP BIG TCP packet (which can be > 64K) over a UDP
tunnel like VXLAN, skb->len will exceed 65535.

Does using udp_set_len_short() here incorrectly trigger the warning for
valid large GSO payloads?
Same here. This is replaced by udp_set_len, which supports len > 64k,
in the patch that actually enables len > 64k.
quoted
quoted
[ ... ]
diff --git a/net/ipv6/ip6_udp_tunnel.c b/net/ipv6/ip6_udp_tunnel.c
index 9adb5775487fa..dcff7fb16ff64 100644
--- a/net/ipv6/ip6_udp_tunnel.c
+++ b/net/ipv6/ip6_udp_tunnel.c
@@ -93,7 +93,7 @@ void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
      uh->dest = dst_port;
      uh->source = src_port;

-     uh->len = htons(skb->len);
+     udp_set_len_short(uh, skb->len);
[Severity: High]
This appears to have the same issue as IPv4 tunnels. If a large GSO payload
is sent through an IPv6 UDP tunnel, won't skb->len exceed 65535 and trigger
the warning?
Same here.

I'm sorry, but I highly doubt that these AI review comments were validated.
Clearly my validation was insufficient.
Sorry about that.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help