Re: [net-next] selftests: net: csum: filter packets by source address and port

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete fix: the patch adds source-address/source-port
  filtering only to the PF_PACKET verification path…

--

selftests: net: csum: filter packets by source address and port

The csum receiver now checks the source address and source port in
recv_verify_packet_tcp(), recv_verify_packet_udp(), recv_verify_packet_ipv4()
and recv_verify_packet_ipv6(), skipping non-matching packets. It also adds a
20us inter-packet delay in do_tx() and drops a stale comment at the top of
the file.

> diff --git a/tools/testing/selftests/net/lib/csum.c b/tools/testing/selftests/net/lib/csum.c
> index e28884ce3ab39..139465054f85a 100644
> --- a/tools/testing/selftests/net/lib/csum.c
> +++ b/tools/testing/selftests/net/lib/csum.c

[ ... ]

> @@ -647,6 +649,9 @@ static int recv_verify_packet_udp(void *th, int len)
>  		return recv_verify_packet_udp_encap(udph + 1,
>  						    len - sizeof(*udph));
>  
> +	if (!cfg_zero_sum && udph->source != htons(cfg_port_src))
> +		return -1;
> +
>  	return recv_verify_csum(th, len, ntohs(udph->source), udph->check);
>  }

[Severity: High]
Does this cover the second receive path used by the UDP tests?

For cfg_proto == IPPROTO_UDP the test also opens a real SOCK_DGRAM socket in
recv_prepare_udp() and drains it in recv_udp(). That socket is only bound to
the local address and cfg_port_dst; there is no connect() to the sender and
recv_udp() uses plain recv() without inspecting the peer:

tools/testing/selftests/net/lib/csum.c:recv_prepare_udp() {
	fd = socket(cfg_family, SOCK_DGRAM, 0);
	...
	if (bind(fd, (void *)&cfg_daddr4, sizeof(cfg_daddr4)))
		error(1, errno, "bind r");
}

tools/testing/selftests/net/lib/csum.c:recv_udp() {
	while (1) {
		ret = recv(fd, buf, sizeof(buf), MSG_DONTWAIT);
		...
		count++;
	}
}

do_rx() then turns that count into a hard failure in exactly the -E case this
patch targets:

tools/testing/selftests/net/lib/csum.c:do_rx() {
	if (cfg_proto == IPPROTO_UDP)
		count_udp += recv_udp(fdr);
	...
	if (cfg_bad_csum && count_udp)
		error(1, 0, "rx: unexpected packets at udp");
}

The test's own crafted bad-checksum datagrams are dropped by the kernel UDP
checksum check, so count_udp is expected to stay zero. An unrelated
background datagram sent to the local address at cfg_port_dst with a valid
checksum is delivered to this socket and counted, and the test aborts. The
in-tree cases ipv4_rx_udp_invalid and ipv6_rx_udp_invalid in
tools/testing/selftests/drivers/net/hw/csum.py run csum with -E and UDP, so
they still appear exposed to background traffic after this change.

Would it make sense to also connect() the receive UDP socket to
cfg_saddr:cfg_port_src, or use recvfrom() and drop datagrams whose peer does
not match, with the same cfg_zero_sum exception used above since the source
port is computed at send time in that mode?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260831210128.1359978-1-willemdebruijn.kernel%40gmail.com