Two related hardening fixes for the build-id cache population path (perf_session__cache_build_ids, called at the end of perf record):
1. build_id_cache__add(): Before hard-linking or copying a file into the cache, verify it is a valid ELF using the shared is_valid_elf() helper (introduced in patch 1/3). If it is not, emit a warning and skip the cache write. This prevents a race condition that can occur with test harnesses such as SPEC CPU, which briefly rename the benchmark binary to a .used.<pid> path and write a shell script placeholder there during run-directory cleanup. If perf's cache write races with that window -- seeing the renamed path in /proc/<pid>/maps and reading the build-id from the still-valid inode, then copying the file after the placeholder has been written -- the cache ends up containing the shell script instead of the ELF. The ELF magic check catches this at copy time. 2. dso__build_id_mismatch(): Previously, if filename__read_build_id_ns() failed (e.g. the file is not an ELF, or has been replaced), the function returned false (no mismatch), allowing caching to proceed with whatever file happened to be at that path. Change the default return value to true (mismatch) so that an unreadable build-id is treated conservatively as a mismatch and caching is skipped. Together these ensure that only genuine ELF binaries with a verifiable build-id matching what perf recorded are written into the cache. Reported-by: Narendra Nalli <[email protected]> Reported-by: Vijay Puliyala <[email protected]> Signed-off-by: Athira Rajeev <[email protected]> --- tools/perf/util/build-id.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c index 28b6b3f8d5d3..32ddbd5e61f6 100644 --- a/tools/perf/util/build-id.c +++ b/tools/perf/util/build-id.c @@ -684,6 +684,10 @@ build_id_cache__add(const char *sbuild_id, const char *name, const char *realnam if (is_kallsyms) { if (copyfile("/proc/kallsyms", filename)) goto out_free; + } else if (!is_valid_elf(realname)) { + pr_warning("build-id cache: skipping non-ELF file: %s\n", + realname); + goto out_free; } else if (nsi && nsinfo__need_setns(nsi)) { if (copyfile_ns(name, filename, nsi)) goto out_free; @@ -874,7 +878,11 @@ static int filename__read_build_id_ns(const char *filename, static bool dso__build_id_mismatch(struct dso *dso, const char *name) { struct build_id bid = { .size = 0, }; - bool ret = false; + /* + * Default to mismatch: if we cannot read the build-id (e.g. file + * replaced or not an ELF), treat it conservatively as a mismatch. + */ + bool ret = true; mutex_lock(dso__lock(dso)); if (filename__read_build_id_ns(name, &bid, dso__nsinfo(dso)) >= 0) -- 2.43.0
