Le mer. 4 nov. 2026 à 03:01, Willem de Bruijn <[email protected]> a écrit : > > Eric Dumazet wrote: > > virtio_net_hdr_tnl_to_skb() trusts the virtio header to describe a > > UDP tunnel over IPv4 or IPv6, but never checks that the frame actually > > carries such an outer header. Because VIRTIO_NET_HDR_F_NEEDS_CSUM is > > mandatory for tunnel GSO, __virtio_net_hdr_to_skb() skips the flow > > dissector, so virtio_net_hdr_match_proto() is never applied either. > > > > A guest (or a tap writer) can then inject a frame with an arbitrary > > outer ethertype, for example ETH_P_NSH, with SKB_GSO_UDP_TUNNEL set. > > The GSO stack then runs nsh_gso_segment() on top of the UDP tunnel > > segmentation, which assumes a different header layout. The result is > > segments with skb->data before skb->head, and a host crash in > > skb_under_panic() as soon as a later layer pushes a header: > > > > skbuff: skb_under_panic: ... put:4 ... dev:gretap1 > > kernel BUG at net/core/skbuff.c:215! > > skb_push > > __gre_xmit > > gre_tap_xmit > > ... > > tun_get_user > > > > Require the outer L3 protocol to match the declared tunnel type: > > for Ethernet frames, look at the ethertype after optional VLAN tags, > > and check that the outer network header fits before outer_th_offset. > > For IFF_TUN, skb->protocol is already set by the caller. > > > > Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO > > tunneling.") > > Reported-by: Hassan Sultan <[email protected]> > > Signed-off-by: Eric Dumazet <[email protected]> > > Cc: Willem de Bruijn <[email protected]> > > Cc: Michael S. Tsirkin <[email protected]> > > Thanks for the fix. It overlaps with > > commit "net: resolve uncommon partial checksums at ingress" > https://lore.kernel.org/netdev/93d0e37c99711bebdf60c120af03010455c2f2e2.1793276116.git....@redhat.com/ > > There are a bunch of subtle issues here. That series is up to v7. > > Two specific issues are that > - skb->protocol in general cannot be trusted, and > - non-L3 sources cannot be assumed Ethernet
Agreed on both. With IFF_TUN and a tun_pi header, skb->protocol is whatever userspace put in pi.proto, and v1 would accept an IPv4 tunnel frame whose IP version says otherwise. > > Would it make sense to take the relevant parts from the quoted commit, > which has hopefully resolved those edge cases, and send that on its own? > The rest of the series can come later. Done in v2: patch 1 now uses virtio_net_l3_start(), virtio_net_l4_start(), virtio_net_ip4_parse() and virtio_net_tnl_offsets_ok() from Michael's v7 4/5, unchanged except that ipv6_find_hdr() stops at AH, as you suggested in your v7 review. The checksum validation/completion parts and the non-GSO carve-out are left to Michael's series. Michael, I added your Co-developed-by and Signed-off-by, since the code is yours; please let me know if you prefer a different attribution. v2 also adds IFF_TUN tests with a mismatching tun_pi protocol, and addresses your comments on patches 2 and 3. One side note: virtio_net_hdr_tnl_to_skb() uses outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6) for IFF_TUN too, so tunnel GSO frames without an Ethernet header are rejected unless the outer IP header carries at least 14 bytes of options (the tests pad with IPv4 options for that reason). This predates the series, I left it alone for now. Thanks! pw-bot: cr
