netdev - Re: [PATCH v3] tools: bpftool: fix reading from /proc/config.gz

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:   Fri, 9 Aug 2019 14:09:56 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Stanislav Fomichev <sdf@...ichev.me>
Cc:     Peter Wu <peter@...ensteyn.nl>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, netdev@...r.kernel.org,
        Stanislav Fomichev <sdf@...gle.com>,
        Quentin Monnet <quentin.monnet@...ronome.com>
Subject: Re: [PATCH v3] tools: bpftool: fix reading from /proc/config.gz

On Fri, 9 Aug 2019 08:32:10 -0700, Stanislav Fomichev wrote:
> On 08/09, Peter Wu wrote:
> > /proc/config has never existed as far as I can see, but /proc/config.gz
> > is present on Arch Linux. Add support for decompressing config.gz using
> > zlib which is a mandatory dependency of libelf. Replace existing stdio
> > functions with gzFile operations since the latter transparently handles
> > uncompressed and gzip-compressed files.
> > 
> > Cc: Quentin Monnet <quentin.monnet@...ronome.com>
> > Signed-off-by: Peter Wu <peter@...ensteyn.nl>

Thanks for the patch, looks good to me now!

> >  tools/bpf/bpftool/Makefile  |   2 +-
> >  tools/bpf/bpftool/feature.c | 105 ++++++++++++++++++------------------
> >  2 files changed, 54 insertions(+), 53 deletions(-)
> > 
> > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > index a7afea4dec47..078bd0dcfba5 100644
> > --- a/tools/bpf/bpftool/Makefile
> > +++ b/tools/bpf/bpftool/Makefile
> > @@ -52,7 +52,7 @@ ifneq ($(EXTRA_LDFLAGS),)
> >  LDFLAGS += $(EXTRA_LDFLAGS)
> >  endif
> >  
> > -LIBS = -lelf $(LIBBPF)
> > +LIBS = -lelf -lz $(LIBBPF)  
> You're saying in the commit description that bpftool already links
> against -lz (via -lelf), but then explicitly add -lz here, why?

It probably won't hurt to enable the zlib test:

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 078bd0dcfba5..8176632e519c 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -58,8 +58,8 @@ INSTALL ?= install
 RM ?= rm -f
 
 FEATURE_USER = .bpftool
-FEATURE_TESTS = libbfd disassembler-four-args reallocarray
-FEATURE_DISPLAY = libbfd disassembler-four-args
+FEATURE_TESTS = libbfd disassembler-four-args reallocarray zlib
+FEATURE_DISPLAY = libbfd disassembler-four-args zlib
 
 check_feat := 1
 NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall

And then we can test for it the way libbpf tests for elf:

all: zdep $(OUTPUT)bpftool

PHONY += zdep

zdep:
	@if [ "$(feature-zlib)" != "1" ]; then echo "No zlib found"; exit 1 ; fi

Or maybe just $(error ...), Stan what's your preference here? 
We don't have a precedent for hard tests of features in bpftool.