netdev - Re: [PATCH bpf-next v2 6/6] selftests/bpf: fix file descriptor assertion in open

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <67b3e8e0928fc_c6df0294b8@willemb.c.googlers.com.notmuch>
Date: Mon, 17 Feb 2025 20:56:48 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Marcus Wichelmann <marcus.wichelmann@...zner-cloud.de>, 
 netdev@...r.kernel.org, 
 linux-kernel@...r.kernel.org, 
 bpf@...r.kernel.org, 
 linux-kselftest@...r.kernel.org
Cc: willemdebruijn.kernel@...il.com, 
 jasowang@...hat.com, 
 andrew+netdev@...n.ch, 
 davem@...emloft.net, 
 edumazet@...gle.com, 
 kuba@...nel.org, 
 pabeni@...hat.com, 
 ast@...nel.org, 
 daniel@...earbox.net, 
 andrii@...nel.org, 
 martin.lau@...ux.dev, 
 eddyz87@...il.com, 
 song@...nel.org, 
 yonghong.song@...ux.dev, 
 john.fastabend@...il.com, 
 kpsingh@...nel.org, 
 sdf@...ichev.me, 
 haoluo@...gle.com, 
 jolsa@...nel.org, 
 mykolal@...com, 
 shuah@...nel.org, 
 hawk@...nel.org, 
 marcus.wichelmann@...zner-cloud.de
Subject: Re: [PATCH bpf-next v2 6/6] selftests/bpf: fix file descriptor
 assertion in open_tuntap helper

Marcus Wichelmann wrote:
> The open_tuntap helper function uses open() to get a file descriptor for
> /dev/net/tun.
> 
> The open(2) manpage writes this about its return value:
> 
>   On success, open(), openat(), and creat() return the new file
>   descriptor (a nonnegative integer).  On error, -1 is returned and
>   errno is set to indicate the error.
> 
> This means that the fd > 0 assertion in the open_tuntap helper is
> incorrect and should rather check for fd >= 0.
> 
> When running the BPF selftests locally, this incorrect assertion was not
> an issue, but the BPF kernel-patches CI failed because of this:
> 
>   open_tuntap:FAIL:open(/dev/net/tun) unexpected open(/dev/net/tun):
>   actual 0 <= expected 0

Wow. What kind of environment is this that 0 is not assigned stdin.
 
> Signed-off-by: Marcus Wichelmann <marcus.wichelmann@...zner-cloud.de>

The code makes sense.

I suppose that if this condition can hit, then it can also affect
existing lwt_* tests and thus should be a fix to commit 43a7c3ef8a15
("selftests/bpf: Add lwt_xmit tests for BPF_REDIRECT"), sent
separately to bpf (not bpf-next)?

Since it's a test and no failure was reported so far, maybe fine
to just merge as part of this bpf-next series, not my call.

> ---
>  tools/testing/selftests/bpf/network_helpers.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index e1cfa1b37754..9b59bfd5d912 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -571,7 +571,7 @@ int open_tuntap(const char *dev_name, bool need_mac)
>  	struct ifreq ifr;
>  	int fd = open("/dev/net/tun", O_RDWR);
>  
> -	if (!ASSERT_GT(fd, 0, "open(/dev/net/tun)"))
> +	if (!ASSERT_GE(fd, 0, "open(/dev/net/tun)"))
>  		return -1;
>  
>  	ifr.ifr_flags = IFF_NO_PI | (need_mac ? IFF_TAP : IFF_TUN);
> -- 
> 2.43.0
>