Re: [PATCH net-next v9 5/9] udp: Validate UDP length in udp_gro_receive
From: Alice Mikityanska <hidden>
Date: 2026-07-16 18:32:58
On Thu, 16 Jul 2026 at 17:48, 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 [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
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
@@ -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.
Min Ethernet payload size is 46 bytes. Outer IP (20) + UDP (8) + VXLAN (8) + inner Ethernet (14) + inner IP (20) + inner TCP (20) is already way bigger than 46, so "a small TCP ACK inside VXLAN" won't be padded. This check should only trigger on actual malformed packets.
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.
The early return does bypass inner GRO matching, but it does not normally deliver the packet immediately. We call skb_gro_flush_final, marking the current skb for flushing. dev_gro_receive then classifies it as GRO_NORMAL, and gro_normal_one queues it on gro->rx_list. At NAPI flush, gro_flush_normal first flushes held GRO aggregates, then submits the normal list, preserving order in the normal case. Of course, gro_normal_one might submit the normal list early, if gro_normal_batch is reached, but it's an edge case, and we are talking about receiving malformed/padded packets, not something that should normally occur in high-performance TCP traffic.
Before this change,
...the same flush mechanism was used for UDP GRO packets, when a malformed one occurred. It could hit the same edge case (when gro_normal_batch is reached), and reordering is more unpleasant for UDP.
the packet would reach inner GRO handlers (like tcp4_gro_receive()) which would detect the sequence or padding mismatch,
The inner TCP handler won't detect padding at the outer UDP layer, it would be done in inet_gro_receive, which will mark the packet for flush.
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.
If this edge case is important, I can send a follow-up that addresses both the existing case (UDP GRO) and the new case (TCP in UDP tunnel) by separating ulen > skb_gro_len and sizeof(*uh) <= ulen < skb_gro_len cases. In the latter case, I can mark the SKB for flush, but proceed with the inner GRO handlers, so that they could return the held aggregate via pp.
Will bypassing this inner flow matching lead to out-of-order delivery for the inner protocol, resulting in duplicate ACKs and throughput degradation?quoted
/* 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