Re: [PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] perf_hpp__setup_output_field and perf_hpp__append_sort_keys corrupt the global perf_hpp_list when passed a local list.
--

commit 22c7f5ad675216ec0ff17ec1087b36169d928283
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf c2c: Free format list entries when c2c_hists__init() fails

When c2c_hists__init() fails partway through hpp_list__parse(),
dynamically allocated format structures that were already added to
hists->list are leaked because he__get_c2c_hists() frees the hists
container without first unregistering the format entries.

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 07c7e8fb315e6..eabb922ef295e 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -226,6 +226,7 @@ he__get_c2c_hists(struct hist_entry *he,
>  
>  	ret = c2c_hists__init(hists, sort, nr_header_lines, env);
>  	if (ret) {
> +		perf_hpp__reset_output_field(&hists->list);

[Severity: High]
This is a pre-existing issue, but it looks like c2c_hists__init() triggers a
state corruption.

When c2c_hists__init() initializes the local &hists->list, does it call
perf_hpp__setup_output_field() and perf_hpp__append_sort_keys() which then
mutate the global perf_hpp_list instead of the passed local list?

Looking at tools/perf/util/hist.h:

static inline void perf_hpp__column_register(struct perf_hpp_fmt *format)
{
	perf_hpp_list__column_register(&perf_hpp_list, format);
}

When perf_hpp__setup_output_field() in tools/perf/ui/hist.c loops over the
provided local list's sort keys, it incorrectly triggers registration into the
global list via perf_hpp__column_register().

Similarly, perf_hpp__append_sort_keys() in tools/perf/ui/hist.c calls
perf_hpp__register_sort_field(), which also hardcodes insertion into the global
&perf_hpp_list.sorts.

Does this mean dynamically allocated format structures from potentially thousands
of local cachelines are erroneously appended to the global UI format list
instead of the local list?

>  		c2c_he->hists = NULL;
>  		free(hists);
>  		return NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616193033.10035-1-acme@kernel.org?part=6