Re: [PATCH v1 3/3] perf kwork: Fix memory management of kwork_work

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The top_print_work function passes work->name to printf without verifying it is non-NULL, which can cause a crash.
--

commit 03666bc88a2d8e65da244923eb1ef63e0a473153
Author: Ian Rogers <irogers@google.com>

perf kwork: Fix memory management of kwork_work

This commit implements a global cleanup function to free all kwork_work
and kwork_atom_page objects at exit. It also fixes memory leaks during
task merging and ensures all name fields in struct kwork_work are properly
allocated and freed, while adding robustness checks for NULL names.

> diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
> index a4604e1520023..f793ea578515d 100644
> --- a/tools/perf/builtin-kwork.c
> +++ b/tools/perf/builtin-kwork.c
[ ... ]
> @@ -1002,13 +1051,16 @@ 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>");
>  	}
>  }
>  
>  static void irq_work_name(struct kwork_work *work, char *buf, int len)
>  {
> -	snprintf(buf, len, "%s:%" PRIu64 "", work->name, work->id);
> +	if (work->name != NULL)
> +		snprintf(buf, len, "%s:%" PRIu64 "", work->name, work->id);
> +	else
> +		snprintf(buf, len, "%" PRIu64 "", work->id);
>  }

[Severity: Medium]
This is a pre-existing issue, but while you are adding NULL checks for
work->name here, should top_print_work() also be updated?

If strdup() fails due to memory pressure, or if bpf_map_lookup_elem() fails
in BPF mode, work->name can be NULL. In top_print_work(), work->name is
passed directly to printf() as a string, which can cause a segmentation fault
on strict libc implementations:

tools/perf/builtin-kwork.c:top_print_work() {
    ...
	if (kwork->use_bpf)
		ret += printf(" %s%s%s",
			      work->is_kthread ? "[" : "",
			      work->name,
			      work->is_kthread ? "]" : "");
	else
		ret += printf(" %-*s", PRINT_TASK_NAME_WIDTH, work->name);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521072429.1283423-1-irogers@google.com?part=3