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 <redacted>
Suggested-by: Willem de Bruijn <willemb@google.com>
Link: https://lore.kernel.org/netdev/93d0e37c99711bebdf60c120af03010455c2f2e2.1793276116.git.mst@redhat.com/ (local)
Cc: Willem de Bruijn <willemb@google.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Co-developed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Reviewed-by: Willem de Bruijn <willemb@google.com>
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.