Re: [PATCH v3 net 0/3] net: always dissect GSO packets in __virtio_net_hdr_to_skb()

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Oct 01, 2026 at 07:11:37PM +0000, Eric Dumazet wrote:
> This series fixes a bypass of untrusted GSO flow dissection in
> __virtio_net_hdr_to_skb() when VIRTIO_NET_HDR_F_NEEDS_CSUM is not set,
> and adds a kselftest covering VLAN-tagged GSO packets without NEEDS_CSUM:
> 
> - Patch 1 fixes __skb_flow_dissect(), which computes key_control->thoff
>   with min_t(u16, ...). This truncates skb->len and returns a bogus small
>   transport offset when skb->len modulo 65536 is smaller than the
>   transport offset. Offsets that do not fit in the u16 thoff now fail the
>   dissection instead of being silently truncated.
> 
> - Patch 2 initializes skb->dev and skb->network_header before calling
>   virtio_net_hdr_*_to_skb() in tun_get_user(), tun_xdp_one(),
>   virtnet_receive_done(), and raw_verify_header(), removes the
>   '&& skb->network_header' condition and the unvalidated
>   'else if (gso_type)' fallback in __virtio_net_hdr_to_skb(), and moves
>   virtio_net_hdr_match_proto() after skb_flow_dissect_flow_keys_basic()
>   so it validates the dissected L3 protocol (keys.basic.n_proto) rather
>   than the outer L2 protocol.
> 
> - Patch 3 adds kselftests in tools/testing/selftests/net/tun.c verifying
>   that VLAN-tagged (802.1Q) TCPv4 GSO packets without NEEDS_CSUM (both
>   flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID) are accepted on a
>   TAP device, that mismatched GSO types and truncated TCP headers without
>   NEEDS_CSUM are rejected with -EINVAL, and that a 65540-byte frame is
>   accepted.
> 
> v3:
>  - New patch 1: avoid u16 truncation of skb->len when computing thoff in
>    __skb_flow_dissect(). Patch 2 makes tun_get_user() dissect IFF_TAP
>    frames before eth_type_trans(), with skb->len up to 65549 for a GSO
>    frame carrying a maximal IPv4 packet (Sashiko).
>  - Patch 3: truncate the TCP header after 10 bytes so that the test
>    requires the transport offset found by flow dissection, and add a
>    65540-byte frame test (Sashiko).
>  - Link to v2: https://lore.kernel.org/netdev/20260928144254.3361044-1-edumazet@kernel.org/
> 
> v2:
>  - Patch 2: drop the pre-dissection virtio_net_hdr_match_proto() check
>    inside 'if (!skb->protocol)' so VLAN-tagged GSO frames without
>    NEEDS_CSUM are not rejected before flow dissection (Michael S. Tsirkin).
>  - Patch 2: clarify the changelog regarding why skb->network_header was 0
>    in those callers and why skb_reset_mac_header() is dropped in
>    tun_get_user() for IFF_TUN (Michael S. Tsirkin).
>  - Patch 3: add selftest in tools/testing/selftests/net/tun.c based on
>    Michael's reproducer.
>  - Link to v1: https://lore.kernel.org/netdev/20260927195536.2489079-1-edumazet@google.com/


Not without trepidation about the amount of stuff we are shoving
into virtio_net_hdr_to_skb which, believe me or not, used to be 50 LOC
of trivial code in 2019:

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> Eric Dumazet (3):
>   flow_dissector: avoid u16 truncation of skb->len when computing thoff
>   net: always dissect GSO packets in __virtio_net_hdr_to_skb()
>   selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM
> 
>  arch/um/drivers/vector_transports.c |   1 +
>  drivers/net/tun.c                   |  23 +++--
>  drivers/net/virtio_net.c            |   2 +
>  include/linux/virtio_net.h          |  60 +++++-------
>  net/core/flow_dissector.c           |   9 +-
>  tools/testing/selftests/net/tun.c   | 136 ++++++++++++++++++++++++++++
>  6 files changed, 186 insertions(+), 45 deletions(-)
> 
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog