Re: [PATCH bpf-next v2] selftests/bpf: fix the incorrect verification of port numbers.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/4/23 9:58 AM, thinker.li@xxxxxxxxx wrote:
From: Kui-Feng Lee <thinker.li@xxxxxxxxx>

Check port numbers before calling htons().

According to Dan Carpenter's report, Smatch identified incorrect port
number checks. It is expected that the returned port number is an integer,
with negative numbers indicating errors. However, the value was mistakenly
verified after being translated by htons().

Major changes from v1:

  - Move the variable 'port' to the same line of 'err'.

Fixes: 539c7e67aa4a ("selftests/bpf: Verify that the cgroup_skb filters receive expected packets.")
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Closes: https://lore.kernel.org/bpf/cafd6585-d5a2-4096-b94f-7556f5aa7737@moroto.mountain/
Acked-by: Yonghong Song <yonghong.song@xxxxxxxxx>
Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
---
  tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c b/tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c
index 95bab61a1e57..d686ef19f705 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c
@@ -110,11 +110,12 @@ static int connect_client_server_v6(int client_fd, int listen_fd)
  		.sin6_family = AF_INET6,
  		.sin6_addr = IN6ADDR_LOOPBACK_INIT,
  	};
-	int err;
+	int err, port;
- addr.sin6_port = htons(get_sock_port_v6(listen_fd));
-	if (addr.sin6_port < 0)
+	port = get_sock_port_v6(listen_fd);
+	if (port < 0)

Applied. Some follow up questions:

Does other get_sock_port_v6() usage need to check -1 also?

It is a good idea to see if similar helpers exist in network_helpers.c e.g. There is get_socket_local_port() that supports both v4 and v6 in network_helpers.c which is equivalent to the get_sock_port_v6() here. To a larger extent, I believe many codes in this new test can be saved by using the helpers in network_helpers.c. For example, the connect_client_server_v6() here can be replaced with connect_to_fd() from network_helpers.c to avoid the mistake this patch fixing. It also has some timeout limit on the socket such that it won't block the test_progs for a long time.




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux