Re: [PATCH net v3 7/8] selftests: net: tun: test more outer header mismatches of UDP tunnel GSO

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] selftests: net: tun: off-by-ETH_HLEN lengths in
  build_udptnl6_frame()
--

commit 5c13d992f1a4a8ebd5983197ac9a5424f5b3dd20
Author: Eric Dumazet <edumazet@kernel.org>

selftests: net: tun: test more outer header mismatches of UDP tunnel GSO

This commit adds tap selftests in tools/testing/selftests/net/tun.c to test
outer header validation for UDP tunnel GSO frames, including IPv6 outer
headers, mismatched address families, VLAN-tagged frames, and non-UDP outer
L4 protocols.

> diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> index cfeca53863ce2..a135d3aa10d18 100644
> --- a/tools/testing/selftests/net/tun.c
> +++ b/tools/testing/selftests/net/tun.c
> @@ -869,6 +869,103 @@ TEST_F(tun_vnet_udptnl_proto, outer_proto_link_type)
>  	ASSERT_EQ(ret, len);
>  }
>  
> +/* Build a 4in6 Geneve GSO frame whose outer ethertype is @l3proto. */
> +static int build_udptnl6_frame(uint8_t *buf, uint16_t l3proto,
> +			       int payload_len)
> +{
> +	int hlen = UDP_TUNNEL_GENEVE_4IN6_HDRLEN;
> +	uint8_t *outer_udph, *cur = buf;
> +	int pktlen;
> +
> +	memset(buf, 0, TUN_VNET_TNL_SIZE + hlen + payload_len);
> +
> +	cur += build_virtio_net_hdr_v1_hash_tunnel(cur, true, hlen, 1400,
> +						   AF_INET6, AF_INET);
> +	cur += build_eth(cur, l3proto, param_hwaddr_outer_dst,
> +			 param_hwaddr_outer_src);
> +
> +	pktlen = UDP_TUNNEL_GENEVE_4IN6_HDRLEN - ETH_HLEN -
> +		 sizeof(struct ipv6hdr) + payload_len;

[Severity: Medium]
Are hlen and pktlen in build_udptnl6_frame() off by ETH_HLEN (14 bytes)?

UDP_TUNNEL_GENEVE_4IN6_HDRLEN only counts one ETH_HLEN for the inner
Ethernet header encapsulated inside Geneve:

#define UDP_TUNNEL_GENEVE_4IN6_HDRLEN                               \
	(ETH_HLEN + sizeof(struct iphdr) + sizeof(struct ipv6hdr) + \
	 GENEVE_HLEN + 2 * sizeof(struct udphdr))

For comparison, the 4in4 counterpart in build_udptnl_frame() adds ETH_HLEN
to hlen for the outer Ethernet header and does not subtract ETH_HLEN when
computing pktlen:

	int hlen = ETH_HLEN + vlan_len + UDP_TUNNEL_GENEVE_4IN4_HDRLEN;
	...
	pktlen = UDP_TUNNEL_GENEVE_4IN4_HDRLEN - sizeof(struct iphdr) +
		 payload_len;

Because build_udptnl6_frame() emits both an outer Ethernet header and an
inner Ethernet header, leaving ETH_HLEN out of hlen and subtracting ETH_HLEN
from pktlen causes memset() to zero 14 bytes fewer than the returned frame
length (cur - buf), sets the virtio header hdr_len to 98 instead of 112
(which is smaller than csum_start at 104), and leaves the outer IPv6
payload_len, outer UDP length, and outer UDP checksum coverage 14 bytes too
short when outer_proto_ok_ipv6 and outer_proto_bad_family write the frame to
the tap device.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105115715.1466924-1-edumazet@kernel.org?part=7