Thread (17 messages) 17 messages, 5 authors, 1d ago

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

flat view

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2026-10-06 22:37:53

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/ (local)

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/ (local)

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
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help