Re: [PATCH 14/15] perf symbols: Add O_NONBLOCK to DSO open() calls for untrusted paths

[Ian Rogers]

On Thu, Jun 11, 2026 at 5:35 PM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
>
> DSO paths from perf.data MMAP events could reference FIFOs, device
> nodes, or other special files.  Opening such paths without O_NONBLOCK
> can hang perf report/script indefinitely waiting for a writer or device.
>
> Add O_NONBLOCK to all open() calls in the DSO symbol resolution paths:
> filename__read_build_id(), filename__read_debuglink(), symsrc__init(),
> do_open(), filename__decompress(), and __dso__read_symbol().
>
> O_NONBLOCK is a no-op on regular files but prevents blocking on special
> files, following the systemd "foreign file" convention.  Also add missing
> O_CLOEXEC flags in symbol-minimal.c for consistency.

I think these changes are wrong and based on the bad O_NONBLOCK prompt
I fixed. I did the original addition of O_NONBLOCK and James
fixed/removed it with:
https://lore.kernel.org/linux-perf-users/20251124-james-perf-non-block-v2-1-54cdc2f13a24@xxxxxxxxxx/
As we don't read from files with TEMP_FAILURE_RETRY the use of
O_NONBLOCK is going to mean slow file systems fail. The
is_regular_file tests are intended to avoid the hangs described in the
commit message.

Thanks,
Ian

> Fixes: a08cae03f430b971 ("perf tools: Allow to close dso fd in case of open failure")
> Reported-by: sashiko-bot <sashiko-bot@xxxxxxxxxx>
> Closes: https://sashiko.dev/finding/6
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Assisted-by: Claude Opus 4.6 <noreply@xxxxxxxxxxxxx>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> ---
>  tools/perf/util/dso.c            | 6 +++---
>  tools/perf/util/symbol-elf.c     | 7 ++++---
>  tools/perf/util/symbol-minimal.c | 6 +++---
>  3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 1a2fc6d18da74d6c..dc0c8b9c63017239 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -344,7 +344,7 @@ int filename__decompress(const char *name, char *pathname,
>          * descriptor to the uncompressed file.
>          */
>         if (!compressions[comp].is_compressed(name)) {
> -               fd = open(name, O_RDONLY | O_CLOEXEC);
> +               fd = open(name, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>                 if (fd < 0)
>                         *err = errno;
>                 if (pathname && len > 0)
> @@ -547,7 +547,7 @@ static void close_first_dso(void);
>  static int do_open(char *name) EXCLUSIVE_LOCKS_REQUIRED(_dso__data_open_lock)
>  {
>         do {
> -               int fd = open(name, O_RDONLY|O_CLOEXEC);
> +               int fd = open(name, O_RDONLY|O_CLOEXEC|O_NONBLOCK);
>
>                 if (fd >= 0)
>                         return fd;
> @@ -1927,7 +1927,7 @@ static const u8 *__dso__read_symbol(struct dso *dso, const char *symfs_filename,
>         int saved_errno;
>
>         nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
> -       fd = open(symfs_filename, O_RDONLY | O_CLOEXEC);
> +       fd = open(symfs_filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         saved_errno = errno;
>         nsinfo__mountns_exit(&nsc);
>         if (fd < 0) {
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index dc48e2d2763379b9..01c7b09fc82bcddb 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -886,7 +886,8 @@ static int read_build_id(const char *filename, struct build_id *bid)
>         if (size < BUILD_ID_SIZE)
>                 goto out;
>
> -       fd = open(filename, O_RDONLY | O_CLOEXEC);
> +       /* O_NONBLOCK avoids hangs on FIFOs/devices from crafted perf.data paths */
> +       fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         if (fd < 0)
>                 goto out;
>
> @@ -1022,7 +1023,7 @@ int filename__read_debuglink(const char *filename, char *debuglink,
>         if (err >= 0)
>                 goto out;
>
> -       fd = open(filename, O_RDONLY | O_CLOEXEC);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         if (fd < 0)
>                 goto out;
>
> @@ -1187,7 +1188,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>
>                 type = dso__symtab_type(dso);
>         } else {
> -               fd = open(name, O_RDONLY | O_CLOEXEC);
> +               fd = open(name, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>                 if (fd < 0) {
>                         *dso__load_errno(dso) = errno;
>                         return -1;
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index ea2de3d50d33cf33..dc4a06b2a03ad3cb 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -118,7 +118,7 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
>         if (!is_regular_file(filename))
>                 return errno == 0 ? -EWOULDBLOCK : -errno;
>
> -       fd = open(filename, O_RDONLY);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         if (fd < 0)
>                 return -1;
>
> @@ -234,7 +234,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
>         char buf[BUFSIZ] __aligned(4);
>         ssize_t len;
>
> -       fd = open(filename, O_RDONLY);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         if (fd < 0)
>                 return -1;
>
> @@ -257,7 +257,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
>  int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>                  enum dso_binary_type type)
>  {
> -       int fd = open(name, O_RDONLY);
> +       int fd = open(name, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         if (fd < 0)
>                 goto out_errno;
>
> --
> 2.54.0
>