Re: [PATCH net v2] net: dst_metadata: fix fortify trap + overread in skb_metadata_dst_cmp

[Jean-Paul Sergent <jpsergent@gmail.com>]

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