netdev - Re: [PATCH bpf-next v3] bpftool: use only nftw for file tree parsing

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]

Message-ID: <CAPGftE-WNgEHxDWHVF9aFNK-ZnY=V66SdQPkPpni4qxRZ4rHZA@mail.gmail.com>
Date:   Mon, 20 Jul 2020 19:14:50 -0700
From:   Tony Ambardar <tony.ambardar@...il.com>
To:     Quentin Monnet <quentin@...valent.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v3] bpftool: use only nftw for file tree parsing

On Mon, 20 Jul 2020 at 01:13, Quentin Monnet <quentin@...valent.com> wrote:
>
> On 17/07/2020 23:55, Tony Ambardar wrote:
> > The bpftool sources include code to walk file trees, but use multiple
> > frameworks to do so: nftw and fts. While nftw conforms to POSIX/SUSv3 and
> > is widely available, fts is not conformant and less common, especially on
> > non-glibc systems. The inconsistent framework usage hampers maintenance
> > and portability of bpftool, in particular for embedded systems.
> >
> > Standardize code usage by rewriting one fts-based function to use nftw and
> > clean up some related function warnings by extending use of "const char *"
> > arguments. This change helps in building bpftool against musl for OpenWrt.

[...]

> >  int build_pinned_obj_table(struct pinned_obj_table *tab,
> >                          enum bpf_obj_type type)
> >  {
>
> [...]
>
> >       while ((mntent = getmntent(mntfile))) {
>
> [...]
>
> > -             while ((ftse = fts_read(fts))) {
>
> [...]
>
> > +             if (nftw(path, do_build_table_cb, nopenfd, flags) == -1)
> > +                     break;
>
> Sorry I missed that on the previous reviews; but I think a simple break
> out of the loop changes the previous behaviour, we should instead
> "return -1" from build_pinned_obj_table() if nftw() returns -1, as we
> were doing so far.

Thanks for the catch. Sorry, that's totally my fault: this was meant
to be an "err = nftw(...)" with a "return err" on the common exit
path, similar to the rest of the patch. Let me fix and resend.

Kind regards,
Tony

> Looks good otherwise.
>
> >       }
> >       fclose(mntfile);
> >       return 0;