netdev - Re: [PATCH bpf-next v1 3/6] libbpf: Ensure that module BTF fd is never 0

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 6 Oct 2021 12:09:23 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Kumar Kartikeya Dwivedi <memxor@...il.com>,
        bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next v1 3/6] libbpf: Ensure that module BTF fd is
 never 0

On Wed, Oct 6, 2021 at 9:43 AM Andrii Nakryiko
<andrii.nakryiko@...il.com> wrote:
>
> On Tue, Oct 5, 2021 at 10:24 PM Kumar Kartikeya Dwivedi
> <memxor@...il.com> wrote:
> >
> > On Wed, Oct 06, 2021 at 10:11:29AM IST, Andrii Nakryiko wrote:
> > > On Tue, Oct 5, 2021 at 5:29 PM Kumar Kartikeya Dwivedi <memxor@...il.com> wrote:
> > > >
> > > > Since the code assumes in various places that BTF fd for modules is
> > > > never 0, if we end up getting fd as 0, obtain a new fd > 0. Even though
> > > > fd 0 being free for allocation is usually an application error, it is
> > > > still possible that we end up getting fd 0 if the application explicitly
> > > > closes its stdin. Deal with this by getting a new fd using dup and
> > > > closing fd 0.
> > > >
> > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@...il.com>
> > > > ---
> > > >  tools/lib/bpf/libbpf.c | 14 ++++++++++++++
> > > >  1 file changed, 14 insertions(+)
> > > >
> > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > > index d286dec73b5f..3e5e460fe63e 100644
> > > > --- a/tools/lib/bpf/libbpf.c
> > > > +++ b/tools/lib/bpf/libbpf.c
> > > > @@ -4975,6 +4975,20 @@ static int load_module_btfs(struct bpf_object *obj)
> > > >                         pr_warn("failed to get BTF object #%d FD: %d\n", id, err);
> > > >                         return err;
> > > >                 }
> > > > +               /* Make sure module BTF fd is never 0, as kernel depends on it
> > > > +                * being > 0 to distinguish between vmlinux and module BTFs,
> > > > +                * e.g. for BPF_PSEUDO_BTF_ID ld_imm64 insns (ksyms).
> > > > +                */
> > > > +               if (!fd) {
> > > > +                       fd = dup(0);
> > >
> > > This is not the only place where we make assumptions that fd > 0 but
> > > technically can get fd == 0. Instead of doing such a check in every
> > > such place, would it be possible to open (cheaply) some FD (/dev/null
> > > or whatever, don't know what's the best file to open), if we detect
> > > that FD == 0 is not allocated? Can we detect that fd 0 is not
> > > allocated?
> > >
> >
> > We can, e.g. using access("/proc/self/fd/0", F_OK), but I think just calling
> > open unconditonally and doing if (ret > 0) close(ret) is better. Also, do I
>
> yeah, I like this idea, let's go with it

FYI some production environments may detect that FDs 0,1,2 are not
pointing to stdin, stdout, stderr and will force close whatever files are there
and open 0,1,2 with canonical files.

libbpf doesn't have to resort to such measures, but it would be prudent to
make libbpf operate on FDs > 2 for all bpf objects to make sure other
frameworks don't ruin libbpf's view of FDs.