Thread (32 messages) read the whole thread 32 messages, 8 authors, 8d ago

Re: [PATCH net-next v9 5/9] udp: Validate UDP length in udp_gro_receive

From: Simon Horman <horms@kernel.org>
Date: 2026-07-16 14:48:26

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 [5]: [PATCH net-next v9 5/9] udp: Validate UDP length in udp_gro_receive ---
commit f1316d873c7549e0a49dc98d7614be1dc5b0f66c
Author: Alice Mikityanska [off-list ref]

udp: Validate UDP length in udp_gro_receive

This patch adds an early check for malformed or padded UDP packets with
len=0 at udp_gro_receive to exclude them from GRO, moving the check out of
udp_gro_receive_segment.
quoted hunk ↗ jump to hunk
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 4f9a3922937cc..8f77c8788f6db 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -782,8 +778,14 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
 	struct sk_buff *p;
 	struct udphdr *uh2;
 	unsigned int off = skb_gro_offset(skb);
+	unsigned int ulen;
 	int flush = 1;
 
+	/* Do not deal with padded or malicious packets, sorry! */
+	ulen = udp_get_len_short(uh);
+	if (ulen <= sizeof(*uh) || ulen != skb_gro_len(skb))
+		goto out;
+
[Severity: High]
Could this early return bypass inner GRO flow matching for padded UDP tunnel
packets?

When a padded UDP tunnel packet, such as a small TCP ACK inside VXLAN or
GENEVE, arrives in net/ipv4/udp_offload.c:udp_gro_receive() while previous
packets of the same inner flow are held in the GRO list, the
ulen != skb_gro_len(skb) check evaluates to true and triggers the goto out.

Because the goto out path yields pp = NULL and flush = 1, dev_gro_receive()
passes the new padded packet to the network stack immediately, leaving older
packets stuck in the GRO list.

Before this change, the packet would reach inner GRO handlers (like
tcp4_gro_receive()) which would detect the sequence or padding mismatch,
set flush = 1, and return the held packet via pp = p. This ensured older
packets were flushed to the stack before the new padded packet, preserving
delivery order.

Will bypassing this inner flow matching lead to out-of-order delivery for
the inner protocol, resulting in duplicate ACKs and throughput degradation?
 	/* We can do L4 aggregation only if the packet can't land in a tunnel
 	 * otherwise we could corrupt the inner stream. Detecting such packets
 	 * cannot be foolproof and the aggregation might still happen in some
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help