Re: [PATCH v6 1/2] perf tests: Add test for uncore event sorting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Double free of `m.event1` when scanning uncore PMUs.
--

commit 5412f65ccaa1b5aa1cce690a35fcf6de242d9132
Author: Ian Rogers <irogers@google.com>

perf tests: Add test for uncore event sorting

This commit adds a test for uncore event sorting that matches multiple PMUs.
It ensures that event groups are correctly resorted when initial parsing
results in mixed PMUs due to common prefixes.

> diff --git a/tools/perf/tests/uncore-event-sorting.c b/tools/perf/tests/uncore-event-sorting.c
> new file mode 100644
> index 0000000000000..88bda7a89a057
> --- /dev/null
> +++ b/tools/perf/tests/uncore-event-sorting.c
[ ... ]
> +static int test__uncore_event_sorting(struct test_suite *test __maybe_unused,
> +				      int subtest __maybe_unused)
> +{
> +	struct evlist *evlist = NULL;
> +	struct parse_events_error err;
> +	struct evsel *evsel;
> +	struct perf_pmu *pmu = NULL;
> +	char *pmu_prefix = NULL;
> +	struct match_state m = { NULL, NULL };
> +	char buf[1024];
> +	int ret;
> +
> +	parse_events_error__init(&err);
> +
> +	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
> +		size_t len;
> +		struct perf_pmu *sibling;
> +
> +		if (pmu->is_core)
> +			continue;
> +
> +		len = pmu_name_len_no_suffix(pmu->name);
> +		if (len == strlen(pmu->name))
> +			continue;
> +
> +		sibling = pmu;
> +		while ((sibling = perf_pmus__scan(sibling)) != NULL) {
> +			if (sibling->is_core)
> +				continue;
> +			if (pmu_name_len_no_suffix(sibling->name) == len &&
> +			    !strncmp(pmu->name, sibling->name, len))
> +				break;
> +		}
> +
> +		if (!sibling)
> +			continue;
> +
> +		m.event1 = m.event2 = NULL;
> +		perf_pmu__for_each_event(pmu, false, &m, event_cb);
> +
> +		if (m.event1 && m.event2) {
> +			pmu_prefix = strndup(pmu->name, len);
> +			break;
> +		}
> +		free(m.event1);
> +	}

Could this lead to a double free of m.event1?

If perf_pmu__for_each_event() allocates m.event1 but not m.event2, the code
falls through the condition and frees m.event1 without setting it back to NULL.

If the loop subsequently terminates, or if the next iteration triggers an early
continue before m.event1 is reset to NULL, the function will eventually
proceed to the out_err label.

[ ... ]
> +out_err:
> +	evlist__delete(evlist);
> +	parse_events_error__exit(&err);
> +	free(pmu_prefix);
> +	free(m.event1);

When it reaches this point, will it call free(m.event1) again on the dangling
pointer?

> +	free(m.event2);
> +	return ret;
> +}

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