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

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

[Andrii Nakryiko]

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(-)

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

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

[Jiri Olsa]

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
> 
> 

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

[Lennart Poettering]

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

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

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

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

[Andrii Nakryiko]

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