Re: [PATCH bpf-next v7 5/7] selftests/bpf: add bpf_icmp_send kfunc cgroup_skb IPv6 tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Type confusion and address truncation in getsockname() due to using struct sockaddr_in for IPv6 addresses.
--

commit 0cc9d08ecac1ed3d21b7e5cbf50d68e27cb610de
Author: Mahe Tardy <mahe.tardy@gmail.com>

selftests/bpf: add bpf_icmp_send kfunc cgroup_skb IPv6 tests

This test extends the existing cgroup_skb tests for the bpf_icmp_send
kfunc by adding IPv6 support.

> diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
> index 0dc6b6ceafb4a..1d6900d6a8f8c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c

[ ... ]

> @@ -79,14 +94,15 @@ static void read_icmp_errqueue(int sockfd, int expected_code)
>  	ASSERT_FAIL("no IP_RECVERR/IPV6_RECVERR control message found");
>  }
>  
> -static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code)
> +static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code,
> +					    int af, const char *ip)
>  {
>  	int srv_fd = -1, client_fd = -1;
>  	struct sockaddr_in addr;
>  	socklen_t len = sizeof(addr);
>  
> -	srv_fd = start_server(AF_INET, SOCK_STREAM, "127.0.0.1", 0, TIMEOUT_MS);
> -	if (!ASSERT_GE(srv_fd, 0, "start_server"))
> +	srv_fd = start_server(af, SOCK_STREAM, ip, 0, TIMEOUT_MS);
> +	if (!ASSERT_OK_FD(srv_fd, "start_server"))
>  		return;
>  
>  	if (getsockname(srv_fd, (struct sockaddr *)&addr, &len)) {
> @@ -94,6 +110,8 @@ static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code)
>  		return;
>  	}
>  	skel->bss->server_port = ntohs(addr.sin_port);

[Severity: Medium]
When af is AF_INET6, does this cause an address truncation in getsockname()
since addr is defined as struct sockaddr_in?

For an IPv6 socket, getsockname() requires struct sockaddr_in6 which is
larger than struct sockaddr_in. While reading addr.sin_port happens to map
to the same memory offset as sin6_port, could this lead to type confusion
if the address fields are ever used?

Would it be safer to use struct sockaddr_storage here to ensure enough space
for either address family, similar to what is used in connect_to_fd_nonblock()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526153708.279717-1-mahe.tardy@gmail.com?part=5