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

Date:   Tue, 28 Apr 2020 11:35:15 -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 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'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).