perf tools: Don't read build-ids from non-regular files

Simplify the build ID reading code by removing the non-blocking option.
Having to pass the correct option to this function was fragile and a
mistake would result in a hang, see the linked fix. Furthermore,
compressed files are always opened blocking anyway, ignoring the
non-blocking option.

We also don't expect to read build IDs from non-regular files. The only
hits to this function that are non-regular are devices that won't be elf
files with build IDs, for example "/dev/dri/renderD129".

Now instead of opening these as non-blocking and failing to read, we
skip them. Even if something like a pipe or character device did have a
build ID, I don't think it would have worked because you need to call
read() in a loop, check for -EAGAIN and handle timeouts to make
non-blocking reads work.

Link: https://lore.kernel.org/linux-perf-users/20251022-james-perf-fix-dso-block-v1-1-c4faab150546@linaro.org/
Signed-off-by: James Clark <james.clark@linaro.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>

diff --git a/tools/perf/bench/inject-buildid.c b/tools/perf/bench/inject-buildid.c
index 12387ea88b9a9cd204f8e2c4d0d7490c827acad4..aad572a78d7fcfdf5091d1d0be09f2e3e0365e08 100644 (file)
--- a/tools/perf/bench/inject-buildid.c
+++ b/tools/perf/bench/inject-buildid.c
@@ -85,7 +85,7 @@ static int add_dso(const char *fpath, const struct stat *sb __maybe_unused,
        if (typeflag == FTW_D || typeflag == FTW_SL)
                return 0;
 
-       if (filename__read_build_id(fpath, &bid, /*block=*/true) < 0)
+       if (filename__read_build_id(fpath, &bid) < 0)
                return 0;
 
        dso->name = realpath(fpath, NULL);

diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index 2e0f2004696ae9f6c3204fa36cec3e1cd478d0aa..c98104481c8a19e45ace6f38456c6697b1600651 100644 (file)
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -180,7 +180,7 @@ static int build_id_cache__add_file(const char *filename, struct nsinfo *nsi)
        struct nscookie nsc;
 
        nsinfo__mountns_enter(nsi, &nsc);
-       err = filename__read_build_id(filename, &bid, /*block=*/true);
+       err = filename__read_build_id(filename, &bid);
        nsinfo__mountns_exit(&nsc);
        if (err < 0) {
                pr_debug("Couldn't read a build-id in %s\n", filename);
@@ -204,7 +204,7 @@ static int build_id_cache__remove_file(const char *filename, struct nsinfo *nsi)
        int err;
 
        nsinfo__mountns_enter(nsi, &nsc);
-       err = filename__read_build_id(filename, &bid, /*block=*/true);
+       err = filename__read_build_id(filename, &bid);
        nsinfo__mountns_exit(&nsc);
        if (err < 0) {
                pr_debug("Couldn't read a build-id in %s\n", filename);
@@ -280,7 +280,7 @@ static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
        if (!dso__build_id_filename(dso, filename, sizeof(filename), false))
                return true;
 
-       if (filename__read_build_id(filename, &bid, /*block=*/true) == -1) {
+       if (filename__read_build_id(filename, &bid) == -1) {
                if (errno == ENOENT)
                        return false;
 
@@ -309,7 +309,7 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
        int err;
 
        nsinfo__mountns_enter(nsi, &nsc);
-       err = filename__read_build_id(filename, &bid, /*block=*/true);
+       err = filename__read_build_id(filename, &bid);
        nsinfo__mountns_exit(&nsc);
        if (err < 0) {
                pr_debug("Couldn't read a build-id in %s\n", filename);

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index bd9245d2dd41aa48b215ec0a43efffd6eb32c74b..0e40f2a4bd18b940b6eaeb102c5da1549a9641ec 100644 (file)
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -668,12 +668,12 @@ static int dso__read_build_id(struct dso *dso)
 
        mutex_lock(dso__lock(dso));
        nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
-       if (filename__read_build_id(dso__long_name(dso), &bid, /*block=*/true) > 0)
+       if (filename__read_build_id(dso__long_name(dso), &bid) > 0)
                dso__set_build_id(dso, &bid);
        else if (dso__nsinfo(dso)) {
                char *new_name = dso__filename_with_chroot(dso, dso__long_name(dso));
 
-               if (new_name && filename__read_build_id(new_name, &bid, /*block=*/true) > 0)
+               if (new_name && filename__read_build_id(new_name, &bid) > 0)
                        dso__set_build_id(dso, &bid);
                free(new_name);
        }

diff --git a/tools/perf/tests/pe-file-parsing.c b/tools/perf/tests/pe-file-parsing.c
index 8b31d1d05f905fdb41dd6778d68601e066dcfde4..30c7da79e109b480215c81c3522370ab571d6b90 100644 (file)
--- 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, /*block=*/true);
+       ret = filename__read_build_id(filename, &bid);
        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, /*block=*/true);
+       ret = filename__read_build_id(debugfile, &bid);
        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,

diff --git a/tools/perf/tests/sdt.c b/tools/perf/tests/sdt.c
index 6132f1af3e22d3920cc9b1e082f368e368c611f2..93baee2eae42ab989f96d5f44e0c9b743643bb9e 100644 (file)
--- a/tools/perf/tests/sdt.c
+++ b/tools/perf/tests/sdt.c
@@ -31,7 +31,7 @@ static int build_id_cache__add_file(const char *filename)
        struct build_id bid = { .size = 0, };
        int err;
 
-       err = filename__read_build_id(filename, &bid, /*block=*/true);
+       err = filename__read_build_id(filename, &bid);
        if (err < 0) {
                pr_debug("Failed to read build id of %s\n", filename);
                return err;

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 35505a1ffd11173a3877b44d6f7a876349313cf3..fdb35133fde43a1d295f64e71249243cf0b1b635 100644 (file)
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -122,7 +122,7 @@ int filename__snprintf_build_id(const char *pathname, char *sbuild_id, size_t sb
        struct build_id bid = { .size = 0, };
        int ret;
 
-       ret = filename__read_build_id(pathname, &bid, /*block=*/true);
+       ret = filename__read_build_id(pathname, &bid);
        if (ret < 0)
                return ret;
 
@@ -848,7 +848,7 @@ static int filename__read_build_id_ns(const char *filename,
        int ret;
 
        nsinfo__mountns_enter(nsi, &nsc);
-       ret = filename__read_build_id(filename, bid, /*block=*/true);
+       ret = filename__read_build_id(filename, bid);
        nsinfo__mountns_exit(&nsc);
 
        return ret;

diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
index bb9ebd84ec2d2a92ab5068f5ed5cc7625f771610..4a559b3e8cdce60aa203323aa945cfeaef268f65 100644 (file)
--- a/tools/perf/util/debuginfo.c
+++ b/tools/perf/util/debuginfo.c
@@ -115,7 +115,7 @@ struct debuginfo *debuginfo__new(const char *path)
         * incase the path isn't for a regular file.
         */
        assert(!dso__has_build_id(dso));
-       if (filename__read_build_id(path, &bid, /*block=*/false) > 0)
+       if (filename__read_build_id(path, &bid) > 0)
                dso__set_build_id(dso, &bid);
 
        for (type = distro_dwarf_types;

diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
index 64c1d65b01496146a31b24cace24d0f86ea3be03..0a7645c7fae7d3151fb8d4f60231ccb0be74fe1d 100644 (file)
--- a/tools/perf/util/dsos.c
+++ b/tools/perf/util/dsos.c
@@ -81,13 +81,13 @@ static int dsos__read_build_ids_cb(struct dso *dso, void *data)
                return 0;
        }
        nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
-       if (filename__read_build_id(dso__long_name(dso), &bid, /*block=*/true) > 0) {
+       if (filename__read_build_id(dso__long_name(dso), &bid) > 0) {
                dso__set_build_id(dso, &bid);
                args->have_build_id = true;
        } else if (errno == ENOENT && dso__nsinfo(dso)) {
                char *new_name = dso__filename_with_chroot(dso, dso__long_name(dso));
 
-               if (new_name && filename__read_build_id(new_name, &bid, /*block=*/true) > 0) {
+               if (new_name && filename__read_build_id(new_name, &bid) > 0) {
                        dso__set_build_id(dso, &bid);
                        args->have_build_id = true;
                }

diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
index 01147fbf73b37638de32a4ed44b64a5dce99d051..0099b415decca28183acc7b5ff983c93f6463921 100644 (file)
--- a/tools/perf/util/libbfd.c
+++ b/tools/perf/util/libbfd.c
@@ -383,13 +383,18 @@ out_close:
        return err;
 }
 
-int libbfd__read_build_id(const char *filename, struct build_id *bid, bool block)
+int libbfd__read_build_id(const char *filename, struct build_id *bid)
 {
        size_t size = sizeof(bid->data);
        int err = -1, fd;
        bfd *abfd;
 
-       fd = open(filename, block ? O_RDONLY : (O_RDONLY | O_NONBLOCK));
+       if (!filename)
+               return -EFAULT;
+       if (!is_regular_file(filename))
+               return -EWOULDBLOCK;
+
+       fd = open(filename, O_RDONLY);
        if (fd < 0)
                return -1;
 

diff --git a/tools/perf/util/libbfd.h b/tools/perf/util/libbfd.h
index e300f171d1bd70ff0c5363711dc581eb3369a365..953886f3d62f37abaf6c29e8f4b730ebab9c0522 100644 (file)
--- a/tools/perf/util/libbfd.h
+++ b/tools/perf/util/libbfd.h
@@ -25,7 +25,7 @@ void dso__free_a2l_libbfd(struct dso *dso);
 int symbol__disassemble_libbfd(const char *filename, struct symbol *sym,
                             struct annotate_args *args);
 
-int libbfd__read_build_id(const char *filename, struct build_id *bid, bool block);
+int libbfd__read_build_id(const char *filename, struct build_id *bid);
 
 int libbfd_filename__read_debuglink(const char *filename, char *debuglink, size_t size);
 
@@ -59,8 +59,7 @@ static inline int symbol__disassemble_libbfd(const char *filename __always_unuse
 }
 
 static inline int libbfd__read_build_id(const char *filename __always_unused,
-                                       struct build_id *bid __always_unused,
-                                       bool block __always_unused)
+                                       struct build_id *bid __always_unused)
 {
        return -1;
 }

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 9e820599bab3885e5e0dbd5701e678de3771b3e5..c5c382c3409a73280cd250200f622ca409d9e745 100644 (file)
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -860,20 +860,20 @@ out:
        return err;
 }
 
-static int read_build_id(const char *filename, struct build_id *bid, bool block)
+static int read_build_id(const char *filename, struct build_id *bid)
 {
        size_t size = sizeof(bid->data);
        int fd, err;
        Elf *elf;
 
-       err = libbfd__read_build_id(filename, bid, block);
+       err = libbfd__read_build_id(filename, bid);
        if (err >= 0)
                goto out;
 
        if (size < BUILD_ID_SIZE)
                goto out;
 
-       fd = open(filename, block ? O_RDONLY : (O_RDONLY | O_NONBLOCK));
+       fd = open(filename, O_RDONLY);
        if (fd < 0)
                goto out;
 
@@ -894,7 +894,7 @@ out:
        return err;
 }
 
-int filename__read_build_id(const char *filename, struct build_id *bid, bool block)
+int filename__read_build_id(const char *filename, struct build_id *bid)
 {
        struct kmod_path m = { .name = NULL, };
        char path[PATH_MAX];
@@ -902,6 +902,8 @@ int filename__read_build_id(const char *filename, struct build_id *bid, bool blo
 
        if (!filename)
                return -EFAULT;
+       if (!is_regular_file(filename))
+               return -EWOULDBLOCK;
 
        err = kmod_path__parse(&m, filename);
        if (err)
@@ -918,10 +920,9 @@ int filename__read_build_id(const char *filename, struct build_id *bid, bool blo
                }
                close(fd);
                filename = path;
-               block = true;
        }
 
-       err = read_build_id(filename, bid, block);
+       err = read_build_id(filename, bid);
 
        if (m.comp)
                unlink(filename);

diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index aeb2532488953a1797881ae2781ae7b35a992773..c6b17c14a2e9941e9702db4aaf04c3fd816d6451 100644 (file)
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -85,7 +85,7 @@ int filename__read_debuglink(const char *filename __maybe_unused,
 /*
  * Just try PT_NOTE header otherwise fails
  */
-int filename__read_build_id(const char *filename, struct build_id *bid, bool block)
+int filename__read_build_id(const char *filename, struct build_id *bid)
 {
        int fd, ret = -1;
        bool need_swap = false, elf32;
@@ -102,7 +102,12 @@ int filename__read_build_id(const char *filename, struct build_id *bid, bool blo
        void *phdr, *buf = NULL;
        ssize_t phdr_size, ehdr_size, buf_size = 0;
 
-       fd = open(filename, block ? O_RDONLY : (O_RDONLY | O_NONBLOCK));
+       if (!filename)
+               return -EFAULT;
+       if (!is_regular_file(filename))
+               return -EWOULDBLOCK;
+
+       fd = open(filename, O_RDONLY);
        if (fd < 0)
                return -1;
 
@@ -323,7 +328,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
        if (ret >= 0)
                RC_CHK_ACCESS(dso)->is_64_bit = ret;
 
-       if (filename__read_build_id(ss->name, &bid, /*block=*/true) > 0)
+       if (filename__read_build_id(ss->name, &bid) > 0)
                dso__set_build_id(dso, &bid);
        return 0;
 }

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index cc26b7bf302b29d6a99cafe9ffe47b67721e143a..d8fc5ea77f849326729050be8818073464b2435d 100644 (file)
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1743,14 +1743,13 @@ int dso__load(struct dso *dso, struct map *map)
 
        /*
         * Read the build id if possible. This is required for
-        * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work. Don't block in case path
-        * isn't for a regular file.
+        * DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work.
         */
        if (!dso__has_build_id(dso)) {
                struct build_id bid = { .size = 0, };
 
                __symbol__join_symfs(name, PATH_MAX, dso__long_name(dso));
-               if (filename__read_build_id(name, &bid, /*block=*/false) > 0)
+               if (filename__read_build_id(name, &bid) > 0)
                        dso__set_build_id(dso, &bid);
        }
 

diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 3471062187992aa05a09c8d33b6bff79c03d9789..3fb5d146d9b15bd406a3630ac1eb1966f75d110a 100644 (file)
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -140,7 +140,7 @@ struct symbol *dso__next_symbol(struct symbol *sym);
 
 enum dso_type dso__type_fd(int fd);
 
-int filename__read_build_id(const char *filename, struct build_id *id, bool block);
+int filename__read_build_id(const char *filename, struct build_id *id);
 int sysfs__read_build_id(const char *filename, struct build_id *bid);
 int modules__parse(const char *filename, void *arg,
                   int (*process_module)(void *arg, const char *name,

diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index 5aa44c4aba62599be6bdc83ec2ffcf9ab4db1a4b..2ba9fa25e00a68016757550ed4b3da7bd5f9ea29 100644 (file)
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -401,7 +401,7 @@ static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
        nsi = nsinfo__new(event->pid);
        nsinfo__mountns_enter(nsi, &nc);
 
-       rc = filename__read_build_id(event->filename, &bid, /*block=*/false) > 0 ? 0 : -1;
+       rc = filename__read_build_id(event->filename, &bid) > 0 ? 0 : -1;
 
        nsinfo__mountns_exit(&nc);
        nsinfo__put(nsi);