diff mbox series

[bpf-next] libbpf: keep FD_CLOEXEC flag when dup()'ing FD

Message ID 20240529223239.504241-1-andrii@kernel.org (mailing list archive)
State Accepted
Commit 531876c80004ecff7bfdbd8ba6c6b48835ef5e22
Delegated to: BPF
Headers show
Series [bpf-next] libbpf: keep FD_CLOEXEC flag when dup()'ing FD | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
netdev/tree_selection success Clearly marked for bpf-next
netdev/apply success Patch already applied to bpf-next-0

Commit Message

Andrii Nakryiko May 29, 2024, 10:32 p.m. UTC 
Make sure to preserve and/or enforce FD_CLOEXEC flag on duped FDs.
Use dup3() with O_CLOEXEC flag for that.

Without this fix libbpf effectively clears FD_CLOEXEC flag on each of BPF
map/prog FD, which is definitely not the right or expected behavior.

Reported-by: Lennart Poettering <lennart@poettering.net>
Fixes: bc308d011ab8 ("libbpf: call dup2() syscall directly")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf_internal.h | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

Jiri Olsa May 30, 2024, 7:12 a.m. UTC | #1 
On Wed, May 29, 2024 at 03:32:39PM -0700, Andrii Nakryiko wrote:
> Make sure to preserve and/or enforce FD_CLOEXEC flag on duped FDs.
> Use dup3() with O_CLOEXEC flag for that.
> 
> Without this fix libbpf effectively clears FD_CLOEXEC flag on each of BPF
> map/prog FD, which is definitely not the right or expected behavior.

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> 
> Reported-by: Lennart Poettering <lennart@poettering.net>
> Fixes: bc308d011ab8 ("libbpf: call dup2() syscall directly")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  tools/lib/bpf/libbpf_internal.h | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index a0dcfb82e455..7e7e686008c6 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -597,13 +597,9 @@ static inline int ensure_good_fd(int fd)
>  	return fd;
>  }
>  
> -static inline int sys_dup2(int oldfd, int newfd)
> +static inline int sys_dup3(int oldfd, int newfd, int flags)
>  {
> -#ifdef __NR_dup2
> -	return syscall(__NR_dup2, oldfd, newfd);
> -#else
> -	return syscall(__NR_dup3, oldfd, newfd, 0);
> -#endif
> +	return syscall(__NR_dup3, oldfd, newfd, flags);
>  }
>  
>  /* Point *fixed_fd* to the same file that *tmp_fd* points to.
> @@ -614,7 +610,7 @@ static inline int reuse_fd(int fixed_fd, int tmp_fd)
>  {
>  	int err;
>  
> -	err = sys_dup2(tmp_fd, fixed_fd);
> +	err = sys_dup3(tmp_fd, fixed_fd, O_CLOEXEC);
>  	err = err < 0 ? -errno : 0;
>  	close(tmp_fd); /* clean up temporary FD */
>  	return err;
> -- 
> 2.43.0
> 
>
Lennart Poettering May 30, 2024, 8:56 a.m. UTC | #2 
On Mi, 29.05.24 15:32, Andrii Nakryiko (andrii@kernel.org) wrote:

> Make sure to preserve and/or enforce FD_CLOEXEC flag on duped FDs.
> Use dup3() with O_CLOEXEC flag for that.
>
> Without this fix libbpf effectively clears FD_CLOEXEC flag on each of BPF
> map/prog FD, which is definitely not the right or expected behavior.

Thanks!

lgtm, superficially.

> Reported-by: Lennart Poettering <lennart@poettering.net>
> Fixes: bc308d011ab8 ("libbpf: call dup2() syscall directly")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  tools/lib/bpf/libbpf_internal.h | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index a0dcfb82e455..7e7e686008c6 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -597,13 +597,9 @@ static inline int ensure_good_fd(int fd)
>  	return fd;
>  }
>
> -static inline int sys_dup2(int oldfd, int newfd)
> +static inline int sys_dup3(int oldfd, int newfd, int flags)
>  {
> -#ifdef __NR_dup2
> -	return syscall(__NR_dup2, oldfd, newfd);
> -#else
> -	return syscall(__NR_dup3, oldfd, newfd, 0);
> -#endif
> +	return syscall(__NR_dup3, oldfd, newfd, flags);
>  }
>
>  /* Point *fixed_fd* to the same file that *tmp_fd* points to.
> @@ -614,7 +610,7 @@ static inline int reuse_fd(int fixed_fd, int tmp_fd)
>  {
>  	int err;
>
> -	err = sys_dup2(tmp_fd, fixed_fd);
> +	err = sys_dup3(tmp_fd, fixed_fd, O_CLOEXEC);
>  	err = err < 0 ? -errno : 0;
>  	close(tmp_fd); /* clean up temporary FD */
>  	return err;
> --
> 2.43.0
>
>

