* [PATCH bpf-next] selftests/bpf: fix the incorrect verification of port numbers.
@ 2023-08-03 21:53 thinker.li
2023-08-04 1:30 ` Yonghong Song
0 siblings, 1 reply; 3+ messages in thread
From: thinker.li @ 2023-08-03 21:53 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii, dan.carpenter
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
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().
Fixes: 8a8c2231cab2 ("selftests/bpf: fix the incorrect verification of port numbers.")
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/bpf/cafd6585-d5a2-4096-b94f-7556f5aa7737@moroto.mountain/
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c | 6 ++++--
1 file changed, 4 insertions(+), 2 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..0df95bc88a9b 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,13 @@ static int connect_client_server_v6(int client_fd, int listen_fd)
.sin6_family = AF_INET6,
.sin6_addr = IN6ADDR_LOOPBACK_INIT,
};
+ int port;
int err;
- 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)
return -1;
+ addr.sin6_port = htons(port);
err = connect(client_fd, (struct sockaddr *)&addr, sizeof(addr));
if (err < 0) {
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH bpf-next] selftests/bpf: fix the incorrect verification of port numbers.
2023-08-03 21:53 [PATCH bpf-next] selftests/bpf: fix the incorrect verification of port numbers thinker.li
@ 2023-08-04 1:30 ` Yonghong Song
2023-08-04 16:14 ` Kui-Feng Lee
0 siblings, 1 reply; 3+ messages in thread
From: Yonghong Song @ 2023-08-04 1:30 UTC (permalink / raw)
To: thinker.li, bpf, ast, martin.lau, song, kernel-team, andrii,
dan.carpenter
Cc: sinquersw, kuifeng
On 8/3/23 2:53 PM, thinker.li@gmail.com wrote:
> From: Kui-Feng Lee <thinker.li@gmail.com>
>
> 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().
>
> Fixes: 8a8c2231cab2 ("selftests/bpf: fix the incorrect verification of port numbers.")
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/bpf/cafd6585-d5a2-4096-b94f-7556f5aa7737@moroto.mountain/
> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
Ack with a small nit below.
Acked-by: Yonghong Song <yonghong.song@linux.dev>
> ---
> tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 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..0df95bc88a9b 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,13 @@ static int connect_client_server_v6(int client_fd, int listen_fd)
> .sin6_family = AF_INET6,
> .sin6_addr = IN6ADDR_LOOPBACK_INIT,
> };
> + int port;
> int err;
No need for a separate line for 'int port'.
Just doing 'int err, port;' sounds better.
>
> - 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)
> return -1;
> + addr.sin6_port = htons(port);
>
> err = connect(client_fd, (struct sockaddr *)&addr, sizeof(addr));
> if (err < 0) {
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH bpf-next] selftests/bpf: fix the incorrect verification of port numbers.
2023-08-04 1:30 ` Yonghong Song
@ 2023-08-04 16:14 ` Kui-Feng Lee
0 siblings, 0 replies; 3+ messages in thread
From: Kui-Feng Lee @ 2023-08-04 16:14 UTC (permalink / raw)
To: yonghong.song, thinker.li, bpf, ast, martin.lau, song,
kernel-team, andrii, dan.carpenter
Cc: kuifeng
On 8/3/23 18:30, Yonghong Song wrote:
>
>
> On 8/3/23 2:53 PM, thinker.li@gmail.com wrote:
>> From: Kui-Feng Lee <thinker.li@gmail.com>
>>
>> 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().
>>
>> Fixes: 8a8c2231cab2 ("selftests/bpf: fix the incorrect verification of
>> port numbers.")
>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>> Closes:
>> https://lore.kernel.org/bpf/cafd6585-d5a2-4096-b94f-7556f5aa7737@moroto.mountain/
>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
>
> Ack with a small nit below.
>
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>
>> ---
>> tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 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..0df95bc88a9b 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,13 @@ static int connect_client_server_v6(int
>> client_fd, int listen_fd)
>> .sin6_family = AF_INET6,
>> .sin6_addr = IN6ADDR_LOOPBACK_INIT,
>> };
>> + int port;
>> int err;
>
> No need for a separate line for 'int port'.
> Just doing 'int err, port;' sounds better.
Got it! Thanks!
>
>> - 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)
>> return -1;
>> + addr.sin6_port = htons(port);
>> err = connect(client_fd, (struct sockaddr *)&addr, sizeof(addr));
>> if (err < 0) {
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-04 16:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-03 21:53 [PATCH bpf-next] selftests/bpf: fix the incorrect verification of port numbers thinker.li
2023-08-04 1:30 ` Yonghong Song
2023-08-04 16:14 ` Kui-Feng Lee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox