[bug report] selftests/bpf: Verify that the cgroup_skb filters receive expected packets.

[bug report] selftests/bpf: Verify that the cgroup_skb filters receive expected packets.

[Dan Carpenter]

Hello Kui-Feng Lee,

The patch 539c7e67aa4a: "selftests/bpf: Verify that the cgroup_skb
filters receive expected packets." from Jun 23, 2023 (linux-next),
leads to the following Smatch static checker warning:

	./tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c:116 connect_client_server_v6()
	warn: unsigned 'addr.sin6_port' is never less than zero.

./tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c
    107 static int connect_client_server_v6(int client_fd, int listen_fd)
    108 {
    109         struct sockaddr_in6 addr = {
    110                 .sin6_family = AF_INET6,
    111                 .sin6_addr = IN6ADDR_LOOPBACK_INIT,
    112         };
    113         int err;
    114 
    115         addr.sin6_port = htons(get_sock_port_v6(listen_fd));
--> 116         if (addr.sin6_port < 0)
                    ^^^^^^^^^^^^^^^^^^
Impossible and also it doesn't make sense to compare network endian data
with < 0.

    117                 return -1;
    118 
    119         err = connect(client_fd, (struct sockaddr *)&addr, sizeof(addr));
    120         if (err < 0) {
    121                 perror("connect");
    122                 return -1;
    123         }
    124 
    125         return 0;
    126 }

regards,
dan carpenter

Re: [bug report] selftests/bpf: Verify that the cgroup_skb filters receive expected packets.

[Kui-Feng Lee]

On 7/31/23 00:27, Dan Carpenter wrote:
> Hello Kui-Feng Lee,
> 
> The patch 539c7e67aa4a: "selftests/bpf: Verify that the cgroup_skb
> filters receive expected packets." from Jun 23, 2023 (linux-next),
> leads to the following Smatch static checker warning:
> 
> 	./tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c:116 connect_client_server_v6()
> 	warn: unsigned 'addr.sin6_port' is never less than zero.
> 
> ./tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c
>      107 static int connect_client_server_v6(int client_fd, int listen_fd)
>      108 {
>      109         struct sockaddr_in6 addr = {
>      110                 .sin6_family = AF_INET6,
>      111                 .sin6_addr = IN6ADDR_LOOPBACK_INIT,
>      112         };
>      113         int err;
>      114
>      115         addr.sin6_port = htons(get_sock_port_v6(listen_fd));
> --> 116         if (addr.sin6_port < 0)
>                      ^^^^^^^^^^^^^^^^^^
> Impossible and also it doesn't make sense to compare network endian data
> with < 0.

Hi Dan,

Thank you for pointing it out. It should check the returned value
of get_sock_port_v6() before calling htons(). I will send a patch
to fix it asap.


> 
>      117                 return -1;
>      118
>      119         err = connect(client_fd, (struct sockaddr *)&addr, sizeof(addr));
>      120         if (err < 0) {
>      121                 perror("connect");
>      122                 return -1;
>      123         }
>      124
>      125         return 0;
>      126 }
> 
> regards,
> dan carpenter
> 

Re: [bug report] selftests/bpf: Verify that the cgroup_skb filters receive expected packets.

[Kui-Feng Lee]

