netdev - Re: [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer

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] [day] [month] [year] [list]

Message-ID: <CAEf4BzbTAm2BWE6zXxr4sjd3E-TSAMMdH_pU+06f4kvMUBdknA@mail.gmail.com>
Date:   Tue, 28 Apr 2020 15:13:00 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>, Julia Kartseva <hex@...com>
Subject: Re: [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor
 with AddressSantizer

On Tue, Apr 28, 2020 at 1:41 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Tue, Apr 28, 2020 at 11:35:15AM -0700, Andrii Nakryiko wrote:
> > On Tue, Apr 28, 2020 at 9:44 AM Alexei Starovoitov
> > <alexei.starovoitov@...il.com> wrote:
> > >
> > > On Mon, Apr 27, 2020 at 09:46:24PM -0700, Andrii Nakryiko wrote:
> > > > Add another flavor of test_progs that is compiled and run with
> > > > AddressSanitizer and LeakSanitizer. This allows to find potential memory
> > > > correction bugs and memory leaks. Due to sometimes not trivial requirements on
> > > > the environment, this is (for now) done as a separate flavor, not by default.
> > > > Eventually I hope to enable it by default.
> > > >
> > > > To run ./test_progs-asan successfully, you need to have libasan installed in
> > > > the system, where version of the package depends on GCC version you have.
> > > > E.g., GCC8 needs libasan5, while GCC7 uses libasan4.
> > > >
> > > > For CentOS 7, to build everything successfully one would need to:
> > > >   $ sudo yum install devtoolset-8-gcc devtoolset-libasan-devel
> > > >
> > > > For Arch Linux to run selftests, one would need to install gcc-libs package to
> > > > get libasan.so.5:
> > > >   $ sudo pacman -S gcc-libs
> > > >
> > > > Cc: Julia Kartseva <hex@...com>
> > > > Signed-off-by: Andrii Nakryiko <andriin@...com>
> > >
> > > It needs a feature check.
> > > selftest shouldn't be forcing asan on everyone.
> > > Even after I did:
> > > sudo yum install devtoolset-8-libasan-devel
> > > it still failed to build:
> > >   BINARY   test_progs-asan
> > > /opt/rh/devtoolset-9/root/usr/libexec/gcc/x86_64-redhat-linux/9/ld: cannot find libasan_preinit.o: No such file or directory
> > > /opt/rh/devtoolset-9/root/usr/libexec/gcc/x86_64-redhat-linux/9/ld: cannot find -lasan
> > >
> >
> > Yeah, it worked for me initially because it still used GCC7 locally
> > and older version of libasan.
> >
> > On CentOS you have to run the following command to set up environment
> > (for current session only, though):
> >
> > $ scl enable devtoolset-8 bash
> >
> > What it does:
> > - adds /opt/rh/devtoolset-8/root/usr/bin to $PATH
> > - sets $LD_LIBRARY_PATH to
> > /opt/rh/devtoolset-8/root/usr/lib64:/opt/rh/devtoolset-8/root/usr/lib:/opt/rh/devtoolset-8/root/usr/lib64/dyninst:/opt/rh/devtoolset-8/root/usr/lib/dyninst:/opt/rh/devtoolset-8/root/usr/lib64:/opt/rh/devtoolset-8/root/usr/lib
>
> I don't want to do this, since I prefer gcc9 for my builds since it has better warnings.
> But yum cannot find devtoolset-9-libasan-devel it seems.

Hmm, strange, there should be devtoolset-9-libasan-devel according to
[0]. No idea.

  [0] https://cbs.centos.org/koji/buildinfo?buildID=27149

>
> > I'm going to add this to patch to ease some pain later. But yeah, I
> > think I have a better plan for ASAN builds. I'll add EXTRA_CFLAGS to
> > selftests Makefile, defaulted to nothing. Then for Travis CI (or
> > locally) one would do:
> >
> > $ make EXTRA_CFLAGS='-fsanitize-address'
> >
> > to build ASAN versions of all the same test runners (including
> > test_verifier, test_maps, etc).
> >
> > I think this will be better overall.
> >
> > > Also I really don't like that skeletons are now built three times for now good reason
> > >   GEN-SKEL [test_progs-asan] test_stack_map.skel.h
> > >   GEN-SKEL [test_progs-asan] test_core_reloc_nesting.skel.h
> > > default vs no_alu32 makes sense. They are different bpf.o files and different skeletons,
> > > but for asan there is no such need.
> >
> > I agree, luckily I don't really have to change anything with the above approach.
> >
> > >
> > > Please resubmit the rest of the patches, since asan isn't a prerequisite.
> >
> > I'll update this patch to just add EXTRA_CFLAGS, if you are ok with
> > this (and will leave instructions on installing libasan).
>
> yeah. EXTRA_CFLAGS approach should work.
> That will build both test_progs and test_progs-no_alu32 with libasan ?
> That would be ideal.

Yep, then everything should be built with ASAN. Ok, will do that then.