* [PATCH bpf-next] libbpf: call dup2() syscall directly
@ 2024-01-19 21:02 Andrii Nakryiko
2024-01-19 21:18 ` Song Liu
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2024-01-19 21:02 UTC (permalink / raw)
To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team
We've ran into issues with using dup2() API in production setting, where
libbpf is linked into large production environment and ends up calling
uninteded custom implementations of dup2(). These custom implementations
don't provide atomic FD replacement guarantees of dup2() syscall,
leading to subtle and hard to debug issues.
To prevent this in the future and guarantee that no libc implementation
will do their own custom non-atomic dup2() implementation, call dup2()
syscall directly with syscall(SYS_dup2).
Note that some architectures don't seem to provide dup2 and have dup3
instead. Try to detect and pick best syscall.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/lib/bpf/libbpf_internal.h | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 27e4e320e1a6..58c547d473e0 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -15,6 +15,7 @@
#include <linux/err.h>
#include <fcntl.h>
#include <unistd.h>
+#include <sys/syscall.h>
#include <libelf.h>
#include "relo_core.h"
@@ -555,6 +556,15 @@ static inline int ensure_good_fd(int fd)
return fd;
}
+static inline int sys_dup2(int oldfd, int newfd)
+{
+#ifdef __NR_dup2
+ return syscall(__NR_dup2, oldfd, newfd);
+#else
+ return syscall(__NR_dup3, oldfd, newfd, 0);
+#endif
+}
+
/* Point *fixed_fd* to the same file that *tmp_fd* points to.
* Regardless of success, *tmp_fd* is closed.
* Whatever *fixed_fd* pointed to is closed silently.
@@ -563,7 +573,7 @@ static inline int reuse_fd(int fixed_fd, int tmp_fd)
{
int err;
- err = dup2(tmp_fd, fixed_fd);
+ err = sys_dup2(tmp_fd, fixed_fd);
err = err < 0 ? -errno : 0;
close(tmp_fd); /* clean up temporary FD */
return err;
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: call dup2() syscall directly
2024-01-19 21:02 [PATCH bpf-next] libbpf: call dup2() syscall directly Andrii Nakryiko
@ 2024-01-19 21:18 ` Song Liu
2024-01-19 21:21 ` Andrii Nakryiko
2024-01-21 6:24 ` Yonghong Song
2024-01-23 23:40 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 8+ messages in thread
From: Song Liu @ 2024-01-19 21:18 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, martin.lau, kernel-team
On Fri, Jan 19, 2024 at 1:02 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> We've ran into issues with using dup2() API in production setting, where
> libbpf is linked into large production environment and ends up calling
> uninteded custom implementations of dup2(). These custom implementations
typo: unintended
> don't provide atomic FD replacement guarantees of dup2() syscall,
> leading to subtle and hard to debug issues.
>
> To prevent this in the future and guarantee that no libc implementation
> will do their own custom non-atomic dup2() implementation, call dup2()
> syscall directly with syscall(SYS_dup2).
>
> Note that some architectures don't seem to provide dup2 and have dup3
> instead. Try to detect and pick best syscall.
I wonder whether we can just always use dup3().
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Song Liu <song@kernel.org>
[...]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: call dup2() syscall directly
2024-01-19 21:18 ` Song Liu
@ 2024-01-19 21:21 ` Andrii Nakryiko
2024-01-19 21:29 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2024-01-19 21:21 UTC (permalink / raw)
To: Song Liu; +Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team
On Fri, Jan 19, 2024 at 1:18 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, Jan 19, 2024 at 1:02 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> >
> > We've ran into issues with using dup2() API in production setting, where
> > libbpf is linked into large production environment and ends up calling
> > uninteded custom implementations of dup2(). These custom implementations
>
> typo: unintended
oops, but probably doesn't warrant respinning
>
> > don't provide atomic FD replacement guarantees of dup2() syscall,
> > leading to subtle and hard to debug issues.
> >
> > To prevent this in the future and guarantee that no libc implementation
> > will do their own custom non-atomic dup2() implementation, call dup2()
> > syscall directly with syscall(SYS_dup2).
> >
> > Note that some architectures don't seem to provide dup2 and have dup3
> > instead. Try to detect and pick best syscall.
>
> I wonder whether we can just always use dup3().
dup3() (according to my git foo) was added in 4.17, which is more
modern than some other usable BPF, so I don't want to just randomly
bump the minimal supported (by libbpf) kernel for something like this.
>
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
> Acked-by: Song Liu <song@kernel.org>
>
> [...]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: call dup2() syscall directly
2024-01-19 21:21 ` Andrii Nakryiko
@ 2024-01-19 21:29 ` Andrii Nakryiko
2024-01-19 21:34 ` Song Liu
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2024-01-19 21:29 UTC (permalink / raw)
To: Song Liu; +Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team
On Fri, Jan 19, 2024 at 1:21 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Jan 19, 2024 at 1:18 PM Song Liu <song@kernel.org> wrote:
> >
> > On Fri, Jan 19, 2024 at 1:02 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> > >
> > > We've ran into issues with using dup2() API in production setting, where
> > > libbpf is linked into large production environment and ends up calling
> > > uninteded custom implementations of dup2(). These custom implementations
> >
> > typo: unintended
>
> oops, but probably doesn't warrant respinning
>
> >
> > > don't provide atomic FD replacement guarantees of dup2() syscall,
> > > leading to subtle and hard to debug issues.
> > >
> > > To prevent this in the future and guarantee that no libc implementation
> > > will do their own custom non-atomic dup2() implementation, call dup2()
> > > syscall directly with syscall(SYS_dup2).
> > >
> > > Note that some architectures don't seem to provide dup2 and have dup3
> > > instead. Try to detect and pick best syscall.
> >
> > I wonder whether we can just always use dup3().
>
> dup3() (according to my git foo) was added in 4.17, which is more
> modern than some other usable BPF, so I don't want to just randomly
> bump the minimal supported (by libbpf) kernel for something like this.
>
Btw, this #ifdef check is the same as what glibc does for its
implementation of dup2() (except for fd equality check which isn't
necessary for libbpf), see [0]
[0] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/dup2.c
> >
> > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> >
> > Acked-by: Song Liu <song@kernel.org>
> >
> > [...]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: call dup2() syscall directly
2024-01-19 21:29 ` Andrii Nakryiko
@ 2024-01-19 21:34 ` Song Liu
2024-01-19 21:42 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Song Liu @ 2024-01-19 21:34 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team
On Fri, Jan 19, 2024 at 1:30 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Jan 19, 2024 at 1:21 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Fri, Jan 19, 2024 at 1:18 PM Song Liu <song@kernel.org> wrote:
> > >
> > > On Fri, Jan 19, 2024 at 1:02 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> > > >
> > > > We've ran into issues with using dup2() API in production setting, where
> > > > libbpf is linked into large production environment and ends up calling
> > > > uninteded custom implementations of dup2(). These custom implementations
> > >
> > > typo: unintended
> >
> > oops, but probably doesn't warrant respinning
> >
> > >
> > > > don't provide atomic FD replacement guarantees of dup2() syscall,
> > > > leading to subtle and hard to debug issues.
> > > >
> > > > To prevent this in the future and guarantee that no libc implementation
> > > > will do their own custom non-atomic dup2() implementation, call dup2()
> > > > syscall directly with syscall(SYS_dup2).
> > > >
> > > > Note that some architectures don't seem to provide dup2 and have dup3
> > > > instead. Try to detect and pick best syscall.
> > >
> > > I wonder whether we can just always use dup3().
> >
> > dup3() (according to my git foo) was added in 4.17, which is more
> > modern than some other usable BPF, so I don't want to just randomly
> > bump the minimal supported (by libbpf) kernel for something like this.
> >
I believe dup3() was added in 3.7.
>
> Btw, this #ifdef check is the same as what glibc does for its
> implementation of dup2() (except for fd equality check which isn't
> necessary for libbpf), see [0]
>
> [0] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/dup2.c
Yep, this looks good.
Thanks,
Song
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: call dup2() syscall directly
2024-01-19 21:34 ` Song Liu
@ 2024-01-19 21:42 ` Andrii Nakryiko
0 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2024-01-19 21:42 UTC (permalink / raw)
To: Song Liu; +Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team
On Fri, Jan 19, 2024 at 1:34 PM Song Liu <song@kernel.org> wrote:
>
> On Fri, Jan 19, 2024 at 1:30 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Fri, Jan 19, 2024 at 1:21 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Fri, Jan 19, 2024 at 1:18 PM Song Liu <song@kernel.org> wrote:
> > > >
> > > > On Fri, Jan 19, 2024 at 1:02 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> > > > >
> > > > > We've ran into issues with using dup2() API in production setting, where
> > > > > libbpf is linked into large production environment and ends up calling
> > > > > uninteded custom implementations of dup2(). These custom implementations
> > > >
> > > > typo: unintended
> > >
> > > oops, but probably doesn't warrant respinning
> > >
> > > >
> > > > > don't provide atomic FD replacement guarantees of dup2() syscall,
> > > > > leading to subtle and hard to debug issues.
> > > > >
> > > > > To prevent this in the future and guarantee that no libc implementation
> > > > > will do their own custom non-atomic dup2() implementation, call dup2()
> > > > > syscall directly with syscall(SYS_dup2).
> > > > >
> > > > > Note that some architectures don't seem to provide dup2 and have dup3
> > > > > instead. Try to detect and pick best syscall.
> > > >
> > > > I wonder whether we can just always use dup3().
> > >
> > > dup3() (according to my git foo) was added in 4.17, which is more
> > > modern than some other usable BPF, so I don't want to just randomly
> > > bump the minimal supported (by libbpf) kernel for something like this.
> > >
>
> I believe dup3() was added in 3.7.
True, my git-foo isn't careful enough, 4.17 is when dup3 kernel
refactoring happened. bpf() syscall was added in 3.17, right? In that
case, yep, I could have just gone with __NR_dup3 directly, I suppose,
but this version should work well anyways, so I wouldn't bother
changing it.
>
> >
> > Btw, this #ifdef check is the same as what glibc does for its
> > implementation of dup2() (except for fd equality check which isn't
> > necessary for libbpf), see [0]
> >
> > [0] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/dup2.c
>
> Yep, this looks good.
>
> Thanks,
> Song
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: call dup2() syscall directly
2024-01-19 21:02 [PATCH bpf-next] libbpf: call dup2() syscall directly Andrii Nakryiko
2024-01-19 21:18 ` Song Liu
@ 2024-01-21 6:24 ` Yonghong Song
2024-01-23 23:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 8+ messages in thread
From: Yonghong Song @ 2024-01-21 6:24 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, ast, daniel, martin.lau; +Cc: kernel-team
On 1/19/24 1:02 PM, Andrii Nakryiko wrote:
> We've ran into issues with using dup2() API in production setting, where
> libbpf is linked into large production environment and ends up calling
> uninteded custom implementations of dup2(). These custom implementations
> don't provide atomic FD replacement guarantees of dup2() syscall,
> leading to subtle and hard to debug issues.
>
> To prevent this in the future and guarantee that no libc implementation
> will do their own custom non-atomic dup2() implementation, call dup2()
> syscall directly with syscall(SYS_dup2).
>
> Note that some architectures don't seem to provide dup2 and have dup3
> instead. Try to detect and pick best syscall.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next] libbpf: call dup2() syscall directly
2024-01-19 21:02 [PATCH bpf-next] libbpf: call dup2() syscall directly Andrii Nakryiko
2024-01-19 21:18 ` Song Liu
2024-01-21 6:24 ` Yonghong Song
@ 2024-01-23 23:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-01-23 23:40 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, ast, daniel, martin.lau, kernel-team
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Fri, 19 Jan 2024 13:02:01 -0800 you wrote:
> We've ran into issues with using dup2() API in production setting, where
> libbpf is linked into large production environment and ends up calling
> uninteded custom implementations of dup2(). These custom implementations
> don't provide atomic FD replacement guarantees of dup2() syscall,
> leading to subtle and hard to debug issues.
>
> To prevent this in the future and guarantee that no libc implementation
> will do their own custom non-atomic dup2() implementation, call dup2()
> syscall directly with syscall(SYS_dup2).
>
> [...]
Here is the summary with links:
- [bpf-next] libbpf: call dup2() syscall directly
https://git.kernel.org/bpf/bpf-next/c/bc308d011ab8
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-01-23 23:40 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-19 21:02 [PATCH bpf-next] libbpf: call dup2() syscall directly Andrii Nakryiko
2024-01-19 21:18 ` Song Liu
2024-01-19 21:21 ` Andrii Nakryiko
2024-01-19 21:29 ` Andrii Nakryiko
2024-01-19 21:34 ` Song Liu
2024-01-19 21:42 ` Andrii Nakryiko
2024-01-21 6:24 ` Yonghong Song
2024-01-23 23:40 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).