Re: [PATCH v7 04/23] perf kwork: Duplicate IRQ name in irq_work_init

Namhyung Kim
2026-04-09T01:55:04Z

On Wed, Apr 08, 2026 at 08:08:11AM +0000, sashiko-bot@kernel.org wrote:
> Sashiko AI review found 2 potential issue(s):
> - [High] Massive per-event memory leak of dynamically allocated IRQ names when updating existing `kwork_work` entries.
> - [Medium] Missing check for `strdup()` allocation failure leads to undefined behavior in `snprintf()`.
> --
>
> commit fd542621f59f12c7e1282f77ef0817d45c3b81a0
> Author: Ian Rogers <irogers@google.com>
>
> perf kwork: Duplicate IRQ name in irq_work_init
>
> This commit duplicates the IRQ name string when initializing a kwork work
> item to avoid lifecycle issues with pointers into the perf event sample.
> It also updates perf_sample__strval to return a const char pointer.
>
> > diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
> > index f600037ee4603..dceb9b79aa6ac 100644
> > --- a/tools/perf/builtin-kwork.c
> > +++ b/tools/perf/builtin-kwork.c
> > @@ -1018,7 +1018,7 @@ static void irq_work_init(struct perf_kwork *kwork,
> > work->name = NULL;
> > } else {
> > work->id = perf_sample__intval(sample, "irq");
> > - work->name = perf_sample__strval(sample, "name");
> > + work->name = strdup(perf_sample__strval(sample, "name") ?: "<unknown>");
>
> Does this code leak the allocated name string when processing existing work
> items?
>
> In event processing loops like work_push_atom() and work_pop_atom(), a
> temporary kwork_work struct is allocated on the stack for every single trace
> event. If the item is already present in the RB-tree, work_findnew() or
> work_search() returns the existing kwork_work object which already has its
> name populated.

Similarly, I think you can drop this patch unless you really want to fix
the memory problems in kwork code base properly.

Thanks,
Namhyung

>
> Since the newly allocated name string on the temporary structure is ignored
> for existing items, does this leak the duplicated string for every repeated
> IRQ trace event?
>
> What happens if strdup() fails and returns NULL here?
>
> Later in irq_work_name(), work->name is passed directly to snprintf() with
> the %s format specifier:
>
> static void irq_work_name(struct kwork_work *work, char *buf, int len)
> {
> snprintf(buf, len, "%s:%" PRIu64 "", work->name, work->id);
> }
>
> Could this regression cause a segmentation fault during memory pressure if
> work->name becomes NULL?
>
> > }
> > }
>
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260408072601.210837-1-irogers@google.com?part=4