Re: [PATCH net v2] net: dst_metadata: fix fortify trap + overread in skb_metadata_dst_cmp
flat view
From: Jean-Paul Sergent <hidden>
Date: 2026-10-04 01:23:56
Also in:
stable
On Sat, Oct 03, 2026 at 03:31:07PM +0200, Ilya Maximets wrote:
Here the function just compares two blocks and they must be already fully initialized and have options_len properly set. If they have options, but the length is zero, that's a bug somewhere else.
Following up on my earlier reply: I captured the outer traffic on the receiving node to answer where the differing options_len come from. Neither packet is buggy or uninitialized - both lengths are legitimate traffic on the same geneve device, and the comparison just cannot handle the mix safely. This cluster runs Cilium with bpf-lb-mode: dsr and bpf-lb-dsr-dispatch: geneve. The service LB node forwards a client's SYN inside geneve with a 12-byte DSR option appended (class 0x014b, type 0x81, carrying the LB address and service port), while the rest of the connection and all regular overlay traffic ride the same tunnel with no options - this Cilium setup adds no identity option to normal overlay packets. 60 seconds of capture (talosctl pcap, unfiltered; GRO is disabled on the geneve device here as the standing mitigation, which does not affect the outer side): 1,423 option-bearing packets from the LB node, every one a TCP SYN, against 70,935 zero-option packets. 1,370 flows carried both classes on the same inner 5-tuple within that one window - the SYN with the option, then data without it - all inbound to one NodePort service. How that turns into the overread: geneve RX pulls the tunnel headers and clears the skb hash (iptunnel_pull_header() -> skb_clear_hash_if_not_l4()), and these virtio netdevs provide no receive hash, so every decapped packet enters the per-CPU gro cell with skb->hash == 0 and dev_gro_receive() buckets them all into the same gro_hash list. In gro_list_prepare() the remaining gates before the metadata-dst comparison are the inner ethernet header compare and the slow_gro path. DSR delivers every flow for a service to the same backend pod, so those flows share the inner destination MAC, and because the LB node encapsulates every client packet it forwards, they share the inner source MAC - the LB node's - as well. With a torrent client running on that pod, hundreds of concurrent connections hit the same service: the SYN of one connection and the data segment of another pass every check up to skb_metadata_dst_cmp(), which then reads 96 + 12 bytes against a 96 + 0 allocation - the "108 byte read of buffer size 96" from the original report, with the earlier packet on the GRO list (skb_a) carrying the longer options, matching the reported trip direction. GRO batches here live exactly one NAPI poll (gro_flush_timeout is 0), so a SYN and a same-service data segment merely have to be processed by the same poll - routine at ~23 DSR SYNs/s mixed into ~70k packets/min of overlay traffic. This also explains why my single-stream iperf attempts never reproduced it: a lone connection's SYN and data are separated by RTT and cannot share a poll, and it has no second same-MAC-pair flow to collide with. The v3 patch restores the options_len equality check, so GRO declines aggregation (clearing same_flow) instead of reading past the shorter allocation; since the two packets belong to different TCP flows, declining aggregation is also the correct protocol behavior. I will post v3 once this discussion settles. Happy to share the capture or the parsing script if that is useful. -- Jean-Paul Sergent