From 2a2cb45b727b7a1041f3d3d93414b774e66454bb Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Sun, 7 Nov 2021 08:55:13 -0800 Subject: [PATCH 1/9] selftests/bpf: Pass sanitizer flags to linker through LDFLAGS When adding -fsanitize=address to SAN_CFLAGS, it has to be passed both to compiler through CFLAGS as well as linker through LDFLAGS. Add SAN_CFLAGS into LDFLAGS to allow building selftests with ASAN. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Reviewed-by: Hengqi Chen Link: https://lore.kernel.org/bpf/20211107165521.9240-2-andrii@kernel.org --- tools/testing/selftests/bpf/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index e19cc6473936..0468ea57650d 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -24,6 +24,7 @@ SAN_CFLAGS ?= CFLAGS += -g -O0 -rdynamic -Wall $(GENFLAGS) $(SAN_CFLAGS) \ -I$(CURDIR) -I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \ -I$(TOOLSINCDIR) -I$(APIDIR) -I$(OUTPUT) +LDFLAGS += $(SAN_CFLAGS) LDLIBS += -lcap -lelf -lz -lrt -lpthread # Silence some warnings when compiled with clang From 8f7b239ea8cfdc8e64c875ee417fed41431a1f37 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Sun, 7 Nov 2021 08:55:14 -0800 Subject: [PATCH 2/9] libbpf: Free up resources used by inner map definition It's not enough to just free(map->inner_map), as inner_map itself can have extra memory allocated, like map name. Fixes: 646f02ffdd49 ("libbpf: Add BTF-defined map-in-map support") Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Reviewed-by: Hengqi Chen Link: https://lore.kernel.org/bpf/20211107165521.9240-3-andrii@kernel.org --- tools/lib/bpf/libbpf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index dfd15cc60ea7..d869ebee1e27 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -9009,7 +9009,10 @@ int bpf_map__set_inner_map_fd(struct bpf_map *map, int fd) pr_warn("error: inner_map_fd already specified\n"); return libbpf_err(-EINVAL); } - zfree(&map->inner_map); + if (map->inner_map) { + bpf_map__destroy(map->inner_map); + zfree(&map->inner_map); + } map->inner_map_fd = fd; return 0; } From 8ba285874913da21ca39a46376e9cc5ce0f45f94 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Sun, 7 Nov 2021 08:55:15 -0800 Subject: [PATCH 3/9] selftests/bpf: Fix memory leaks in btf_type_c_dump() helper Free up memory and resources used by temporary allocated memstream and btf_dump instance. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Reviewed-by: Hengqi Chen Link: https://lore.kernel.org/bpf/20211107165521.9240-4-andrii@kernel.org --- tools/testing/selftests/bpf/btf_helpers.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/btf_helpers.c b/tools/testing/selftests/bpf/btf_helpers.c index b5b6b013a245..3d1a748d09d8 100644 --- a/tools/testing/selftests/bpf/btf_helpers.c +++ b/tools/testing/selftests/bpf/btf_helpers.c @@ -251,18 +251,23 @@ const char *btf_type_c_dump(const struct btf *btf) d = btf_dump__new(btf, NULL, &opts, btf_dump_printf); if (libbpf_get_error(d)) { fprintf(stderr, "Failed to create btf_dump instance: %ld\n", libbpf_get_error(d)); - return NULL; + goto err_out; } for (i = 1; i < btf__type_cnt(btf); i++) { err = btf_dump__dump_type(d, i); if (err) { fprintf(stderr, "Failed to dump type [%d]: %d\n", i, err); - return NULL; + goto err_out; } } + btf_dump__free(d); fflush(buf_file); fclose(buf_file); return buf; +err_out: + btf_dump__free(d); + fclose(buf_file); + return NULL; } From b8b26e585f3a0fbcee1032c622f046787da57390 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Sun, 7 Nov 2021 08:55:16 -0800 Subject: [PATCH 4/9] selftests/bpf: Free per-cpu values array in bpf_iter selftest Array holding per-cpu values wasn't freed. Fix that. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Link: https://lore.kernel.org/bpf/20211107165521.9240-5-andrii@kernel.org --- tools/testing/selftests/bpf/prog_tests/bpf_iter.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c index 9454331aaf85..3e10abce3e5a 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c @@ -699,14 +699,13 @@ static void test_bpf_percpu_hash_map(void) char buf[64]; void *val; - val = malloc(8 * bpf_num_possible_cpus()); - skel = bpf_iter_bpf_percpu_hash_map__open(); if (CHECK(!skel, "bpf_iter_bpf_percpu_hash_map__open", "skeleton open failed\n")) return; skel->rodata->num_cpus = bpf_num_possible_cpus(); + val = malloc(8 * bpf_num_possible_cpus()); err = bpf_iter_bpf_percpu_hash_map__load(skel); if (CHECK(!skel, "bpf_iter_bpf_percpu_hash_map__load", @@ -770,6 +769,7 @@ free_link: bpf_link__destroy(link); out: bpf_iter_bpf_percpu_hash_map__destroy(skel); + free(val); } static void test_bpf_array_map(void) @@ -870,14 +870,13 @@ static void test_bpf_percpu_array_map(void) void *val; int len; - val = malloc(8 * bpf_num_possible_cpus()); - skel = bpf_iter_bpf_percpu_array_map__open(); if (CHECK(!skel, "bpf_iter_bpf_percpu_array_map__open", "skeleton open failed\n")) return; skel->rodata->num_cpus = bpf_num_possible_cpus(); + val = malloc(8 * bpf_num_possible_cpus()); err = bpf_iter_bpf_percpu_array_map__load(skel); if (CHECK(!skel, "bpf_iter_bpf_percpu_array_map__load", @@ -933,6 +932,7 @@ free_link: bpf_link__destroy(link); out: bpf_iter_bpf_percpu_array_map__destroy(skel); + free(val); } /* An iterator program deletes all local storage in a map. */ From 5309b516bcc6f76dda0e44a7a1824324277093d6 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Sun, 7 Nov 2021 08:55:17 -0800 Subject: [PATCH 5/9] selftests/bpf: Free inner strings index in btf selftest Inner array of allocated strings wasn't freed on success. Now it's always freed. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Reviewed-by: Hengqi Chen Link: https://lore.kernel.org/bpf/20211107165521.9240-6-andrii@kernel.org --- tools/testing/selftests/bpf/prog_tests/btf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c index ac596cb06e40..ebd1aa4d09d6 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf.c +++ b/tools/testing/selftests/bpf/prog_tests/btf.c @@ -4046,11 +4046,9 @@ static void *btf_raw_create(const struct btf_header *hdr, next_str_idx < strs_cnt ? strs_idx[next_str_idx] : NULL; done: + free(strs_idx); if (err) { - if (raw_btf) - free(raw_btf); - if (strs_idx) - free(strs_idx); + free(raw_btf); return NULL; } return raw_btf; From f79587520a6007a3734b23a3c2eb4c62aa457533 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Sun, 7 Nov 2021 08:55:18 -0800 Subject: [PATCH 6/9] selftests/bpf: Clean up btf and btf_dump in dump_datasec test Free up used resources at the end and on error. Also make it more obvious that there is btf__parse() call that creates struct btf instance. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Reviewed-by: Hengqi Chen Link: https://lore.kernel.org/bpf/20211107165521.9240-7-andrii@kernel.org --- tools/testing/selftests/bpf/prog_tests/btf_dump.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/btf_dump.c b/tools/testing/selftests/bpf/prog_tests/btf_dump.c index aa76360d8f49..a04961942dfa 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_dump.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_dump.c @@ -814,21 +814,25 @@ static void test_btf_datasec(struct btf *btf, struct btf_dump *d, char *str, static void test_btf_dump_datasec_data(char *str) { - struct btf *btf = btf__parse("xdping_kern.o", NULL); + struct btf *btf; struct btf_dump_opts opts = { .ctx = str }; char license[4] = "GPL"; struct btf_dump *d; + btf = btf__parse("xdping_kern.o", NULL); if (!ASSERT_OK_PTR(btf, "xdping_kern.o BTF not found")) return; d = btf_dump__new(btf, NULL, &opts, btf_dump_snprintf); if (!ASSERT_OK_PTR(d, "could not create BTF dump")) - return; + goto out; test_btf_datasec(btf, d, str, "license", "SEC(\"license\") char[4] _license = (char[4])['G','P','L',];", license, sizeof(license)); +out: + btf_dump__free(d); + btf__free(btf); } void test_btf_dump() { From f92321d706a810b89a905e04658e38931c4bb0e0 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Sun, 7 Nov 2021 08:55:19 -0800 Subject: [PATCH 7/9] selftests/bpf: Avoid duplicate btf__parse() call btf__parse() is repeated after successful setup, leaving the first instance leaked. Remove redundant and premature call. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Reviewed-by: Hengqi Chen Link: https://lore.kernel.org/bpf/20211107165521.9240-8-andrii@kernel.org --- tools/testing/selftests/bpf/prog_tests/core_reloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c index 55ec85ba7375..1041d0c593f6 100644 --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c @@ -433,7 +433,7 @@ static int setup_type_id_case_local(struct core_reloc_test_case *test) static int setup_type_id_case_success(struct core_reloc_test_case *test) { struct core_reloc_type_id_output *exp = (void *)test->output; - struct btf *targ_btf = btf__parse(test->btf_src_file, NULL); + struct btf *targ_btf; int err; err = setup_type_id_case_local(test); From f91231eeeed752119f49eb6620cae44ec745a007 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Sun, 7 Nov 2021 08:55:20 -0800 Subject: [PATCH 8/9] selftests/bpf: Destroy XDP link correctly bpf_link__detach() was confused with bpf_link__destroy() and leaves leaked FD in the process. Fix the problem. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Reviewed-by: Hengqi Chen Link: https://lore.kernel.org/bpf/20211107165521.9240-9-andrii@kernel.org --- tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c b/tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c index 7589c03fd26b..eb2feaac81fe 100644 --- a/tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c +++ b/tools/testing/selftests/bpf/prog_tests/migrate_reuseport.c @@ -204,8 +204,8 @@ static int pass_ack(struct migrate_reuseport_test_case *test_case) { int err; - err = bpf_link__detach(test_case->link); - if (!ASSERT_OK(err, "bpf_link__detach")) + err = bpf_link__destroy(test_case->link); + if (!ASSERT_OK(err, "bpf_link__destroy")) return -1; test_case->link = NULL; From 8c7a95520184b6677ca6075e12df9c208d57d088 Mon Sep 17 00:00:00 2001 From: Andrii Nakryiko Date: Sun, 7 Nov 2021 08:55:21 -0800 Subject: [PATCH 9/9] selftests/bpf: Fix bpf_object leak in skb_ctx selftest skb_ctx selftest didn't close bpf_object implicitly allocated by bpf_prog_test_load() helper. Fix the problem by explicitly calling bpf_object__close() at the end of the test. Signed-off-by: Andrii Nakryiko Signed-off-by: Alexei Starovoitov Reviewed-by: Hengqi Chen Link: https://lore.kernel.org/bpf/20211107165521.9240-10-andrii@kernel.org --- tools/testing/selftests/bpf/prog_tests/skb_ctx.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c index d3106078838c..b5319ba2ee27 100644 --- a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c +++ b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c @@ -111,4 +111,6 @@ void test_skb_ctx(void) "ctx_out_mark", "skb->mark == %u, expected %d\n", skb.mark, 10); + + bpf_object__close(obj); }