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 outer headers. 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
>
> Validate the outer headers with the parsing helpers from Michael's
> series "net: resolve uncommon partial checksums at ingress":
>
> - locate the L3 header from the device type: Ethernet with optional
> VLAN tags, or the network header set by the caller. skb->protocol is
> not trusted when it disagrees with the frame.
> - require the outer L3 protocol to match the declared tunnel type.
> - require the outer UDP header exactly at outer_th_offset, after IPv4
> options or IPv6 extension headers, and reject fragments.
>
> Unlike the original helpers, ipv6_find_hdr() stops at an AH header,
> as Willem suggested in the review of that series.
>
> Fixes: a2fb4bc4e2a6 ("net: implement virtio helpers to handle UDP GSO
> tunneling.")
> Reported-by: Hassan Sultan <[email protected]>
> Suggested-by: Willem de Bruijn <[email protected]>
> Link:
> https://lore.kernel.org/netdev/93d0e37c99711bebdf60c120af03010455c2f2e2.1793276116.git....@redhat.com/
> Cc: Willem de Bruijn <[email protected]>
> Cc: Michael S. Tsirkin <[email protected]>
> Co-developed-by: Michael S. Tsirkin <[email protected]>
> Signed-off-by: Michael S. Tsirkin <[email protected]>
> Signed-off-by: Eric Dumazet <[email protected]>
Reviewed-by: Willem de Bruijn <[email protected]>
Thanks Eric!
My bot points out a preexisting issue with this udp tunnel feature.
_Not_ reason for changing this patch.
virtio_net_hdr_tnl_to_skb already does not support IFF_TUN, as it has
outer_l3min = ETH_HLEN + virtio_l3min(outer_isv6);
So the only reachable path with IFF_TUN is L3 with IPV4_OPT_LEN 16
(or greater) as indeed test patch 3/3 introduces.
