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

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

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

The receiver process uses a PF_PACKET socket to verify incoming packets
and test hardware checksum offload. While the socket filter matches the
protocol and destination port, packet verification did not validate the
packet source address or source port.

If background traffic arrives at the destination port from an unrelated
sender, the receiver attempts to verify it. During tests expecting an
invalid checksum (-E), any legitimate background packet with a valid
checksum causes the receiver to report a checksum error and fail the test.

Add source address and source port verification in recv_verify_packet_*:
Non-matching packets return -1 and are skipped.

Also
- add an inter-packet delay to avoid drops from bursts.
- remove a comment that is no longer correct.

Fixes: 91a7de85600d ("selftests/net: add csum offload test")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 tools/testing/selftests/net/lib/csum.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/lib/csum.c b/tools/testing/selftests/net/lib/csum.c
index e28884ce3ab3..139465054f85 100644
--- a/tools/testing/selftests/net/lib/csum.c
+++ b/tools/testing/selftests/net/lib/csum.c
@@ -2,8 +2,7 @@
 
 /* Test hardware checksum offload: Rx + Tx, IPv4 + IPv6, TCP + UDP.
  *
- * The test runs on two machines to exercise the NIC. For this reason it
- * is not integrated in kselftests.
+ * The test runs on two machines to exercise the NIC.
  *
  *     CMD=$((./csum -[46] -[tu] -S $SADDR -D $DADDR -[RT] -r 1 $EXTRA_ARGS))
  *
@@ -620,6 +619,9 @@ static int recv_verify_packet_tcp(void *th, int len)
 	if (len < sizeof(*tcph) || tcph->dest != htons(cfg_port_dst))
 		return -1;
 
+	if (tcph->source != htons(cfg_port_src))
+		return -1;
+
 	return recv_verify_csum(th, len, ntohs(tcph->source), tcph->check);
 }
 
@@ -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);
 }
 
@@ -659,6 +664,9 @@ static int recv_verify_packet_ipv4(void *nh, int len)
 	if (len < sizeof(*iph) || iph->protocol != proto)
 		return -1;
 
+	if (iph->saddr != cfg_saddr4.sin_addr.s_addr)
+		return -1;
+
 	ip_len = ntohs(iph->tot_len);
 	if (ip_len > len || ip_len < sizeof(*iph))
 		return -1;
@@ -680,6 +688,9 @@ static int recv_verify_packet_ipv6(void *nh, int len)
 	if (len < sizeof(*ip6h) || ip6h->nexthdr != proto)
 		return -1;
 
+	if (memcmp(&ip6h->saddr, &cfg_saddr6.sin6_addr, sizeof(ip6h->saddr)))
+		return -1;
+
 	payload_len = ntohs(ip6h->payload_len);
 	if (payload_len > len - sizeof(*ip6h))
 		return -1;
@@ -940,6 +951,9 @@ static void do_tx(void)
 			cfg_payload_len = rand() % MAX_PAYLOAD_LEN;
 			buf = build_packet(_buf, sizeof(_buf), &len);
 		}
+
+		/* avoid bursting */
+		usleep(20);
 	}
 
 	if (close(fd))
-- 
2.55.0.897.gb25b4bd76c-goog

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

[netdev-bot+sashiko]

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

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

[Willem de Bruijn]

netdev-bot+sashiko@ wrote:
> 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…
> 
> 
> 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?

That's a great suggestion. I'll add recvfrom() src verification in v2.