Re: [PATCH bpf-next] libbpf: call dup2() syscall directly

[Song Liu <song@kernel.org>]

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