On 8/3/23 12:52, Kui-Feng Lee wrote:
> 
> 
> On 7/31/23 00:27, Dan Carpenter wrote:
>> Hello Kui-Feng Lee,
>>
>> The patch 539c7e67aa4a: "selftests/bpf: Verify that the cgroup_skb
>> filters receive expected packets." from Jun 23, 2023 (linux-next),
>> leads to the following Smatch static checker warning:
>>
>>     ./tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c:116 
>> connect_client_server_v6()
>>     warn: unsigned 'addr.sin6_port' is never less than zero.
>>
>> ./tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c
>>      107 static int connect_client_server_v6(int client_fd, int 
>> listen_fd)
>>      108 {
>>      109         struct sockaddr_in6 addr = {
>>      110                 .sin6_family = AF_INET6,
>>      111                 .sin6_addr = IN6ADDR_LOOPBACK_INIT,
>>      112         };
>>      113         int err;
>>      114
>>      115         addr.sin6_port = htons(get_sock_port_v6(listen_fd));
>> --> 116         if (addr.sin6_port < 0)
>>                      ^^^^^^^^^^^^^^^^^^
>> Impossible and also it doesn't make sense to compare network endian data
>> with < 0.
> 
> Hi Dan,
> 
> Thank you for pointing it out. It should check the returned value
> of get_sock_port_v6() before calling htons(). I will send a patch
> to fix it asap.

Could you show me how to run Smatch againt bpf selftests?

> 
> 
>>
>>      117                 return -1;
>>      118
>>      119         err = connect(client_fd, (struct sockaddr *)&addr, 
>> sizeof(addr));
>>      120         if (err < 0) {
>>      121                 perror("connect");
>>      122                 return -1;
>>      123         }
>>      124
>>      125         return 0;
>>      126 }
>>
>> regards,
>> dan carpenter
>>

Re: [bug report] selftests/bpf: Verify that the cgroup_skb filters receive expected packets.

[Dan Carpenter]

On Thu, Aug 03, 2023 at 01:43:33PM -0700, Kui-Feng Lee wrote:
> > >      113         int err;
> > >      114
> > >      115         addr.sin6_port = htons(get_sock_port_v6(listen_fd));
> > > --> 116         if (addr.sin6_port < 0)
> > >                      ^^^^^^^^^^^^^^^^^^
> > > Impossible and also it doesn't make sense to compare network endian data
> > > with < 0.
> > 
> > Hi Dan,
> > 
> > Thank you for pointing it out. It should check the returned value
> > of get_sock_port_v6() before calling htons(). I will send a patch
> > to fix it asap.
> 
> Could you show me how to run Smatch againt bpf selftests?
> 

Oh wow...  You don't want to know.  So sometimes I'll go through and
do a `find -name \*.c` and if there isn't a matching .o file I'll just
run:

	~/path/to/smatch tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c

Smatch has a thing where if the .h file is missing it will just include
the nearest .h file with a similar name.  This doesn't work well and
generates a ton of errors.  But I grep the output for specific types
of errors like "is never less than zero".

regards,
dan carpenter

Re: [bug report] selftests/bpf: Verify that the cgroup_skb filters receive expected packets.

[Kui-Feng Lee]

On 8/3/23 22:13, Dan Carpenter wrote:
> On Thu, Aug 03, 2023 at 01:43:33PM -0700, Kui-Feng Lee wrote:
>>>>       113         int err;
>>>>       114
>>>>       115         addr.sin6_port = htons(get_sock_port_v6(listen_fd));
>>>> --> 116         if (addr.sin6_port < 0)
>>>>                       ^^^^^^^^^^^^^^^^^^
>>>> Impossible and also it doesn't make sense to compare network endian data
>>>> with < 0.
>>>
>>> Hi Dan,
>>>
>>> Thank you for pointing it out. It should check the returned value
>>> of get_sock_port_v6() before calling htons(). I will send a patch
>>> to fix it asap.
>>
>> Could you show me how to run Smatch againt bpf selftests?
>>
> 
> Oh wow...  You don't want to know.  So sometimes I'll go through and
> do a `find -name \*.c` and if there isn't a matching .o file I'll just
> run:
> 
> 	~/path/to/smatch tools/testing/selftests/bpf/prog_tests/cgroup_tcp_skb.c
> 
> Smatch has a thing where if the .h file is missing it will just include
> the nearest .h file with a similar name.  This doesn't work well and
> generates a ton of errors.  But I grep the output for specific types
> of errors like "is never less than zero".

Got it! Thanks!

> 
> regards,
> dan carpenter
>