Re: [PATCH v5 9/9] perf debuginfo: Fix libdw API contract violations

Re: [PATCH v5 9/9] perf debuginfo: Fix libdw API contract violations

[Ian Rogers]

On Mon, May 4, 2026 at 3:53 AM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] This problem wasn't introduced by this patch, but the open() call in debuginfo__init_offline_dwarf() lacks the O_NONBLOCK flag, exposing a local Denial of Service (DoS) vulnerability.
> --
>
> commit a6e339ae66465b909762a437f958cdaed424b666
> Author: Ian Rogers <irogers@google.com>
>
> perf debuginfo: Fix libdw API contract violations
>
> This commit adds return value checks for dwfl_report_end and
> dwfl_module_relocation_info to prevent potential segmentation faults. It also
> fixes a file descriptor leak in the error path of
> debuginfo__init_offline_dwarf() when offline initialization fails.
>
> > diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
> > index 0e35c13abd04..84a78b30ceac 100644
> > --- a/tools/perf/util/debuginfo.c
> > +++ b/tools/perf/util/debuginfo.c
> > @@ -42,6 +42,7 @@ static int debuginfo__init_offline_dwarf(struct debuginfo *dbg,
> >  {
> >       GElf_Addr dummy;
> >       int fd;
> > +     bool fd_consumed = false;
> >
> >       fd = open(path, O_RDONLY);
>
> While this wasn't introduced by this patch, could this open() call cause a
> regression by hanging on a FIFO?
>
> If the path originates from user-controlled sources, opening it without
> O_NONBLOCK can cause the perf record session to hang indefinitely. Using
> O_RDONLY | O_NONBLOCK would prevent this.

So this review is bad and based on this prompt:
https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/perf.md#file-io-safety-on-untrusted-paths

I'm trying to get the bad prompt removed. The prompt was automatically
generated from fixes commits when we added O_NONBLOCK to address build
ID reading hangs, particularly for data mmaps during perf record
synthesis. We now avoid those hangs by checking for a "regular" file
with stat. Therefore, the advice regarding O_NONBLOCK and EINTR from
the prompts is incorrect and does not reflect what the code base does.

Side note: this is the only Sashiko feedback on this series:
https://sashiko.dev/#/patchset/20260504081227.2203848-1-irogers%40google.com

Thanks,
Ian


> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260504081227.2203848-1-irogers@google.com?part=9

Re: [PATCH v5 9/9] perf debuginfo: Fix libdw API contract violations

[Arnaldo Carvalho de Melo]

On Mon, May 04, 2026 at 08:26:34AM -0700, Ian Rogers wrote:
> On Mon, May 4, 2026 at 3:53 AM <sashiko-bot@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Critical] This problem wasn't introduced by this patch, but the open() call in debuginfo__init_offline_dwarf() lacks the O_NONBLOCK flag, exposing a local Denial of Service (DoS) vulnerability.
> > --
> >
> > commit a6e339ae66465b909762a437f958cdaed424b666
> > Author: Ian Rogers <irogers@google.com>
> >
> > perf debuginfo: Fix libdw API contract violations
> >
> > This commit adds return value checks for dwfl_report_end and
> > dwfl_module_relocation_info to prevent potential segmentation faults. It also
> > fixes a file descriptor leak in the error path of
> > debuginfo__init_offline_dwarf() when offline initialization fails.
> >
> > > diff --git a/tools/perf/util/debuginfo.c b/tools/perf/util/debuginfo.c
> > > index 0e35c13abd04..84a78b30ceac 100644
> > > --- a/tools/perf/util/debuginfo.c
> > > +++ b/tools/perf/util/debuginfo.c
> > > @@ -42,6 +42,7 @@ static int debuginfo__init_offline_dwarf(struct debuginfo *dbg,
> > >  {
> > >       GElf_Addr dummy;
> > >       int fd;
> > > +     bool fd_consumed = false;
> > >
> > >       fd = open(path, O_RDONLY);
> >
> > While this wasn't introduced by this patch, could this open() call cause a
> > regression by hanging on a FIFO?
> >
> > If the path originates from user-controlled sources, opening it without
> > O_NONBLOCK can cause the perf record session to hang indefinitely. Using
> > O_RDONLY | O_NONBLOCK would prevent this.
> 
> So this review is bad and based on this prompt:
> https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/perf.md#file-io-safety-on-untrusted-paths
> 
> I'm trying to get the bad prompt removed. The prompt was automatically
> generated from fixes commits when we added O_NONBLOCK to address build
> ID reading hangs, particularly for data mmaps during perf record
> synthesis. We now avoid those hangs by checking for a "regular" file
> with stat. Therefore, the advice regarding O_NONBLOCK and EINTR from
> the prompts is incorrect and does not reflect what the code base does.
> 
> Side note: this is the only Sashiko feedback on this series:
> https://sashiko.dev/#/patchset/20260504081227.2203848-1-irogers%40google.com

I'll process it soon,

- Arnaldo