* [PATCH bpf-next 0/6] Add ASAN to selftest and fix found problems
@ 2020-04-28 4:46 Andrii Nakryiko
2020-04-28 4:46 ` [PATCH bpf-next 1/6] selftests/bpf: ensure test flavors use correct skeletons Andrii Nakryiko
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Andrii Nakryiko @ 2020-04-28 4:46 UTC (permalink / raw)
To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
Add ASAN flavor for test_progs and fix all found memory leaks and other memory
use problems.
Andrii Nakryiko (6):
selftests/bpf: ensure test flavors use correct skeletons
selftests/bpf: add test_progs-asan flavor with AddressSantizer
selftests/bpf: convert test_hashmap into test_progs test
libbpf: fix memory leak and possible double-free in hashmap__clear
selftests/bpf: fix memory leak in test selector
selftests/bpf: fix memory leak in extract_build_id()
tools/lib/bpf/hashmap.c | 7 +
tools/testing/selftests/bpf/.gitignore | 3 +-
tools/testing/selftests/bpf/Makefile | 20 +-
.../{test_hashmap.c => prog_tests/hashmap.c} | 280 +++++++++---------
tools/testing/selftests/bpf/test_progs.c | 21 +-
5 files changed, 179 insertions(+), 152 deletions(-)
rename tools/testing/selftests/bpf/{test_hashmap.c => prog_tests/hashmap.c} (53%)
--
2.24.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH bpf-next 1/6] selftests/bpf: ensure test flavors use correct skeletons 2020-04-28 4:46 [PATCH bpf-next 0/6] Add ASAN to selftest and fix found problems Andrii Nakryiko @ 2020-04-28 4:46 ` Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer Andrii Nakryiko ` (4 subsequent siblings) 5 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2020-04-28 4:46 UTC (permalink / raw) To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko Ensure that test runner flavors include their own skeletons from <flavor>/ directory. Previously, skeletons generated for no-flavor test_progs were used. Apart from fixing correctness, this also makes it possible to compile only flavors individually: $ make clean && make test_progs-no_alu32 ... now succeeds ... Fixes: 74b5a5968fe8 ("selftests/bpf: Replace test_progs and test_maps w/ general rule") Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- tools/testing/selftests/bpf/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 7729892e0b04..fd56e31a5b4f 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -323,7 +323,7 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \ $(TRUNNER_BPF_SKELS) \ $$(BPFOBJ) | $(TRUNNER_OUTPUT) $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@) - cd $$(@D) && $$(CC) $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) + cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \ %.c \ -- 2.24.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer 2020-04-28 4:46 [PATCH bpf-next 0/6] Add ASAN to selftest and fix found problems Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 1/6] selftests/bpf: ensure test flavors use correct skeletons Andrii Nakryiko @ 2020-04-28 4:46 ` Andrii Nakryiko 2020-04-28 16:44 ` Alexei Starovoitov 2020-04-28 4:46 ` [PATCH bpf-next 3/6] selftests/bpf: convert test_hashmap into test_progs test Andrii Nakryiko ` (3 subsequent siblings) 5 siblings, 1 reply; 11+ messages in thread From: Andrii Nakryiko @ 2020-04-28 4:46 UTC (permalink / raw) To: bpf, netdev, ast, daniel Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Julia Kartseva 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@fb.com> Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- tools/testing/selftests/bpf/.gitignore | 1 + tools/testing/selftests/bpf/Makefile | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore index c30079c86998..69b545ca51b8 100644 --- a/tools/testing/selftests/bpf/.gitignore +++ b/tools/testing/selftests/bpf/.gitignore @@ -36,6 +36,7 @@ test_current_pid_tgid_new_ns xdping test_cpp *.skel.h +/asan /no_alu32 /bpf_gcc /tools diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index fd56e31a5b4f..e54d069b27a6 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -33,7 +33,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \ test_cgroup_storage \ test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \ - test_progs-no_alu32 \ + test_progs-no_alu32 test_progs-asan \ test_current_pid_tgid_new_ns # Also test bpf-gcc, if present @@ -344,7 +344,8 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \ $(TRUNNER_EXTRA_OBJS) $$(BPFOBJ) \ | $(TRUNNER_BINARY)-extras $$(call msg,BINARY,,$$@) - $$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@ + $$(CC) $$(CFLAGS) $(TRUNNER_SAN_CFLAGS) $$(filter %.a %.o,$$^) \ + $$(LDLIBS) -o $$@ endef @@ -358,11 +359,18 @@ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \ TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) TRUNNER_BPF_LDFLAGS := -mattr=+alu32 +TRUNNER_SAN_CFLAGS := $(eval $(call DEFINE_TEST_RUNNER,test_progs)) +# Define test_progs-asan test runner. +TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE +TRUNNER_SAN_CFLAGS := -fsanitize=address +$(eval $(call DEFINE_TEST_RUNNER,test_progs,asan)) + # Define test_progs-no_alu32 test runner. TRUNNER_BPF_BUILD_RULE := CLANG_NOALU32_BPF_BUILD_RULE TRUNNER_BPF_LDFLAGS := +TRUNNER_SAN_CFLAGS := $(eval $(call DEFINE_TEST_RUNNER,test_progs,no_alu32)) # Define test_progs BPF-GCC-flavored test runner. @@ -370,6 +378,7 @@ ifneq ($(BPF_GCC),) TRUNNER_BPF_BUILD_RULE := GCC_BPF_BUILD_RULE TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(call get_sys_includes,gcc) TRUNNER_BPF_LDFLAGS := +TRUNNER_SAN_CFLAGS := $(eval $(call DEFINE_TEST_RUNNER,test_progs,bpf_gcc)) endif @@ -381,6 +390,7 @@ TRUNNER_EXTRA_FILES := TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built) TRUNNER_BPF_CFLAGS := TRUNNER_BPF_LDFLAGS := +TRUNNER_SAN_CFLAGS := $(eval $(call DEFINE_TEST_RUNNER,test_maps)) # Define test_verifier test runner. @@ -406,4 +416,4 @@ $(OUTPUT)/test_cpp: test_cpp.cpp $(OUTPUT)/test_core_extern.skel.h $(BPFOBJ) EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) \ prog_tests/tests.h map_tests/tests.h verifier/tests.h \ feature \ - $(addprefix $(OUTPUT)/,*.o *.skel.h no_alu32 bpf_gcc) + $(addprefix $(OUTPUT)/,*.o *.skel.h asan no_alu32 bpf_gcc) -- 2.24.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer 2020-04-28 4:46 ` [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer Andrii Nakryiko @ 2020-04-28 16:44 ` Alexei Starovoitov 2020-04-28 18:35 ` Andrii Nakryiko 0 siblings, 1 reply; 11+ messages in thread From: Alexei Starovoitov @ 2020-04-28 16:44 UTC (permalink / raw) To: Andrii Nakryiko Cc: bpf, netdev, ast, daniel, andrii.nakryiko, kernel-team, Julia Kartseva 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@fb.com> > Signed-off-by: Andrii Nakryiko <andriin@fb.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 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. Please resubmit the rest of the patches, since asan isn't a prerequisite. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer 2020-04-28 16:44 ` Alexei Starovoitov @ 2020-04-28 18:35 ` Andrii Nakryiko 2020-04-28 20:41 ` Alexei Starovoitov 0 siblings, 1 reply; 11+ messages in thread From: Andrii Nakryiko @ 2020-04-28 18:35 UTC (permalink / raw) To: Alexei Starovoitov Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov, Daniel Borkmann, Kernel Team, Julia Kartseva On Tue, Apr 28, 2020 at 9:44 AM Alexei Starovoitov <alexei.starovoitov@gmail.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@fb.com> > > Signed-off-by: Andrii Nakryiko <andriin@fb.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). ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer 2020-04-28 18:35 ` Andrii Nakryiko @ 2020-04-28 20:41 ` Alexei Starovoitov 2020-04-28 22:13 ` Andrii Nakryiko 0 siblings, 1 reply; 11+ messages in thread From: Alexei Starovoitov @ 2020-04-28 20:41 UTC (permalink / raw) To: Andrii Nakryiko Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov, Daniel Borkmann, Kernel Team, Julia Kartseva 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@gmail.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@fb.com> > > > Signed-off-by: Andrii Nakryiko <andriin@fb.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. > 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer 2020-04-28 20:41 ` Alexei Starovoitov @ 2020-04-28 22:13 ` Andrii Nakryiko 0 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2020-04-28 22:13 UTC (permalink / raw) To: Alexei Starovoitov Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov, Daniel Borkmann, Kernel Team, Julia Kartseva On Tue, Apr 28, 2020 at 1:41 PM Alexei Starovoitov <alexei.starovoitov@gmail.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@gmail.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@fb.com> > > > > Signed-off-by: Andrii Nakryiko <andriin@fb.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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next 3/6] selftests/bpf: convert test_hashmap into test_progs test 2020-04-28 4:46 [PATCH bpf-next 0/6] Add ASAN to selftest and fix found problems Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 1/6] selftests/bpf: ensure test flavors use correct skeletons Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer Andrii Nakryiko @ 2020-04-28 4:46 ` Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 4/6] libbpf: fix memory leak and possible double-free in hashmap__clear Andrii Nakryiko ` (2 subsequent siblings) 5 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2020-04-28 4:46 UTC (permalink / raw) To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko Fold stand-alone test_hashmap test into test_progs. Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- tools/testing/selftests/bpf/.gitignore | 2 - tools/testing/selftests/bpf/Makefile | 2 +- .../{test_hashmap.c => prog_tests/hashmap.c} | 280 +++++++++--------- 3 files changed, 140 insertions(+), 144 deletions(-) rename tools/testing/selftests/bpf/{test_hashmap.c => prog_tests/hashmap.c} (53%) diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore index 69b545ca51b8..db2e142c7635 100644 --- a/tools/testing/selftests/bpf/.gitignore +++ b/tools/testing/selftests/bpf/.gitignore @@ -30,8 +30,6 @@ test_tcpnotify_user test_libbpf test_tcp_check_syncookie_user test_sysctl -test_hashmap -test_btf_dump test_current_pid_tgid_new_ns xdping test_cpp diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index e54d069b27a6..d20405e2fc77 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -32,7 +32,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \ test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \ test_cgroup_storage \ - test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \ + test_netcnt test_tcpnotify_user test_sock_fields test_sysctl \ test_progs-no_alu32 test_progs-asan \ test_current_pid_tgid_new_ns diff --git a/tools/testing/selftests/bpf/test_hashmap.c b/tools/testing/selftests/bpf/prog_tests/hashmap.c similarity index 53% rename from tools/testing/selftests/bpf/test_hashmap.c rename to tools/testing/selftests/bpf/prog_tests/hashmap.c index c490e012c23f..428d488830c6 100644 --- a/tools/testing/selftests/bpf/test_hashmap.c +++ b/tools/testing/selftests/bpf/prog_tests/hashmap.c @@ -5,26 +5,17 @@ * * Copyright (c) 2019 Facebook */ -#include <stdio.h> -#include <errno.h> -#include <linux/err.h> +#include "test_progs.h" #include "bpf/hashmap.h" -#define CHECK(condition, format...) ({ \ - int __ret = !!(condition); \ - if (__ret) { \ - fprintf(stderr, "%s:%d:FAIL ", __func__, __LINE__); \ - fprintf(stderr, format); \ - } \ - __ret; \ -}) +static int duration = 0; -size_t hash_fn(const void *k, void *ctx) +static size_t hash_fn(const void *k, void *ctx) { return (long)k; } -bool equal_fn(const void *a, const void *b, void *ctx) +static bool equal_fn(const void *a, const void *b, void *ctx) { return (long)a == (long)b; } @@ -49,53 +40,55 @@ static inline size_t exp_cap(size_t sz) #define ELEM_CNT 62 -int test_hashmap_generic(void) +static void test_hashmap_generic(void) { struct hashmap_entry *entry, *tmp; int err, bkt, found_cnt, i; long long found_msk; struct hashmap *map; - fprintf(stderr, "%s: ", __func__); - map = hashmap__new(hash_fn, equal_fn, NULL); - if (CHECK(IS_ERR(map), "failed to create map: %ld\n", PTR_ERR(map))) - return 1; + if (CHECK(IS_ERR(map), "hashmap__new", + "failed to create map: %ld\n", PTR_ERR(map))) + return; for (i = 0; i < ELEM_CNT; i++) { const void *oldk, *k = (const void *)(long)i; void *oldv, *v = (void *)(long)(1024 + i); err = hashmap__update(map, k, v, &oldk, &oldv); - if (CHECK(err != -ENOENT, "unexpected result: %d\n", err)) - return 1; + if (CHECK(err != -ENOENT, "hashmap__update", + "unexpected result: %d\n", err)) + goto cleanup; if (i % 2) { err = hashmap__add(map, k, v); } else { err = hashmap__set(map, k, v, &oldk, &oldv); - if (CHECK(oldk != NULL || oldv != NULL, + if (CHECK(oldk != NULL || oldv != NULL, "check_kv", "unexpected k/v: %p=%p\n", oldk, oldv)) - return 1; + goto cleanup; } - if (CHECK(err, "failed to add k/v %ld = %ld: %d\n", + if (CHECK(err, "elem_add", "failed to add k/v %ld = %ld: %d\n", (long)k, (long)v, err)) - return 1; + goto cleanup; - if (CHECK(!hashmap__find(map, k, &oldv), + if (CHECK(!hashmap__find(map, k, &oldv), "elem_find", "failed to find key %ld\n", (long)k)) - return 1; - if (CHECK(oldv != v, "found value is wrong: %ld\n", (long)oldv)) - return 1; + goto cleanup; + if (CHECK(oldv != v, "elem_val", + "found value is wrong: %ld\n", (long)oldv)) + goto cleanup; } - if (CHECK(hashmap__size(map) != ELEM_CNT, + if (CHECK(hashmap__size(map) != ELEM_CNT, "hashmap__size", "invalid map size: %zu\n", hashmap__size(map))) - return 1; + goto cleanup; if (CHECK(hashmap__capacity(map) != exp_cap(hashmap__size(map)), + "hashmap_cap", "unexpected map capacity: %zu\n", hashmap__capacity(map))) - return 1; + goto cleanup; found_msk = 0; hashmap__for_each_entry(map, entry, bkt) { @@ -103,42 +96,47 @@ int test_hashmap_generic(void) long v = (long)entry->value; found_msk |= 1ULL << k; - if (CHECK(v - k != 1024, "invalid k/v pair: %ld = %ld\n", k, v)) - return 1; + if (CHECK(v - k != 1024, "check_kv", + "invalid k/v pair: %ld = %ld\n", k, v)) + goto cleanup; } - if (CHECK(found_msk != (1ULL << ELEM_CNT) - 1, + if (CHECK(found_msk != (1ULL << ELEM_CNT) - 1, "elem_cnt", "not all keys iterated: %llx\n", found_msk)) - return 1; + goto cleanup; for (i = 0; i < ELEM_CNT; i++) { const void *oldk, *k = (const void *)(long)i; void *oldv, *v = (void *)(long)(256 + i); err = hashmap__add(map, k, v); - if (CHECK(err != -EEXIST, "unexpected add result: %d\n", err)) - return 1; + if (CHECK(err != -EEXIST, "hashmap__add", + "unexpected add result: %d\n", err)) + goto cleanup; if (i % 2) err = hashmap__update(map, k, v, &oldk, &oldv); else err = hashmap__set(map, k, v, &oldk, &oldv); - if (CHECK(err, "failed to update k/v %ld = %ld: %d\n", - (long)k, (long)v, err)) - return 1; - if (CHECK(!hashmap__find(map, k, &oldv), + if (CHECK(err, "elem_upd", + "failed to update k/v %ld = %ld: %d\n", + (long)k, (long)v, err)) + goto cleanup; + if (CHECK(!hashmap__find(map, k, &oldv), "elem_find", "failed to find key %ld\n", (long)k)) - return 1; - if (CHECK(oldv != v, "found value is wrong: %ld\n", (long)oldv)) - return 1; + goto cleanup; + if (CHECK(oldv != v, "elem_val", + "found value is wrong: %ld\n", (long)oldv)) + goto cleanup; } - if (CHECK(hashmap__size(map) != ELEM_CNT, + if (CHECK(hashmap__size(map) != ELEM_CNT, "hashmap__size", "invalid updated map size: %zu\n", hashmap__size(map))) - return 1; + goto cleanup; if (CHECK(hashmap__capacity(map) != exp_cap(hashmap__size(map)), + "hashmap__capacity", "unexpected map capacity: %zu\n", hashmap__capacity(map))) - return 1; + goto cleanup; found_msk = 0; hashmap__for_each_entry_safe(map, entry, tmp, bkt) { @@ -146,20 +144,21 @@ int test_hashmap_generic(void) long v = (long)entry->value; found_msk |= 1ULL << k; - if (CHECK(v - k != 256, + if (CHECK(v - k != 256, "elem_check", "invalid updated k/v pair: %ld = %ld\n", k, v)) - return 1; + goto cleanup; } - if (CHECK(found_msk != (1ULL << ELEM_CNT) - 1, + if (CHECK(found_msk != (1ULL << ELEM_CNT) - 1, "elem_cnt", "not all keys iterated after update: %llx\n", found_msk)) - return 1; + goto cleanup; found_cnt = 0; hashmap__for_each_key_entry(map, entry, (void *)0) { found_cnt++; } - if (CHECK(!found_cnt, "didn't find any entries for key 0\n")) - return 1; + if (CHECK(!found_cnt, "found_cnt", + "didn't find any entries for key 0\n")) + goto cleanup; found_msk = 0; found_cnt = 0; @@ -173,30 +172,31 @@ int test_hashmap_generic(void) found_cnt++; found_msk |= 1ULL << (long)k; - if (CHECK(!hashmap__delete(map, k, &oldk, &oldv), + if (CHECK(!hashmap__delete(map, k, &oldk, &oldv), "elem_del", "failed to delete k/v %ld = %ld\n", (long)k, (long)v)) - return 1; - if (CHECK(oldk != k || oldv != v, + goto cleanup; + if (CHECK(oldk != k || oldv != v, "check_old", "invalid deleted k/v: expected %ld = %ld, got %ld = %ld\n", (long)k, (long)v, (long)oldk, (long)oldv)) - return 1; - if (CHECK(hashmap__delete(map, k, &oldk, &oldv), + goto cleanup; + if (CHECK(hashmap__delete(map, k, &oldk, &oldv), "elem_del", "unexpectedly deleted k/v %ld = %ld\n", (long)oldk, (long)oldv)) - return 1; + goto cleanup; } - if (CHECK(!found_cnt || !found_msk, + if (CHECK(!found_cnt || !found_msk, "found_entries", "didn't delete any key entries\n")) - return 1; - if (CHECK(hashmap__size(map) != ELEM_CNT - found_cnt, + goto cleanup; + if (CHECK(hashmap__size(map) != ELEM_CNT - found_cnt, "elem_cnt", "invalid updated map size (already deleted: %d): %zu\n", found_cnt, hashmap__size(map))) - return 1; + goto cleanup; if (CHECK(hashmap__capacity(map) != exp_cap(hashmap__size(map)), + "hashmap__capacity", "unexpected map capacity: %zu\n", hashmap__capacity(map))) - return 1; + goto cleanup; hashmap__for_each_entry_safe(map, entry, tmp, bkt) { const void *oldk, *k; @@ -208,53 +208,56 @@ int test_hashmap_generic(void) found_cnt++; found_msk |= 1ULL << (long)k; - if (CHECK(!hashmap__delete(map, k, &oldk, &oldv), + if (CHECK(!hashmap__delete(map, k, &oldk, &oldv), "elem_del", "failed to delete k/v %ld = %ld\n", (long)k, (long)v)) - return 1; - if (CHECK(oldk != k || oldv != v, + goto cleanup; + if (CHECK(oldk != k || oldv != v, "elem_check", "invalid old k/v: expect %ld = %ld, got %ld = %ld\n", (long)k, (long)v, (long)oldk, (long)oldv)) - return 1; - if (CHECK(hashmap__delete(map, k, &oldk, &oldv), + goto cleanup; + if (CHECK(hashmap__delete(map, k, &oldk, &oldv), "elem_del", "unexpectedly deleted k/v %ld = %ld\n", (long)k, (long)v)) - return 1; + goto cleanup; } if (CHECK(found_cnt != ELEM_CNT || found_msk != (1ULL << ELEM_CNT) - 1, + "found_cnt", "not all keys were deleted: found_cnt:%d, found_msk:%llx\n", found_cnt, found_msk)) - return 1; - if (CHECK(hashmap__size(map) != 0, + goto cleanup; + if (CHECK(hashmap__size(map) != 0, "hashmap__size", "invalid updated map size (already deleted: %d): %zu\n", found_cnt, hashmap__size(map))) - return 1; + goto cleanup; found_cnt = 0; hashmap__for_each_entry(map, entry, bkt) { - CHECK(false, "unexpected map entries left: %ld = %ld\n", - (long)entry->key, (long)entry->value); - return 1; + CHECK(false, "elem_exists", + "unexpected map entries left: %ld = %ld\n", + (long)entry->key, (long)entry->value); + goto cleanup; } - hashmap__free(map); + hashmap__clear(map); hashmap__for_each_entry(map, entry, bkt) { - CHECK(false, "unexpected map entries left: %ld = %ld\n", - (long)entry->key, (long)entry->value); - return 1; + CHECK(false, "elem_exists", + "unexpected map entries left: %ld = %ld\n", + (long)entry->key, (long)entry->value); + goto cleanup; } - fprintf(stderr, "OK\n"); - return 0; +cleanup: + hashmap__free(map); } -size_t collision_hash_fn(const void *k, void *ctx) +static size_t collision_hash_fn(const void *k, void *ctx) { return 0; } -int test_hashmap_multimap(void) +static void test_hashmap_multimap(void) { void *k1 = (void *)0, *k2 = (void *)1; struct hashmap_entry *entry; @@ -262,121 +265,116 @@ int test_hashmap_multimap(void) long found_msk; int err, bkt; - fprintf(stderr, "%s: ", __func__); - /* force collisions */ map = hashmap__new(collision_hash_fn, equal_fn, NULL); - if (CHECK(IS_ERR(map), "failed to create map: %ld\n", PTR_ERR(map))) - return 1; - + if (CHECK(IS_ERR(map), "hashmap__new", + "failed to create map: %ld\n", PTR_ERR(map))) + return; /* set up multimap: * [0] -> 1, 2, 4; * [1] -> 8, 16, 32; */ err = hashmap__append(map, k1, (void *)1); - if (CHECK(err, "failed to add k/v: %d\n", err)) - return 1; + if (CHECK(err, "elem_add", "failed to add k/v: %d\n", err)) + goto cleanup; err = hashmap__append(map, k1, (void *)2); - if (CHECK(err, "failed to add k/v: %d\n", err)) - return 1; + if (CHECK(err, "elem_add", "failed to add k/v: %d\n", err)) + goto cleanup; err = hashmap__append(map, k1, (void *)4); - if (CHECK(err, "failed to add k/v: %d\n", err)) - return 1; + if (CHECK(err, "elem_add", "failed to add k/v: %d\n", err)) + goto cleanup; err = hashmap__append(map, k2, (void *)8); - if (CHECK(err, "failed to add k/v: %d\n", err)) - return 1; + if (CHECK(err, "elem_add", "failed to add k/v: %d\n", err)) + goto cleanup; err = hashmap__append(map, k2, (void *)16); - if (CHECK(err, "failed to add k/v: %d\n", err)) - return 1; + if (CHECK(err, "elem_add", "failed to add k/v: %d\n", err)) + goto cleanup; err = hashmap__append(map, k2, (void *)32); - if (CHECK(err, "failed to add k/v: %d\n", err)) - return 1; + if (CHECK(err, "elem_add", "failed to add k/v: %d\n", err)) + goto cleanup; - if (CHECK(hashmap__size(map) != 6, + if (CHECK(hashmap__size(map) != 6, "hashmap_size", "invalid map size: %zu\n", hashmap__size(map))) - return 1; + goto cleanup; /* verify global iteration still works and sees all values */ found_msk = 0; hashmap__for_each_entry(map, entry, bkt) { found_msk |= (long)entry->value; } - if (CHECK(found_msk != (1 << 6) - 1, + if (CHECK(found_msk != (1 << 6) - 1, "found_msk", "not all keys iterated: %lx\n", found_msk)) - return 1; + goto cleanup; /* iterate values for key 1 */ found_msk = 0; hashmap__for_each_key_entry(map, entry, k1) { found_msk |= (long)entry->value; } - if (CHECK(found_msk != (1 | 2 | 4), + if (CHECK(found_msk != (1 | 2 | 4), "found_msk", "invalid k1 values: %lx\n", found_msk)) - return 1; + goto cleanup; /* iterate values for key 2 */ found_msk = 0; hashmap__for_each_key_entry(map, entry, k2) { found_msk |= (long)entry->value; } - if (CHECK(found_msk != (8 | 16 | 32), + if (CHECK(found_msk != (8 | 16 | 32), "found_msk", "invalid k2 values: %lx\n", found_msk)) - return 1; + goto cleanup; - fprintf(stderr, "OK\n"); - return 0; +cleanup: + hashmap__free(map); } -int test_hashmap_empty() +static void test_hashmap_empty() { struct hashmap_entry *entry; int bkt; struct hashmap *map; void *k = (void *)0; - fprintf(stderr, "%s: ", __func__); - /* force collisions */ map = hashmap__new(hash_fn, equal_fn, NULL); - if (CHECK(IS_ERR(map), "failed to create map: %ld\n", PTR_ERR(map))) - return 1; + if (CHECK(IS_ERR(map), "hashmap__new", + "failed to create map: %ld\n", PTR_ERR(map))) + goto cleanup; - if (CHECK(hashmap__size(map) != 0, + if (CHECK(hashmap__size(map) != 0, "hashmap__size", "invalid map size: %zu\n", hashmap__size(map))) - return 1; - if (CHECK(hashmap__capacity(map) != 0, + goto cleanup; + if (CHECK(hashmap__capacity(map) != 0, "hashmap__capacity", "invalid map capacity: %zu\n", hashmap__capacity(map))) - return 1; - if (CHECK(hashmap__find(map, k, NULL), "unexpected find\n")) - return 1; - if (CHECK(hashmap__delete(map, k, NULL, NULL), "unexpected delete\n")) - return 1; + goto cleanup; + if (CHECK(hashmap__find(map, k, NULL), "elem_find", + "unexpected find\n")) + goto cleanup; + if (CHECK(hashmap__delete(map, k, NULL, NULL), "elem_del", + "unexpected delete\n")) + goto cleanup; hashmap__for_each_entry(map, entry, bkt) { - CHECK(false, "unexpected iterated entry\n"); - return 1; + CHECK(false, "elem_found", "unexpected iterated entry\n"); + goto cleanup; } hashmap__for_each_key_entry(map, entry, k) { - CHECK(false, "unexpected key entry\n"); - return 1; + CHECK(false, "key_found", "unexpected key entry\n"); + goto cleanup; } - fprintf(stderr, "OK\n"); - return 0; +cleanup: + hashmap__free(map); } -int main(int argc, char **argv) +void test_hashmap() { - bool failed = false; - - if (test_hashmap_generic()) - failed = true; - if (test_hashmap_multimap()) - failed = true; - if (test_hashmap_empty()) - failed = true; - - return failed; + if (test__start_subtest("generic")) + test_hashmap_generic(); + if (test__start_subtest("multimap")) + test_hashmap_multimap(); + if (test__start_subtest("empty")) + test_hashmap_empty(); } -- 2.24.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next 4/6] libbpf: fix memory leak and possible double-free in hashmap__clear 2020-04-28 4:46 [PATCH bpf-next 0/6] Add ASAN to selftest and fix found problems Andrii Nakryiko ` (2 preceding siblings ...) 2020-04-28 4:46 ` [PATCH bpf-next 3/6] selftests/bpf: convert test_hashmap into test_progs test Andrii Nakryiko @ 2020-04-28 4:46 ` Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 5/6] selftests/bpf: fix memory leak in test selector Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 6/6] selftests/bpf: fix memory leak in extract_build_id() Andrii Nakryiko 5 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2020-04-28 4:46 UTC (permalink / raw) To: bpf, netdev, ast, daniel Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Alston Tang Fix memory leak in hashmap_clear() not freeing hashmap_entry structs for each of the remaining entries. Also NULL-out bucket list to prevent possible double-free between hashmap__clear() and hashmap__free(). Running test_progs-asan flavor clearly showed this problem. Cc: Alston Tang <alston64@fb.com> Reported-by: Alston Tang <alston64@fb.com> Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- tools/lib/bpf/hashmap.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/lib/bpf/hashmap.c b/tools/lib/bpf/hashmap.c index 54c30c802070..cffb96202e0d 100644 --- a/tools/lib/bpf/hashmap.c +++ b/tools/lib/bpf/hashmap.c @@ -59,7 +59,14 @@ struct hashmap *hashmap__new(hashmap_hash_fn hash_fn, void hashmap__clear(struct hashmap *map) { + struct hashmap_entry *cur, *tmp; + int bkt; + + hashmap__for_each_entry_safe(map, cur, tmp, bkt) { + free(cur); + } free(map->buckets); + map->buckets = NULL; map->cap = map->cap_bits = map->sz = 0; } -- 2.24.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next 5/6] selftests/bpf: fix memory leak in test selector 2020-04-28 4:46 [PATCH bpf-next 0/6] Add ASAN to selftest and fix found problems Andrii Nakryiko ` (3 preceding siblings ...) 2020-04-28 4:46 ` [PATCH bpf-next 4/6] libbpf: fix memory leak and possible double-free in hashmap__clear Andrii Nakryiko @ 2020-04-28 4:46 ` Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 6/6] selftests/bpf: fix memory leak in extract_build_id() Andrii Nakryiko 5 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2020-04-28 4:46 UTC (permalink / raw) To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko Free test selector substrings, which were strdup()'ed. Fixes: b65053cd94f4 ("selftests/bpf: Add whitelist/blacklist of test names to test_progs") Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- tools/testing/selftests/bpf/test_progs.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index b521e0a512b6..86d0020c9eec 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -420,6 +420,18 @@ static int libbpf_print_fn(enum libbpf_print_level level, return 0; } +static void free_str_set(const struct str_set *set) +{ + int i; + + if (!set) + return; + + for (i = 0; i < set->cnt; i++) + free((void *)set->strs[i]); + free(set->strs); +} + static int parse_str_list(const char *s, struct str_set *set) { char *input, *state = NULL, *next, **tmp, **strs = NULL; @@ -756,11 +768,11 @@ int main(int argc, char **argv) fprintf(stdout, "Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n", env.succ_cnt, env.sub_succ_cnt, env.skip_cnt, env.fail_cnt); - free(env.test_selector.blacklist.strs); - free(env.test_selector.whitelist.strs); + free_str_set(&env.test_selector.blacklist); + free_str_set(&env.test_selector.whitelist); free(env.test_selector.num_set); - free(env.subtest_selector.blacklist.strs); - free(env.subtest_selector.whitelist.strs); + free_str_set(&env.subtest_selector.blacklist); + free_str_set(&env.subtest_selector.whitelist); free(env.subtest_selector.num_set); return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS; -- 2.24.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next 6/6] selftests/bpf: fix memory leak in extract_build_id() 2020-04-28 4:46 [PATCH bpf-next 0/6] Add ASAN to selftest and fix found problems Andrii Nakryiko ` (4 preceding siblings ...) 2020-04-28 4:46 ` [PATCH bpf-next 5/6] selftests/bpf: fix memory leak in test selector Andrii Nakryiko @ 2020-04-28 4:46 ` Andrii Nakryiko 5 siblings, 0 replies; 11+ messages in thread From: Andrii Nakryiko @ 2020-04-28 4:46 UTC (permalink / raw) To: bpf, netdev, ast, daniel Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko, Song Liu getline() allocates string, which has to be freed. Cc: Song Liu <songliubraving@fb.com> Fixes: 81f77fd0deeb ("bpf: add selftest for stackmap with BPF_F_STACK_BUILD_ID") Signed-off-by: Andrii Nakryiko <andriin@fb.com> --- tools/testing/selftests/bpf/test_progs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 86d0020c9eec..93970ec1c9e9 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -351,6 +351,7 @@ int extract_build_id(char *build_id, size_t size) len = size; memcpy(build_id, line, len); build_id[len] = '\0'; + free(line); return 0; err: fclose(fp); -- 2.24.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-04-28 22:13 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-28 4:46 [PATCH bpf-next 0/6] Add ASAN to selftest and fix found problems Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 1/6] selftests/bpf: ensure test flavors use correct skeletons Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 2/6] selftests/bpf: add test_progs-asan flavor with AddressSantizer Andrii Nakryiko 2020-04-28 16:44 ` Alexei Starovoitov 2020-04-28 18:35 ` Andrii Nakryiko 2020-04-28 20:41 ` Alexei Starovoitov 2020-04-28 22:13 ` Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 3/6] selftests/bpf: convert test_hashmap into test_progs test Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 4/6] libbpf: fix memory leak and possible double-free in hashmap__clear Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 5/6] selftests/bpf: fix memory leak in test selector Andrii Nakryiko 2020-04-28 4:46 ` [PATCH bpf-next 6/6] selftests/bpf: fix memory leak in extract_build_id() Andrii Nakryiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).