* [PATCHES v4 0/2] perf c2c hardening
@ 2026-08-03 14:41 Arnaldo Carvalho de Melo
2026-08-03 14:41 ` [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() Arnaldo Carvalho de Melo
2026-08-03 14:41 ` [PATCH 2/2] perf c2c: Clean up registered formats on c2c_hists__init() failure Arnaldo Carvalho de Melo
0 siblings, 2 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-08-03 14:41 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo
Hi,
Fixes for 'perf c2c' found by the sashiko-bot AI reviewer.
Patch 1 fixes three silent failure modes in hpp_list__parse().
Patch 2 fixes a format list leak: when c2c_hists__init() fails partway
through, entries registered via perf_hpp_list__column_register() and
perf_hpp_list__register_sort_field() are left on the hpp_list.
Please consider merging,
- Arnaldo
Changes since v3:
- Use 'goto out' to break out from both the switch and the for loop in
__hpp_list__parse() as noticed by sashiko.
Changes since v2:
- Added the string.h and stdlib.h missing headers.
- Addressed sashiko comment on PARSE_LIST, turning it into a function
and handling all errors returned from _fn().
Changes since v1:
- Addressed sashiko-bot [Medium] finding: the early exits added by v1
skip perf_hpp__setup_output_field().
- Dismissed sashiko-bot [Low] finding (missing string.h/stdlib.h):
strdup() and free() were already used in this function before this
patch; string2.h (included at the top of the file) pulls in string.h.
No new library calls were introduced.
- Subject updated to reflect the additional caller fixes.
This series was developed with AI assistance (Claude Sonnet 4.6).
Arnaldo Carvalho de Melo (2):
perf c2c: Fix error masking, OOM, and unchecked caller errors in
hpp_list__parse()
perf c2c: Clean up registered formats on c2c_hists__init() failure
tools/perf/builtin-c2c.c | 90 ++++++++++++++++++++++++++++------------
tools/perf/util/sort.c | 38 ++++++++++-------
2 files changed, 86 insertions(+), 42 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() 2026-08-03 14:41 [PATCHES v4 0/2] perf c2c hardening Arnaldo Carvalho de Melo @ 2026-08-03 14:41 ` Arnaldo Carvalho de Melo 2026-08-03 15:02 ` sashiko-bot 2026-08-03 14:41 ` [PATCH 2/2] perf c2c: Clean up registered formats on c2c_hists__init() failure Arnaldo Carvalho de Melo 1 sibling, 1 reply; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2026-08-03 14:41 UTC (permalink / raw) To: Namhyung Kim Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers, Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot From: Arnaldo Carvalho de Melo <acme@redhat.com> hpp_list__parse() has three bugs: 1. The PARSE_LIST macro resets ret = 0 at the start of each invocation, so an error from output parsing is silently overwritten when the sort parsing block runs. The function returns success with partially initialized state. 2. When the caller passes a non-NULL output_ or sort_ string, but strdup() returns NULL due to OOM, NULL is passed to PARSE_LIST which treats it as empty input (the "if (!_list) break" branch). No error is returned. 3. When the called _fn function fails and returns something other than -ESRCH or -EINVAL (-ENOMEM, for instance) it was not bailing out of the strtok loop. Fix them by checking strdup() return values before proceeding and adding a cleanup label so that ret from each PARSE_LIST call is checked before the next runs, preserving the first error. The early exits now skip perf_hpp__setup_output_field(), which means c2c_hists__reinit() can return a non-zero value in cases that previously always succeeded silently. Both callers discarded its return: resort_cl_cb() continued into hists__collapse_resort() on a broken list, and perf_c2c__report() proceeded with uninitialised hists. Fix the full chain: check and propagate the error in resort_cl_cb() -- hists__iterate_cb() already stops iteration and returns the callback error -- and check both c2c_hists__reinit() and hists__iterate_cb() in perf_c2c__report(). Also turn PARSE_LIST into a function, using a switch to catch other errors, converting the called functions to return an appropriate errno instead of -1 on failure. Fixes: 2d388bd0c9d3 ("perf c2c report: Add stdio output support") Reported-by: sashiko-bot <sashiko-bot@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Assisted-by: Claude:claude-sonnet-4.6 Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/builtin-c2c.c | 80 +++++++++++++++++++++++++++------------- tools/perf/util/sort.c | 38 +++++++++++-------- 2 files changed, 77 insertions(+), 41 deletions(-) diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c index c9584dbedf77afe8..b0f2ba8318304914 100644 --- a/tools/perf/builtin-c2c.c +++ b/tools/perf/builtin-c2c.c @@ -12,6 +12,8 @@ */ #include <errno.h> #include <inttypes.h> +#include <stdlib.h> +#include <string.h> #include <asm/bug.h> #include <linux/compiler.h> @@ -2063,26 +2065,34 @@ static int c2c_hists__init_sort(struct perf_hpp_list *hpp_list, char *name, stru return 0; } -#define PARSE_LIST(_list, _fn) \ - do { \ - char *tmp, *tok; \ - ret = 0; \ - \ - if (!_list) \ - break; \ - \ - for (tok = strtok_r((char *)_list, ", ", &tmp); \ - tok; tok = strtok_r(NULL, ", ", &tmp)) { \ - ret = _fn(hpp_list, tok, env); \ - if (ret == -EINVAL) { \ - pr_err("Invalid --fields key: `%s'", tok); \ - break; \ - } else if (ret == -ESRCH) { \ - pr_err("Unknown --fields key: `%s'", tok); \ - break; \ - } \ - } \ - } while (0) +static int __hpp_list__parse(struct perf_hpp_list *hpp_list, char *_list, struct perf_env *env, + int (*_fn)(struct perf_hpp_list *hpp_list, char *name, struct perf_env *env)) +{ + char *tmp, *tok; + int ret = 0; + + if (!_list) + return 0; + + for (tok = strtok_r(_list, ", ", &tmp); tok; tok = strtok_r(NULL, ", ", &tmp)) { + ret = _fn(hpp_list, tok, env); + switch (ret) { + case 0: + continue; + case -EINVAL: + pr_err("Invalid --fields key: `%s'", tok); + goto out; + case -ESRCH: + pr_err("Unknown --fields key: `%s'", tok); + goto out; + default: + pr_err("%m for --fields key: `%s'", tok); + goto out; + } + } +out: + return ret; +} static int hpp_list__parse(struct perf_hpp_list *hpp_list, const char *output_, @@ -2093,8 +2103,18 @@ static int hpp_list__parse(struct perf_hpp_list *hpp_list, char *sort = sort_ ? strdup(sort_) : NULL; int ret; - PARSE_LIST(output, c2c_hists__init_output); - PARSE_LIST(sort, c2c_hists__init_sort); + /* strdup() returns NULL on OOM, don't silently treat as empty */ + if ((output_ && !output) || (sort_ && !sort)) { + ret = -ENOMEM; + goto out; + } + + ret = __hpp_list__parse(hpp_list, output, env, c2c_hists__init_output); + if (ret) + goto out; + ret = __hpp_list__parse(hpp_list, sort, env, c2c_hists__init_sort); + if (ret) + goto out; /* copy sort keys to output fields */ perf_hpp__setup_output_field(hpp_list); @@ -2111,6 +2131,7 @@ static int hpp_list__parse(struct perf_hpp_list *hpp_list, perf_hpp__append_sort_keys(&hists->list); #endif +out: free(output); free(sort); return ret; @@ -2281,6 +2302,7 @@ static int resort_cl_cb(struct hist_entry *he, void *arg) struct c2c_hist_entry *c2c_he; struct c2c_hists *c2c_hists; bool display = he__display(he, &c2c.shared_clines_stats); + int ret; c2c_he = container_of(he, struct c2c_hist_entry, he); c2c_hists = c2c_he->hists; @@ -2291,7 +2313,9 @@ static int resort_cl_cb(struct hist_entry *he, void *arg) c2c_he->cacheline_idx = idx++; calc_width(c2c_he); - c2c_hists__reinit(c2c_hists, c2c.cl_output, c2c.cl_resort, env); + ret = c2c_hists__reinit(c2c_hists, c2c.cl_output, c2c.cl_resort, env); + if (ret) + return ret; hists__collapse_resort(&c2c_hists->hists, NULL); hists__output_resort_cb(&c2c_hists->hists, NULL, filter_cb); @@ -3356,13 +3380,19 @@ static int perf_c2c__report(int argc, const char **argv) else if (c2c.display == DISPLAY_SNP_PEER) sort_str = "tot_peer"; - c2c_hists__reinit(&c2c.hists, output_str, sort_str, perf_session__env(session)); + err = c2c_hists__reinit(&c2c.hists, output_str, sort_str, perf_session__env(session)); + if (err) { + pr_err("Failed to reinitialize hists\n"); + goto out_mem2node; + } ui_progress__init(&prog, c2c.hists.hists.nr_entries, "Sorting..."); hists__collapse_resort(&c2c.hists.hists, NULL); hists__output_resort_cb(&c2c.hists.hists, &prog, resort_shared_cl_cb); - hists__iterate_cb(&c2c.hists.hists, resort_cl_cb, perf_session__env(session)); + err = hists__iterate_cb(&c2c.hists.hists, resort_cl_cb, perf_session__env(session)); + if (err) + goto out_mem2node; ui_progress__finish(); diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index dcf9189786f8aeba..2f59e37089ef06a2 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -3105,7 +3105,7 @@ static int __sort_dimension__add_hpp_sort(struct sort_dimension *sd, struct hpp_sort_entry *hse = __sort_dimension__alloc_hpp(sd, level); if (hse == NULL) - return -1; + return -ENOMEM; perf_hpp_list__register_sort_field(list, &hse->hpp); return 0; @@ -3118,7 +3118,7 @@ static int __sort_dimension__add_hpp_output(struct sort_dimension *sd, struct hpp_sort_entry *hse = __sort_dimension__alloc_hpp(sd, level); if (hse == NULL) - return -1; + return -ENOMEM; perf_hpp_list__column_register(list, &hse->hpp); return 0; @@ -3742,14 +3742,18 @@ static int __sort_dimension__add(struct sort_dimension *sd, struct perf_hpp_list *list, int level) { + int ret; + if (sd->taken) return 0; - if (__sort_dimension__add_hpp_sort(sd, list, level) < 0) - return -1; + ret = __sort_dimension__add_hpp_sort(sd, list, level); + if (ret < 0) + return ret; - if (__sort_dimension__update(sd, list) < 0) - return -1; + ret = __sort_dimension__update(sd, list); + if (ret < 0) + return ret; sd->taken = 1; @@ -3767,7 +3771,7 @@ static int __hpp_dimension__add(struct hpp_dimension *hd, fmt = __hpp_dimension__alloc_hpp(hd, level); if (!fmt) - return -1; + return -ENOMEM; hd->taken = 1; hd->was_taken = 1; @@ -3779,14 +3783,18 @@ static int __sort_dimension__add_output(struct perf_hpp_list *list, struct sort_dimension *sd, int level) { + int ret; + if (sd->taken) return 0; - if (__sort_dimension__add_hpp_output(sd, list, level) < 0) - return -1; + ret = __sort_dimension__add_hpp_output(sd, list, level); + if (ret < 0) + return ret; - if (__sort_dimension__update(sd, list) < 0) - return -1; + ret = __sort_dimension__update(sd, list); + if (ret < 0) + return ret; sd->taken = 1; return 0; @@ -3803,7 +3811,7 @@ static int __hpp_dimension__add_output(struct perf_hpp_list *list, fmt = __hpp_dimension__alloc_hpp(hd, level); if (!fmt) - return -1; + return -ENOMEM; hd->taken = 1; perf_hpp_list__column_register(list, fmt); @@ -3869,8 +3877,7 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok, strlen(tok))) return -EINVAL; - __sort_dimension__add(sd, list, level); - return 0; + return __sort_dimension__add(sd, list, level); } for (i = 0; i < ARRAY_SIZE(memory_sort_dimensions); i++) { @@ -3882,8 +3889,7 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok, if (sort__mode != SORT_MODE__MEMORY) return -EINVAL; - __sort_dimension__add(sd, list, level); - return 0; + return __sort_dimension__add(sd, list, level); } for (i = 0; i < ARRAY_SIZE(hpp_sort_dimensions); i++) { -- 2.55.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() 2026-08-03 14:41 ` [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() Arnaldo Carvalho de Melo @ 2026-08-03 15:02 ` sashiko-bot 2026-08-03 17:14 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 6+ messages in thread From: sashiko-bot @ 2026-08-03 15:02 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The `%m` format specifier is used in `__hpp_list__parse` to log generic callback errors, which can result in incorrect or stale error messages. -- --- Patch [1]: [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() --- commit 318c268818f1c0f5bf8bb06a4099029b27a33ee1 Author: Arnaldo Carvalho de Melo <acme@redhat.com> perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() This commit addresses multiple bugs in hpp_list__parse(). It ensures that errors from output and sort parsing are not silently masked, handles OOM conditions from strdup() correctly, and properly propagates failures out of the strtok loop and up the call chain. > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c > index c9584dbedf77a..b0f2ba8318304 100644 > --- a/tools/perf/builtin-c2c.c > +++ b/tools/perf/builtin-c2c.c [ ... ] > @@ -2063,26 +2065,34 @@ static int c2c_hists__init_sort(struct perf_hpp_list *hpp_list, char *name, stru [ ... ] > +static int __hpp_list__parse(struct perf_hpp_list *hpp_list, char *_list, struct perf_env *env, > + int (*_fn)(struct perf_hpp_list *hpp_list, char *name, struct perf_env *env)) > +{ > + char *tmp, *tok; > + int ret = 0; > + > + if (!_list) > + return 0; > + > + for (tok = strtok_r(_list, ", ", &tmp); tok; tok = strtok_r(NULL, ", ", &tmp)) { > + ret = _fn(hpp_list, tok, env); > + switch (ret) { > + case 0: > + continue; > + case -EINVAL: > + pr_err("Invalid --fields key: `%s'", tok); > + goto out; > + case -ESRCH: > + pr_err("Unknown --fields key: `%s'", tok); > + goto out; > + default: > + pr_err("%m for --fields key: `%s'", tok); [Severity: Medium] Will %m print the correct error message here? The _fn callback returns a negative error code in ret (such as -ENOMEM), but it does not necessarily set errno. Even if it did, if the callback or its callees log anything, functions called internally by pr_err() (like fprintf or localtime_r) can clobber errno even on success. This could cause %m to print a stale or incorrect error string (e.g., "No such file or directory" instead of "Cannot allocate memory"). Could we format the string using the actual error code available in ret instead of relying on the implicit global errno variable? > + goto out; > + } > + } > +out: > + return ret; > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/20260803144119.185637-1-acme@kernel.org?part=1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() 2026-08-03 15:02 ` sashiko-bot @ 2026-08-03 17:14 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2026-08-03 17:14 UTC (permalink / raw) To: sashiko-reviews; +Cc: linux-perf-users On Mon, Aug 03, 2026 at 03:02:29PM +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] The `%m` format specifier is used in `__hpp_list__parse` to log generic callback errors, which can result in incorrect or stale error messages. > -- > > --- Patch [1]: [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() --- > commit 318c268818f1c0f5bf8bb06a4099029b27a33ee1 > Author: Arnaldo Carvalho de Melo <acme@redhat.com> > > perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() > > This commit addresses multiple bugs in hpp_list__parse(). It ensures that errors > from output and sort parsing are not silently masked, handles OOM conditions > from strdup() correctly, and properly propagates failures out of the strtok loop > and up the call chain. > > > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c > > index c9584dbedf77a..b0f2ba8318304 100644 > > --- a/tools/perf/builtin-c2c.c > > +++ b/tools/perf/builtin-c2c.c > [ ... ] > > @@ -2063,26 +2065,34 @@ static int c2c_hists__init_sort(struct perf_hpp_list *hpp_list, char *name, stru > [ ... ] > > +static int __hpp_list__parse(struct perf_hpp_list *hpp_list, char *_list, struct perf_env *env, > > + int (*_fn)(struct perf_hpp_list *hpp_list, char *name, struct perf_env *env)) > > +{ > > + char *tmp, *tok; > > + int ret = 0; > > + > > + if (!_list) > > + return 0; > > + > > + for (tok = strtok_r(_list, ", ", &tmp); tok; tok = strtok_r(NULL, ", ", &tmp)) { > > + ret = _fn(hpp_list, tok, env); > > + switch (ret) { > > + case 0: > > + continue; > > + case -EINVAL: > > + pr_err("Invalid --fields key: `%s'", tok); > > + goto out; > > + case -ESRCH: > > + pr_err("Unknown --fields key: `%s'", tok); > > + goto out; > > + default: > > + pr_err("%m for --fields key: `%s'", tok); > > [Severity: Medium] > Will %m print the correct error message here? > > The _fn callback returns a negative error code in ret (such as -ENOMEM), but it > does not necessarily set errno. > > Even if it did, if the callback or its callees log anything, functions called > internally by pr_err() (like fprintf or localtime_r) can clobber errno even on > success. This could cause %m to print a stale or incorrect error string (e.g., > "No such file or directory" instead of "Cannot allocate memory"). > > Could we format the string using the actual error code available in ret instead > of relying on the implicit global errno variable? Indeed, I'll use str_error_r() in v5. - Arnaldo > > + goto out; > > + } > > + } > > +out: > > + return ret; > > +} > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20260803144119.185637-1-acme@kernel.org?part=1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] perf c2c: Clean up registered formats on c2c_hists__init() failure 2026-08-03 14:41 [PATCHES v4 0/2] perf c2c hardening Arnaldo Carvalho de Melo 2026-08-03 14:41 ` [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() Arnaldo Carvalho de Melo @ 2026-08-03 14:41 ` Arnaldo Carvalho de Melo 2026-08-03 15:07 ` sashiko-bot 1 sibling, 1 reply; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2026-08-03 14:41 UTC (permalink / raw) To: Namhyung Kim Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers, Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users, Arnaldo Carvalho de Melo, sashiko-bot From: Arnaldo Carvalho de Melo <acme@redhat.com> When c2c_hists__init() calls hpp_list__parse() and it fails partway through, format structures registered via perf_hpp_list__column_register() and perf_hpp_list__register_sort_field() are left on the hpp_list. The c2c_he__alloc_hists() caller handles this with perf_hpp__reset_output_field(), but perf_c2c_report() does not, leaking the partially registered entries. Fix by adding cleanup inside c2c_hists__init() itself, so all callers are protected. Fixes: 78b275437873 ("perf c2c report: Add sample processing") Reported-by: sashiko-bot <sashiko-bot@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Assisted-by: Claude:claude-opus-4.6 Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/builtin-c2c.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c index b0f2ba8318304914..e66e9ca8618dc927 100644 --- a/tools/perf/builtin-c2c.c +++ b/tools/perf/builtin-c2c.c @@ -2142,6 +2142,8 @@ static int c2c_hists__init(struct c2c_hists *hists, int nr_header_lines, struct perf_env *env) { + int ret; + __hists__init(&hists->hists, &hists->list); /* @@ -2154,7 +2156,13 @@ static int c2c_hists__init(struct c2c_hists *hists, /* Overload number of header lines.*/ hists->list.nr_header_lines = nr_header_lines; - return hpp_list__parse(&hists->list, /*output=*/NULL, sort, env); + ret = hpp_list__parse(&hists->list, /*output=*/NULL, sort, env); + + /* Unregister any formats added before the failure point */ + if (ret) + perf_hpp__reset_output_field(&hists->list); + + return ret; } static int c2c_hists__reinit(struct c2c_hists *c2c_hists, -- 2.55.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] perf c2c: Clean up registered formats on c2c_hists__init() failure 2026-08-03 14:41 ` [PATCH 2/2] perf c2c: Clean up registered formats on c2c_hists__init() failure Arnaldo Carvalho de Melo @ 2026-08-03 15:07 ` sashiko-bot 0 siblings, 0 replies; 6+ messages in thread From: sashiko-bot @ 2026-08-03 15:07 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] Unconditional memory leak of dynamically allocated format structures in `print_pareto()`. - [High] Architectural/API Misuse: `perf_hpp__setup_output_field()` and `perf_hpp__append_sort_keys()` silently corrupt the global `perf_hpp_list`. -- --- Patch [2]: [PATCH 2/2] perf c2c: Clean up registered formats on c2c_hists__init() failure --- commit c2ba37f5e0548f4ffed858d39e640349044131b2 Author: Arnaldo Carvalho de Melo <acme@redhat.com> perf c2c: Clean up registered formats on c2c_hists__init() failure This commit addresses a memory leak when c2c_hists__init() fails partway through parsing. It ensures partially registered format structures are unregistered before returning the error. This protects callers from leaking memory during report generation. > diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c > index b0f2ba8318304..e66e9ca8618dc 100644 > --- a/tools/perf/builtin-c2c.c > +++ b/tools/perf/builtin-c2c.c > @@ -2142,6 +2142,8 @@ static int c2c_hists__init(struct c2c_hists *hists, > int nr_header_lines, > struct perf_env *env) > { > + int ret; > + > __hists__init(&hists->hists, &hists->list); > > /* > @@ -2154,7 +2156,13 @@ static int c2c_hists__init(struct c2c_hists *hists, > /* Overload number of header lines.*/ > hists->list.nr_header_lines = nr_header_lines; > > - return hpp_list__parse(&hists->list, NULL, sort, env); > + ret = hpp_list__parse(&hists->list, NULL, sort, env); > + > + /* Unregister any formats added before the failure point */ > + if (ret) > + perf_hpp__reset_output_field(&hists->list); > + > + return ret; > } [Severity: High] This is a pre-existing issue, but does print_pareto() unconditionally leak dynamically allocated format structures? Every time print_pareto() is called during C2C reporting, it initializes a local stack variable for the formats: tools/perf/builtin-c2c.c:print_pareto() { struct perf_hpp_list hpp_list; ... perf_hpp_list__init(&hpp_list); ret = hpp_list__parse(&hpp_list, cl_output, NULL, env); ... } However, when the function exits on both success and error paths, it doesn't call perf_hpp__reset_output_field() on hpp_list. Does this mean the formats attached to this local list are permanently leaked when the list goes out of scope? [Severity: High] This is a pre-existing issue, but do perf_hpp__setup_output_field() and perf_hpp__append_sort_keys() unintentionally corrupt the global perf_hpp_list instead of updating the passed local list? When these functions are called with a local list, they iterate over the local list but invoke the register functions directly: tools/perf/ui/hist.c:perf_hpp__setup_output_field() { ... perf_hpp__column_register(fmt); ... } tools/perf/ui/hist.c:perf_hpp__append_sort_keys() { ... perf_hpp__register_sort_field(fmt); ... } These wrappers appear to be hardcoded to insert elements into the global list: tools/perf/util/hist.h: #define perf_hpp__column_register(format) \ perf_hpp_list__column_register(&perf_hpp_list, format) Does this leave the required fields missing from the local list, while simultaneously polluting the global perf_hpp_list state with dynamically allocated objects? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260803144119.185637-1-acme@kernel.org?part=2 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-03 17:14 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-03 14:41 [PATCHES v4 0/2] perf c2c hardening Arnaldo Carvalho de Melo 2026-08-03 14:41 ` [PATCH 1/2] perf c2c: Fix error masking, OOM, and unchecked caller errors in hpp_list__parse() Arnaldo Carvalho de Melo 2026-08-03 15:02 ` sashiko-bot 2026-08-03 17:14 ` Arnaldo Carvalho de Melo 2026-08-03 14:41 ` [PATCH 2/2] perf c2c: Clean up registered formats on c2c_hists__init() failure Arnaldo Carvalho de Melo 2026-08-03 15:07 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox