Re: [PATCH net-next 07/14] selftests/bpf: Test skb_ext read from cgroup_skb and sk_filter hooks

[email protected]
Newsgroups org.kernel.vger.bpf
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Implicit truncation of error code to unsigned short masks get_socket_local_port() failure.
--

--- Patch [7]: [PATCH net-next 07/14] selftests/bpf: Test skb_ext read from cgroup_skb and sk_filter hooks ---
commit b524b9baad48611cd0f3fbe2f1b770ea85d7c5df
Author: Jakub Sitnicki <[email protected]>

selftests/bpf: Test skb_ext read from cgroup_skb and sk_filter hooks

This commit extends the skb_ext cross-hook coverage to UDP readers. It attaches
a TC ingress program on loopback to write the extension, sends a UDP datagram,
and reads the extension back from a cgroup_skb/ingress program and a socket
filter.

> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
> index de60303025361..808eb158fe0f1 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
[ ... ]
> +/* Send test_payload over loopback UDP to recv_fd */
> +static int send_loopback_udp(int recv_fd)
> +{
> +	struct sockaddr_in addr = {
> +		.sin_family = AF_INET,
> +		.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
> +	};
> +	char buf[TEST_PAYLOAD_LEN];
> +	int ret = -1;
> +	int fd = -1;
> +	__be16 port;
> +
> +	port = get_socket_local_port(recv_fd);
> +	if (!ASSERT_GE(port, 0, "get_port"))
> +		goto out;

[Severity: Medium]
Will this error check ever trigger if get_socket_local_port() fails?

Because port is declared as a __be16 (an unsigned 16-bit integer), a -1 return
value from get_socket_local_port() would be implicitly truncated and converted
to 65535.

The ASSERT_GE(port, 0, ...) check would then compare 65535 >= 0, which is always
true. This masks the failure and allows the test to proceed with an invalid
port.

Should the return value be captured in an int variable for the error check
before assigning it to the __be16 port?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814-bpf-meta-inside-skb-ext-v1-0-767edd862656@cloudflare.com?part=7
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.