From d7b67dd6f9db7bd2c49b415e901849b182ff0735 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 Sep 2025 11:17:11 -0700 Subject: perf bpf-event: Fix use-after-free in synthesis Calls to perf_env__insert_bpf_prog_info may fail as a sideband thread may already have inserted the bpf_prog_info. Such failures may yield info_linear being freed which then causes use-after-free issues with the internal bpf_prog_info info struct. Make it so that perf_env__insert_bpf_prog_info trigger early non-error paths and fix the use-after-free in perf_event__synthesize_one_bpf_prog. Add proper return error handling to perf_env__add_bpf_info (that calls perf_env__insert_bpf_prog_info) and propagate the return value in its callers. Closes: https://lore.kernel.org/lkml/CAP-5=fWJQcmUOP7MuCA2ihKnDAHUCOBLkQFEkQES-1ZZTrgf8Q@mail.gmail.com/ Fixes: 03edb7020bb9 ("perf bpf: Fix two memory leakages when calling perf_env__insert_bpf_prog_info()") Reviewed-by: Namhyung Kim Signed-off-by: Ian Rogers Link: https://lore.kernel.org/r/20250902181713.309797-2-irogers@google.com Signed-off-by: Namhyung Kim --- tools/perf/util/bpf-event.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) (limited to 'tools/perf') diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c index 5b6d3e899e11..2298cd396c42 100644 --- a/tools/perf/util/bpf-event.c +++ b/tools/perf/util/bpf-event.c @@ -657,9 +657,15 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session, info_node->info_linear = info_linear; info_node->metadata = NULL; if (!perf_env__insert_bpf_prog_info(env, info_node)) { - free(info_linear); + /* + * Insert failed, likely because of a duplicate event + * made by the sideband thread. Ignore synthesizing the + * metadata. + */ free(info_node); + goto out; } + /* info_linear is now owned by info_node and shouldn't be freed below. */ info_linear = NULL; /* @@ -827,18 +833,18 @@ int perf_event__synthesize_bpf_events(struct perf_session *session, return err; } -static void perf_env__add_bpf_info(struct perf_env *env, u32 id) +static int perf_env__add_bpf_info(struct perf_env *env, u32 id) { struct bpf_prog_info_node *info_node; struct perf_bpil *info_linear; struct btf *btf = NULL; u64 arrays; u32 btf_id; - int fd; + int fd, err = 0; fd = bpf_prog_get_fd_by_id(id); if (fd < 0) - return; + return -EINVAL; arrays = 1UL << PERF_BPIL_JITED_KSYMS; arrays |= 1UL << PERF_BPIL_JITED_FUNC_LENS; @@ -852,6 +858,7 @@ static void perf_env__add_bpf_info(struct perf_env *env, u32 id) info_linear = get_bpf_prog_info_linear(fd, arrays); if (IS_ERR_OR_NULL(info_linear)) { pr_debug("%s: failed to get BPF program info. aborting\n", __func__); + err = PTR_ERR(info_linear); goto out; } @@ -862,38 +869,46 @@ static void perf_env__add_bpf_info(struct perf_env *env, u32 id) info_node->info_linear = info_linear; info_node->metadata = bpf_metadata_create(&info_linear->info); if (!perf_env__insert_bpf_prog_info(env, info_node)) { + pr_debug("%s: duplicate add bpf info request for id %u\n", + __func__, btf_id); free(info_linear); free(info_node); + goto out; } - } else + } else { free(info_linear); + err = -ENOMEM; + goto out; + } if (btf_id == 0) goto out; btf = btf__load_from_kernel_by_id(btf_id); - if (libbpf_get_error(btf)) { - pr_debug("%s: failed to get BTF of id %u, aborting\n", - __func__, btf_id); - goto out; + if (!btf) { + err = -errno; + pr_debug("%s: failed to get BTF of id %u %d\n", __func__, btf_id, err); + } else { + perf_env__fetch_btf(env, btf_id, btf); } - perf_env__fetch_btf(env, btf_id, btf); out: btf__free(btf); close(fd); + return err; } static int bpf_event__sb_cb(union perf_event *event, void *data) { struct perf_env *env = data; + int ret = 0; if (event->header.type != PERF_RECORD_BPF_EVENT) return -1; switch (event->bpf.type) { case PERF_BPF_EVENT_PROG_LOAD: - perf_env__add_bpf_info(env, event->bpf.id); + ret = perf_env__add_bpf_info(env, event->bpf.id); case PERF_BPF_EVENT_PROG_UNLOAD: /* @@ -907,7 +922,7 @@ static int bpf_event__sb_cb(union perf_event *event, void *data) break; } - return 0; + return ret; } int evlist__add_bpf_sb_event(struct evlist *evlist, struct perf_env *env) -- cgit v1.2.3 From 1654a0e4d576d9e43fbb10ccf6a1b307c5c18566 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 Sep 2025 11:17:12 -0700 Subject: perf bpf-utils: Constify bpil_array_desc The array's contents is a compile time constant. Constify to make the code more intention revealing and avoid unintended errors. Reviewed-by: Namhyung Kim Signed-off-by: Ian Rogers Link: https://lore.kernel.org/r/20250902181713.309797-3-irogers@google.com Signed-off-by: Namhyung Kim --- tools/perf/util/bpf-utils.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) (limited to 'tools/perf') diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c index 80b1d2b3729b..64a558344696 100644 --- a/tools/perf/util/bpf-utils.c +++ b/tools/perf/util/bpf-utils.c @@ -20,7 +20,7 @@ struct bpil_array_desc { */ }; -static struct bpil_array_desc bpil_array_desc[] = { +static const struct bpil_array_desc bpil_array_desc[] = { [PERF_BPIL_JITED_INSNS] = { offsetof(struct bpf_prog_info, jited_prog_insns), offsetof(struct bpf_prog_info, jited_prog_len), @@ -129,12 +129,10 @@ get_bpf_prog_info_linear(int fd, __u64 arrays) /* step 2: calculate total size of all arrays */ for (i = PERF_BPIL_FIRST_ARRAY; i < PERF_BPIL_LAST_ARRAY; ++i) { + const struct bpil_array_desc *desc = &bpil_array_desc[i]; bool include_array = (arrays & (1UL << i)) > 0; - struct bpil_array_desc *desc; __u32 count, size; - desc = bpil_array_desc + i; - /* kernel is too old to support this field */ if (info_len < desc->array_offset + sizeof(__u32) || info_len < desc->count_offset + sizeof(__u32) || @@ -163,13 +161,12 @@ get_bpf_prog_info_linear(int fd, __u64 arrays) ptr = info_linear->data; for (i = PERF_BPIL_FIRST_ARRAY; i < PERF_BPIL_LAST_ARRAY; ++i) { - struct bpil_array_desc *desc; + const struct bpil_array_desc *desc = &bpil_array_desc[i]; __u32 count, size; if ((arrays & (1UL << i)) == 0) continue; - desc = bpil_array_desc + i; count = bpf_prog_info_read_offset_u32(&info, desc->count_offset); size = bpf_prog_info_read_offset_u32(&info, desc->size_offset); bpf_prog_info_set_offset_u32(&info_linear->info, @@ -192,13 +189,12 @@ get_bpf_prog_info_linear(int fd, __u64 arrays) /* step 6: verify the data */ for (i = PERF_BPIL_FIRST_ARRAY; i < PERF_BPIL_LAST_ARRAY; ++i) { - struct bpil_array_desc *desc; + const struct bpil_array_desc *desc = &bpil_array_desc[i]; __u32 v1, v2; if ((arrays & (1UL << i)) == 0) continue; - desc = bpil_array_desc + i; v1 = bpf_prog_info_read_offset_u32(&info, desc->count_offset); v2 = bpf_prog_info_read_offset_u32(&info_linear->info, desc->count_offset); @@ -224,13 +220,12 @@ void bpil_addr_to_offs(struct perf_bpil *info_linear) int i; for (i = PERF_BPIL_FIRST_ARRAY; i < PERF_BPIL_LAST_ARRAY; ++i) { - struct bpil_array_desc *desc; + const struct bpil_array_desc *desc = &bpil_array_desc[i]; __u64 addr, offs; if ((info_linear->arrays & (1UL << i)) == 0) continue; - desc = bpil_array_desc + i; addr = bpf_prog_info_read_offset_u64(&info_linear->info, desc->array_offset); offs = addr - ptr_to_u64(info_linear->data); @@ -244,13 +239,12 @@ void bpil_offs_to_addr(struct perf_bpil *info_linear) int i; for (i = PERF_BPIL_FIRST_ARRAY; i < PERF_BPIL_LAST_ARRAY; ++i) { - struct bpil_array_desc *desc; + const struct bpil_array_desc *desc = &bpil_array_desc[i]; __u64 addr, offs; if ((info_linear->arrays & (1UL << i)) == 0) continue; - desc = bpil_array_desc + i; offs = bpf_prog_info_read_offset_u64(&info_linear->info, desc->array_offset); addr = offs + ptr_to_u64(info_linear->data); -- cgit v1.2.3 From 01be43f2a0eaeed83e94dee054742f37625c86d9 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 2 Sep 2025 11:17:13 -0700 Subject: perf bpf-utils: Harden get_bpf_prog_info_linear In get_bpf_prog_info_linear two calls to bpf_obj_get_info_by_fd are made, the first to compute memory requirements for a struct perf_bpil and the second to fill it in. Previously the code would warn when the second call didn't match the first. Such races can be common place in things like perf test, whose perf trace tests will frequently load BPF programs. Rather than a debug message, return actual errors for this case. Out of paranoia also validate the read bpf_prog_info array value. Change the type of ptr to avoid mismatched pointer type compiler warnings. Add some additional debug print outs and sanity asserts. Closes: https://lore.kernel.org/lkml/CAP-5=fWJQcmUOP7MuCA2ihKnDAHUCOBLkQFEkQES-1ZZTrgf8Q@mail.gmail.com/ Fixes: 6ac22d036f86 ("perf bpf: Pull in bpf_program__get_prog_info_linear()") Reviewed-by: Namhyung Kim Signed-off-by: Ian Rogers Link: https://lore.kernel.org/r/20250902181713.309797-4-irogers@google.com Signed-off-by: Namhyung Kim --- tools/perf/util/bpf-utils.c | 43 +++++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) (limited to 'tools/perf') diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c index 64a558344696..5a66dc8594aa 100644 --- a/tools/perf/util/bpf-utils.c +++ b/tools/perf/util/bpf-utils.c @@ -115,7 +115,7 @@ get_bpf_prog_info_linear(int fd, __u64 arrays) __u32 info_len = sizeof(info); __u32 data_len = 0; int i, err; - void *ptr; + __u8 *ptr; if (arrays >> PERF_BPIL_LAST_ARRAY) return ERR_PTR(-EINVAL); @@ -126,6 +126,8 @@ get_bpf_prog_info_linear(int fd, __u64 arrays) pr_debug("can't get prog info: %s", strerror(errno)); return ERR_PTR(-EFAULT); } + if (info.type >= __MAX_BPF_PROG_TYPE) + pr_debug("%s:%d: unexpected program type %u\n", __func__, __LINE__, info.type); /* step 2: calculate total size of all arrays */ for (i = PERF_BPIL_FIRST_ARRAY; i < PERF_BPIL_LAST_ARRAY; ++i) { @@ -173,6 +175,8 @@ get_bpf_prog_info_linear(int fd, __u64 arrays) desc->count_offset, count); bpf_prog_info_set_offset_u32(&info_linear->info, desc->size_offset, size); + assert(ptr >= info_linear->data); + assert(ptr < &info_linear->data[data_len]); bpf_prog_info_set_offset_u64(&info_linear->info, desc->array_offset, ptr_to_u64(ptr)); @@ -186,26 +190,45 @@ get_bpf_prog_info_linear(int fd, __u64 arrays) free(info_linear); return ERR_PTR(-EFAULT); } + if (info_linear->info.type >= __MAX_BPF_PROG_TYPE) { + pr_debug("%s:%d: unexpected program type %u\n", + __func__, __LINE__, info_linear->info.type); + } /* step 6: verify the data */ + ptr = info_linear->data; for (i = PERF_BPIL_FIRST_ARRAY; i < PERF_BPIL_LAST_ARRAY; ++i) { const struct bpil_array_desc *desc = &bpil_array_desc[i]; - __u32 v1, v2; + __u32 count1, count2, size1, size2; + __u64 ptr2; if ((arrays & (1UL << i)) == 0) continue; - v1 = bpf_prog_info_read_offset_u32(&info, desc->count_offset); - v2 = bpf_prog_info_read_offset_u32(&info_linear->info, + count1 = bpf_prog_info_read_offset_u32(&info, desc->count_offset); + count2 = bpf_prog_info_read_offset_u32(&info_linear->info, desc->count_offset); - if (v1 != v2) - pr_warning("%s: mismatch in element count\n", __func__); + if (count1 != count2) { + pr_warning("%s: mismatch in element count %u vs %u\n", __func__, count1, count2); + free(info_linear); + return ERR_PTR(-ERANGE); + } - v1 = bpf_prog_info_read_offset_u32(&info, desc->size_offset); - v2 = bpf_prog_info_read_offset_u32(&info_linear->info, + size1 = bpf_prog_info_read_offset_u32(&info, desc->size_offset); + size2 = bpf_prog_info_read_offset_u32(&info_linear->info, desc->size_offset); - if (v1 != v2) - pr_warning("%s: mismatch in rec size\n", __func__); + if (size1 != size2) { + pr_warning("%s: mismatch in rec size %u vs %u\n", __func__, size1, size2); + free(info_linear); + return ERR_PTR(-ERANGE); + } + ptr2 = bpf_prog_info_read_offset_u64(&info_linear->info, desc->array_offset); + if (ptr_to_u64(ptr) != ptr2) { + pr_warning("%s: mismatch in array %p vs %llx\n", __func__, ptr, ptr2); + free(info_linear); + return ERR_PTR(-ERANGE); + } + ptr += roundup(count1 * size1, sizeof(__u64)); } /* step 7: update info_len and data_len */ -- cgit v1.2.3 From 666d2206f1ee8a4f21ffbec438381a524a62815b Mon Sep 17 00:00:00 2001 From: James Clark Date: Wed, 3 Sep 2025 16:15:26 +0100 Subject: perf tests: Fix "PE file support" test build filename__read_build_id() now takes a blocking/non-blocking argument. The original behavior of filename__read_build_id() was blocking so add block=true to fix the build. Fixes: 2c369d91d093 ("perf symbol: Add blocking argument to filename__read_build_id") Signed-off-by: James Clark Reviewed-by: Ian Rogers Reviewed-by: Arnaldo Carvalho de Melo Link: https://lore.kernel.org/r/20250903-james-perf-read-build-id-fix-v1-1-6a694d0a980f@linaro.org Signed-off-by: Namhyung Kim --- tools/perf/tests/pe-file-parsing.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tools/perf') diff --git a/tools/perf/tests/pe-file-parsing.c b/tools/perf/tests/pe-file-parsing.c index 30c7da79e109..8b31d1d05f90 100644 --- a/tools/perf/tests/pe-file-parsing.c +++ b/tools/perf/tests/pe-file-parsing.c @@ -37,7 +37,7 @@ static int run_dir(const char *d) size_t idx; scnprintf(filename, PATH_MAX, "%s/pe-file.exe", d); - ret = filename__read_build_id(filename, &bid); + ret = filename__read_build_id(filename, &bid, /*block=*/true); TEST_ASSERT_VAL("Failed to read build_id", ret == sizeof(expect_build_id)); TEST_ASSERT_VAL("Wrong build_id", !memcmp(bid.data, expect_build_id, @@ -49,7 +49,7 @@ static int run_dir(const char *d) !strcmp(debuglink, expect_debuglink)); scnprintf(debugfile, PATH_MAX, "%s/%s", d, debuglink); - ret = filename__read_build_id(debugfile, &bid); + ret = filename__read_build_id(debugfile, &bid, /*block=*/true); TEST_ASSERT_VAL("Failed to read debug file build_id", ret == sizeof(expect_build_id)); TEST_ASSERT_VAL("Wrong build_id", !memcmp(bid.data, expect_build_id, -- cgit v1.2.3 From 744175e972ac7c1f1be53556e186e5976e39f735 Mon Sep 17 00:00:00 2001 From: Thomas Richter Date: Fri, 22 Aug 2025 14:25:39 +0200 Subject: perf test: Checking BPF metadata collection fails on version string commit edf2cadf01e8f ("perf test: add test for BPF metadata collection") fails consistently on the version string check. The perf version string on some of the constant integration test machines contains characters with special meaning in grep's extended regular expression matching algorithm. The output of perf version is: # perf version perf version 6.17.0-20250814.rc1.git20.24ea63ea3877.63.fc42.s390x+git # and the '+' character has special meaning in egrep command. Also the use of egrep is deprecated. Change the perf version string check to fixed character matching and get rid of egrep's warning being deprecated. Use grep -F instead. Output before: # perf test -F 102 Checking BPF metadata collection egrep: warning: egrep is obsolescent; using grep -E Basic BPF metadata test [Failed invalid output] 102: BPF metadata collection test : FAILED! # Output after: # perf test -F 102 Checking BPF metadata collection Basic BPF metadata test [Success] 102: BPF metadata collection test : Ok # Fixes: edf2cadf01e8f ("perf test: add test for BPF metadata collection") Signed-off-by: Thomas Richter Reviewed-by: Ian Rogers Reviewed-by: Arnaldo Carvalho de Melo Acked-by: Sumanth Korikkar Cc: Blake Jones Link: https://lore.kernel.org/r/20250822122540.4104658-1-tmricht@linux.ibm.com Signed-off-by: Namhyung Kim --- tools/perf/tests/shell/test_bpf_metadata.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf') diff --git a/tools/perf/tests/shell/test_bpf_metadata.sh b/tools/perf/tests/shell/test_bpf_metadata.sh index 69e3c2055134..be67d56e0f09 100755 --- a/tools/perf/tests/shell/test_bpf_metadata.sh +++ b/tools/perf/tests/shell/test_bpf_metadata.sh @@ -61,7 +61,7 @@ test_bpf_metadata() { /perf_version/ { if (entry) print $NF; } - ' | egrep "$VERS" > /dev/null + ' | grep -qF "$VERS" then echo "Basic BPF metadata test [Failed invalid output]" err=1 -- cgit v1.2.3 From ca81e74dc34734078d34485d4aa123561ba75b15 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 4 Sep 2025 09:17:31 -0700 Subject: perf symbol-elf: Add support for the block argument for libbfd James Clark caught that the BUILD_NONDISTRO=1 build with libbfd was broken due to an update to the read_build_id function adding a blocking argument. Add support for this argument by first opening the file blocking or non-blocking, then switching from bfd_openr to bfd_fdopenr and passing the opened fd. bfd_fdopenr closes the fd on error and when bfd_close are called. Reported-by: James Clark Closes: https://lore.kernel.org/lkml/20250903-james-perf-read-build-id-fix-v1-2-6a694d0a980f@linaro.org/ Fixes: 2c369d91d093 ("perf symbol: Add blocking argument to filename__read_build_id") Signed-off-by: Ian Rogers Reviewed-by: James Clark Link: https://lore.kernel.org/r/20250904161731.1193729-1-irogers@google.com Signed-off-by: Namhyung Kim --- tools/perf/util/symbol-elf.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'tools/perf') diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 033c79231a54..1346fd180653 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -873,13 +873,17 @@ out: #ifdef HAVE_LIBBFD_BUILDID_SUPPORT -static int read_build_id(const char *filename, struct build_id *bid) +static int read_build_id(const char *filename, struct build_id *bid, bool block) { size_t size = sizeof(bid->data); - int err = -1; + int err = -1, fd; bfd *abfd; - abfd = bfd_openr(filename, NULL); + fd = open(filename, block ? O_RDONLY : (O_RDONLY | O_NONBLOCK)); + if (fd < 0) + return -1; + + abfd = bfd_fdopenr(filename, /*target=*/NULL, fd); if (!abfd) return -1; -- cgit v1.2.3