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
