[Bug Report] fortify false-positive + real overread in skb_metadata_dst_cmp (geneve, gro_cell_poll)

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

Hi,

We are hitting a fortify false-positive (followed by a fatal Oops) in
skb_metadata_dst_cmp() on Cilium geneve overlay traffic, via the
gro_cells GRO path. Full trace from the serial console:

  memcmp: detected buffer overflow: 108 byte read of buffer size 96
  WARNING: CPU: 0 PID: 15 at lib/string_helpers.c:1036 __fortify_report+0x45/0x60
  Modules linked in: usbhid virtio_net virtio_scsi virtio_console psmouse uhci_hcd virtio_pci virtio_pci_legacy_dev vmgenid ehci_pci ehci_hcd virtio_pci_modern_dev ata_piix button
  CPU: 0 UID: 0 PID: 15 Comm: ksoftirqd/0 Not tainted 6.18.54-talos #1 PREEMPT(none)
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
  RIP: 0010:__fortify_report+0x45/0x60
  Call Trace:
   <TASK>
   __fortify_panic+0x9/0x10
   skb_metadata_dst_cmp+0x11b/0x120
   dev_gro_receive+0x303/0x620
   gro_receive_skb+0xc5/0x230
   gro_cell_poll+0x67/0xa0
   __napi_poll+0x2f/0x190
   net_rx_action+0x2e3/0x500
   handle_softirqs+0xe7/0x310
   run_ksoftirqd+0x26/0x50
   smpboot_thread_fn+0x167/0x250
   kthread+0x201/0x260
   ret_from_fork+0x10c/0x190
   </TASK>
  ---[ end trace 0000000000000000 ]---

The WARNING is immediately followed by a fatal Oops (kernel BUG at
lib/string_helpers.c:1043, "Oops: invalid opcode: 0000 [#1] SMP PTI"),
which takes the node down. We caught the same crash twice: once in
ksoftirqd/0 after ~11.5h of uptime, once in IRQ context (Comm:
containerd-shim, dev_gro_receive reached straight from the IP receive
path) ~14 minutes into a heavy-traffic boot, same signature both
times.

Environment: 12-node Talos v1.14.2 cluster (kernel 6.18.54-talos,
built with clang + CONFIG_FORTIFY_SOURCE) as QEMU/KVM guests
(i440FX, vmxnet3 NICs), Cilium 1.18 with geneve encapsulation and
kube-proxy replacement. Workload at crash time: sustained cross-node
overlay traffic (BitTorrent + NFS + Cilium service traffic).

skb_metadata_dst_cmp() does a single memcmp of struct ip_tunnel_info
plus options:

  memcmp(&a->u.tun_info, &b->u.tun_info,
         sizeof(a->u.tun_info) + a->u.tun_info.options_len);

Geneve carries 108 bytes of options here, but kmalloc_flex() in
metadata_dst_alloc() sets __counted_by(options_len) while options_len
is still 0 at comparison time, so the compiler's view of the tail is
96 bytes -> 108-byte read of a 96-byte view. Same disease as the
tun_dst_unclone fix 4c6d43db2a4d ("net: dst_metadata: fix
false-positive memcpy overflow in tun_dst_unclone"), on the RX/GRO
sibling path; that fix carries Fixes: 69050f8d6d07 for the same
reason.

While reading the function I believe there is also a real overread,
independent of fortify: the memcmp length uses a->options_len for
BOTH sides, so when b's metadata carries fewer options than a's, the
comparison reads past b's allocation. In the geneve/GRO case both
sides come from the same tunnel and carry equal option lengths, but
the helper is generic (any METADATA_IP_TUNNEL comparison).

A patch follows: pre-check options_len equality, compare the fixed
part, then compare options via ip_tunnel_info_opts() on both sides so
the counted_by view matches the read length - the same two-stage
shape as 4c6d43db2a4d.

NOTE: I could not build-test the fix (no kernel build environment on
the reporting host). The reproducer is the production workload itself
(sustained cross-node geneve RX under GRO): with GRO left enabled the
node panicked twice; since disabling GRO offload on the geneve device
(ethtool -K cilium_geneve gro off) as a stopgap, the same workload has
run clean - consistent with the crash living in the geneve device's
gro_cells stage.

-- 
Jean-Paul Sergent