Lennart

--
Lennart Poettering, Berlin
patchwork-bot+netdevbpf@kernel.org June 1, 2024, 3:40 a.m. UTC | #3 
Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Wed, 29 May 2024 15:32:39 -0700 you wrote:
> Make sure to preserve and/or enforce FD_CLOEXEC flag on duped FDs.
> Use dup3() with O_CLOEXEC flag for that.
> 
> Without this fix libbpf effectively clears FD_CLOEXEC flag on each of BPF
> map/prog FD, which is definitely not the right or expected behavior.
> 
> Reported-by: Lennart Poettering <lennart@poettering.net>
> Fixes: bc308d011ab8 ("libbpf: call dup2() syscall directly")
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> 
> [...]

Here is the summary with links:
  - [bpf-next] libbpf: keep FD_CLOEXEC flag when dup()'ing FD
    https://git.kernel.org/bpf/bpf-next/c/531876c80004

You are awesome, thank you!
Andrii Nakryiko June 3, 2024, 9:16 p.m. UTC | #4 
On Thu, May 30, 2024 at 1:57 AM Lennart Poettering
<lennart@poettering.net> wrote:
>
> On Mi, 29.05.24 15:32, Andrii Nakryiko (andrii@kernel.org) wrote:
>
> > Make sure to preserve and/or enforce FD_CLOEXEC flag on duped FDs.
> > Use dup3() with O_CLOEXEC flag for that.
> >
> > Without this fix libbpf effectively clears FD_CLOEXEC flag on each of BPF
> > map/prog FD, which is definitely not the right or expected behavior.
>
> Thanks!
>
> lgtm, superficially.

This is now in libbpf v1.4.3 bug fix release ([0])

  [0] https://github.com/libbpf/libbpf/releases/tag/v1.4.3

>
> > Reported-by: Lennart Poettering <lennart@poettering.net>
> > Fixes: bc308d011ab8 ("libbpf: call dup2() syscall directly")
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> >  tools/lib/bpf/libbpf_internal.h | 10 +++-------
> >  1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> > index a0dcfb82e455..7e7e686008c6 100644
> > --- a/tools/lib/bpf/libbpf_internal.h
> > +++ b/tools/lib/bpf/libbpf_internal.h
> > @@ -597,13 +597,9 @@ static inline int ensure_good_fd(int fd)
> >       return fd;
> >  }
> >
> > -static inline int sys_dup2(int oldfd, int newfd)
> > +static inline int sys_dup3(int oldfd, int newfd, int flags)
> >  {
> > -#ifdef __NR_dup2
> > -     return syscall(__NR_dup2, oldfd, newfd);
> > -#else
> > -     return syscall(__NR_dup3, oldfd, newfd, 0);
> > -#endif
> > +     return syscall(__NR_dup3, oldfd, newfd, flags);
> >  }
> >
> >  /* Point *fixed_fd* to the same file that *tmp_fd* points to.
> > @@ -614,7 +610,7 @@ static inline int reuse_fd(int fixed_fd, int tmp_fd)
> >  {
> >       int err;
> >
> > -     err = sys_dup2(tmp_fd, fixed_fd);
> > +     err = sys_dup3(tmp_fd, fixed_fd, O_CLOEXEC);
> >       err = err < 0 ? -errno : 0;
> >       close(tmp_fd); /* clean up temporary FD */
> >       return err;
> > --
> > 2.43.0
> >
> >
>
> Lennart
>
> --
> Lennart Poettering, Berlin
diff mbox series

Patch

diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index a0dcfb82e455..7e7e686008c6 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -597,13 +597,9 @@  static inline int ensure_good_fd(int fd)
 	return fd;
 }
 
-static inline int sys_dup2(int oldfd, int newfd)
+static inline int sys_dup3(int oldfd, int newfd, int flags)
 {
-#ifdef __NR_dup2
-	return syscall(__NR_dup2, oldfd, newfd);
-#else
-	return syscall(__NR_dup3, oldfd, newfd, 0);
-#endif
+	return syscall(__NR_dup3, oldfd, newfd, flags);
 }
 
 /* Point *fixed_fd* to the same file that *tmp_fd* points to.
@@ -614,7 +610,7 @@  static inline int reuse_fd(int fixed_fd, int tmp_fd)
 {
 	int err;
 
-	err = sys_dup2(tmp_fd, fixed_fd);
+	err = sys_dup3(tmp_fd, fixed_fd, O_CLOEXEC);
 	err = err < 0 ? -errno : 0;
 	close(tmp_fd); /* clean up temporary FD */
 	return err;