[PATCH v1 00/13] perf python: Fix python extension bugs and memory leaks

[PATCH v1 00/13] perf python: Fix python extension bugs and memory leaks

[Ian Rogers]

This patch series addresses a number of leaks and python extension bugs
discovered during the review of the python scripting phase 1 refactoring
series.

These fixes have been separated from the scripting changes into their own
dedicated patch series to ease review and integration, as they address
regressions or existing bugs rather than adding new features.

The series breaks down the monolithic python extension fixes into discrete
logical patches addressing validation, initialization, and resource leaks.

Ian Rogers (13):
  perf script: Fix metric_evlist leak in script_find_metrics
  perf stat: Fix evsel_list leak in cmd_stat
  perf top: Fix sb_evlist leak
  perf python: Fix memory leak in pyrf_evlist__get_pollfd
  perf synthetic-events: Fix uninitialized pthread_join
  perf data: Fix directory file descriptor leak in perf_data__switch
  perf test: Fix skiplist leak in cmd_test
  perf python: Check counts_values size in set_values
  perf python: Validate CPU and thread maps in pyrf_evsel__open
  perf python: Validate attribute setters in pyrf_evsel
  perf python: Zero initialize perf_data in pyrf_data__init
  perf python: Add thread uninitialized checks
  perf python: Fix MetricGroup return type in perf.pyi

 tools/perf/builtin-script.c        |   2 +-
 tools/perf/builtin-stat.c          |   9 ++-
 tools/perf/builtin-top.c           |   1 +
 tools/perf/python/perf.pyi         |   4 +-
 tools/perf/tests/builtin-test.c    |   5 +-
 tools/perf/util/data.c             |   3 +-
 tools/perf/util/python.c           | 114 +++++++++++++++++++++++++----
 tools/perf/util/synthetic-events.c |   4 +-
 8 files changed, 116 insertions(+), 26 deletions(-)

-- 
2.55.0.rc0.786.g65d90a0328-goog

[PATCH v1 01/13] perf script: Fix metric_evlist leak in script_find_metrics

[Ian Rogers]

Fix a memory leak in script_find_metrics() where metric_evlist is leaked
when returning early on error paths.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 3622990efaab ("perf script: Change metric format to use json metrics")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-script.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 47afd8cdc2b7..6b69e982ba62 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2275,8 +2275,8 @@ static int script_find_metrics(const struct pmu_metric *pm,
 	}
 	pr_debug("Found metric '%s' whose evsels match those of in the perf data\n",
 		 pm->metric_name);
-	evlist__put(metric_evlist);
 out:
+	evlist__put(metric_evlist);
 	return 0;
 }
 
-- 
2.55.0.rc0.786.g65d90a0328-goog

[PATCH v1 02/13] perf stat: Fix evsel_list leak in cmd_stat

[Ian Rogers]

Fix a memory leak in cmd_stat() where evsel_list is leaked if an error
occurs while opening the output file.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 361c99a661a7 ("perf evsel: Introduce perf_evlist")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-stat.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 3f897b2e8638..7cb6ea937e87 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2718,7 +2718,8 @@ int cmd_stat(int argc, const char **argv)
 		output = fopen(output_name, mode);
 		if (!output) {
 			perror("failed to create output file");
-			return -1;
+			status = -1;
+			goto out;
 		}
 		if (!stat_config.json_output) {
 			clock_gettime(CLOCK_REALTIME, &tm);
@@ -2729,7 +2730,8 @@ int cmd_stat(int argc, const char **argv)
 		output = fdopen(output_fd, mode);
 		if (!output) {
 			perror("Failed opening logfd");
-			return -errno;
+			status = -errno;
+			goto out;
 		}
 	}
 
@@ -2738,7 +2740,8 @@ int cmd_stat(int argc, const char **argv)
 		parse_options_usage(stat_usage, stat_options, "o", 1);
 		parse_options_usage(NULL, stat_options, "log-fd", 0);
 		parse_options_usage(NULL, stat_options, "interval-clear", 0);
-		return -1;
+		status = -1;
+		goto out;
 	}
 
 	stat_config.output = output;
-- 
2.55.0.rc0.786.g65d90a0328-goog

[PATCH v1 03/13] perf top: Fix sb_evlist leak

[Ian Rogers]

Fix a memory leak in cmd_top() where top.sb_evlist was not freed if
evlist__add_bpf_sb_event() fails. Note that evlist__start_sb_thread() and
evlist__stop_sb_thread() take ownership of the evlist and free it, so
we must only free it if we fail before starting the thread.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: b38d85ef49cf ("perf bpf: Decouple creating the evlist from adding the SB event")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-top.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 5933c46ee137..abc74b8ab759 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1881,6 +1881,7 @@ int cmd_top(int argc, const char **argv)
 		if (evlist__add_bpf_sb_event(top.sb_evlist, &host_env)) {
 			pr_err("Couldn't ask for PERF_RECORD_BPF_EVENT side band events.\n.");
 			status = -EINVAL;
+			evlist__put(top.sb_evlist);
 			goto out_put_evlist;
 		}
 	}
-- 
2.55.0.rc0.786.g65d90a0328-goog

Re: [PATCH v1 03/13] perf top: Fix sb_evlist leak

[sashiko-bot]

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

Pre-existing issues:
- [High] Error path in `record__setup_sb_evlist()` bypasses state update, leading to a segmentation fault from an uninitialized `pthread_join()`.
- [High] Sideband thread and `sb_evlist` are leaked when using `--switch-output-event` together with `--no-bpf-event`.
--

commit 9bc374ffd1b7ae00f5342c051ca479b32bc92942
Author: Ian Rogers <irogers@google.com>

perf top: Fix sb_evlist leak

Fix a memory leak in cmd_top() where top.sb_evlist was not freed if
evlist__add_bpf_sb_event() fails. Note that evlist__start_sb_thread() and
evlist__stop_sb_thread() take ownership of the evlist and free it, so
we must only free it if we fail before starting the thread.

> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 5933c46ee137e..abc74b8ab7599 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1881,6 +1881,7 @@ int cmd_top(int argc, const char **argv)
>  		if (evlist__add_bpf_sb_event(top.sb_evlist, &host_env)) {
>  			pr_err("Couldn't ask for PERF_RECORD_BPF_EVENT side band events.\n.");
>  			status = -EINVAL;
> +			evlist__put(top.sb_evlist);
>  			goto out_put_evlist;
>  		}
>  	}

[Severity: High]
This is a pre-existing issue, but does tools/perf/builtin-record.c have a
similar problem where the error path in record__setup_sb_evlist() bypasses
updating the state?

If evlist__add_bpf_sb_event() fails in record__setup_sb_evlist(), it
returns -1 without updating the opts->no_bpf_event flag:

tools/perf/builtin-record.c:record__setup_sb_evlist() {
    ...
	if (evlist__add_bpf_sb_event(rec->sb_evlist, perf_session__env(rec->session))) {
		pr_err("Couldn't ask for PERF_RECORD_BPF_EVENT side band events.\n.");
		return -1;
	}
    ...
}

During cleanup in __cmd_record(), the unmodified no_bpf_event flag causes
evlist__stop_sb_thread() to be unconditionally invoked:

tools/perf/builtin-record.c:__cmd_record() {
    ...
	if (!opts->no_bpf_event)
		evlist__stop_sb_thread(rec->sb_evlist);
    ...
}

This could blindly call pthread_join() on the uninitialized thread ID in
evlist__stop_sb_thread():

tools/perf/util/sideband_evlist.c:evlist__stop_sb_thread() {
    ...
	pthread_join(*evlist__sb_thread_th(evlist), NULL);
    ...
}

Could this trigger undefined behavior and a crash?

[Severity: High]
This is also a pre-existing issue, but is there a leak of the sideband
thread and sb_evlist in tools/perf/builtin-record.c when using
--switch-output-event together with --no-bpf-event?

In record__setup_sb_evlist(), the sideband thread is started when using
--switch-output-event, even if BPF events are disabled:

tools/perf/builtin-record.c:record__setup_sb_evlist() {
    ...
	if (evlist__start_sb_thread(rec->sb_evlist, &rec->opts.target)) {
    ...
}

However, cleanup in __cmd_record() is bypassed because opts->no_bpf_event
evaluates to true:

tools/perf/builtin-record.c:__cmd_record() {
    ...
	if (!opts->no_bpf_event)
		evlist__stop_sb_thread(rec->sb_evlist);
    ...
}

Does this permanently strand the background poll thread and leak the
allocated evlist during process teardown?

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

[PATCH v1 04/13] perf python: Fix memory leak in pyrf_evlist__get_pollfd

[Ian Rogers]

Fix a Python list object leak in pyrf_evlist__get_pollfd() by adding
a missing Py_DECREF on the error exit path.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 877108e42b1b ("perf tools: Initial python binding")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 11e1f39c1bea..21e8b5a23a1c 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -2843,6 +2843,7 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist,
 
 	return list;
 free_list:
+	Py_DECREF(list);
 	return PyErr_NoMemory();
 }
 
-- 
2.55.0.rc0.786.g65d90a0328-goog

Re: [PATCH v1 04/13] perf python: Fix memory leak in pyrf_evlist__get_pollfd

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added `Py_DECREF(list)` introduces a segmentation fault on the error path if the initial `PyList_New(0)` allocation fails.
--

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

perf python: Fix memory leak in pyrf_evlist__get_pollfd

Fix a Python list object leak in pyrf_evlist__get_pollfd() by adding
a missing Py_DECREF on the error exit path.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 11e1f39c1bea9..21e8b5a23a1c9 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -2843,6 +2843,7 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist,
>  
>  	return list;
>  free_list:
> +	Py_DECREF(list);

[Severity: High]
If PyList_New(0) fails and returns NULL, this will attempt to decrement the
reference count of a NULL pointer.

Because Py_DECREF unconditionally dereferences its argument, will this result
in a segmentation fault on the error path?

Should we check list for NULL earlier, or use Py_XDECREF(list) instead?

>  	return PyErr_NoMemory();
>  }

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

[PATCH v1 05/13] perf synthetic-events: Fix uninitialized pthread_join

[Ian Rogers]

In perf_event__synthesize_threads(), fix an uninitialized pthread_join()
call when thread creation fails by only joining the successfully
created threads.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 340b47f510bb ("perf top: Implement multithreading for perf_event__synthesize_threads")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/synthetic-events.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index b75f9dcf4dbf..35de41ad14d3 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -1104,8 +1104,8 @@ int perf_event__synthesize_threads(const struct perf_tool *tool,
 	}
 	err = 0;
 out_join:
-	for (i = 0; i < thread_nr; i++)
-		pthread_join(synthesize_threads[i], NULL);
+	for (j = 0; j < i; j++)
+		pthread_join(synthesize_threads[j], NULL);
 	free(args);
 free_threads:
 	free(synthesize_threads);
-- 
2.55.0.rc0.786.g65d90a0328-goog

Re: [PATCH v1 05/13] perf synthetic-events: Fix uninitialized pthread_join

[sashiko-bot]

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

Pre-existing issues:
- [High] Divide by zero crash in perf_event__synthesize_threads when no tasks are found.
--

commit 30888fa2ce27193736ae38293c89b11a5e47a514
Author: Ian Rogers <irogers@google.com>

perf synthetic-events: Fix uninitialized pthread_join

This commit fixes an uninitialized pthread_join() call when thread creation
fails in perf_event__synthesize_threads() by ensuring only successfully
created threads are joined.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index b75f9dcf4dbfe..35de41ad14d33 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -1104,8 +1104,8 @@ int perf_event__synthesize_threads(const struct perf_tool *tool,

[Severity: High]
This is a pre-existing issue, but does this code suffer from a fatal
division by zero when no tasks are found?

If scandir() finds no matching tasks in /proc (which can happen in an empty
chroot or isolated container), n is 0. If the system has multiple CPU cores
(thread_nr > 1), we bypass the single-thread fast path.

perf_event__synthesize_threads() then clamps thread_nr:

    if (thread_nr > n)
        thread_nr = n;

And shortly after evaluates:

    num_per_thread = n / thread_nr;

Does this divide by zero (0 / 0) and cause the perf tool to crash with a
fatal SIGFPE? Could we handle the n == 0 case earlier to prevent this
regression?

>  	}
>  	err = 0;
>  out_join:
> -	for (i = 0; i < thread_nr; i++)
> -		pthread_join(synthesize_threads[i], NULL);
> +	for (j = 0; j < i; j++)
> +		pthread_join(synthesize_threads[j], NULL);
>  	free(args);
>  free_threads:
>  	free(synthesize_threads);

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

[PATCH v1 06/13] perf data: Fix directory file descriptor leak in perf_data__switch

[Ian Rogers]

Fix a file descriptor leak in perf_data__switch() when running in
directory mode. By substituting perf_data__close() for
perf_data_file__close(), any thread-local directory files are properly
closed during file rotation.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 040f9915e99e ("perf data: Add perf_data_file__switch() helper")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/data.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
index 33fd1b82019e..32d4d908ad38 100644
--- a/tools/perf/util/data.c
+++ b/tools/perf/util/data.c
@@ -472,8 +472,7 @@ int perf_data__switch(struct perf_data *data,
 		pr_warning("Failed to rename %s to %s\n", data->path, *new_filepath);
 
 	if (!at_exit) {
-		perf_data_file__close(&data->file);
-		data->open = false;
+		perf_data__close(data);
 		ret = perf_data__open(data);
 		if (ret < 0)
 			goto out;
-- 
2.55.0.rc0.786.g65d90a0328-goog

[PATCH v1 07/13] perf test: Fix skiplist leak in cmd_test

[Ian Rogers]

Fix a memory leak in cmd_test() where skiplist was not freed on
exit paths.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 2ae828786c65 ("perf test: Allow skipping tests")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/builtin-test.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 7e75f590f225..cf5625cf4f11 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -1781,9 +1781,12 @@ int cmd_test(int argc, const char **argv)
 	rlimit__bump_memlock();
 
 	suites = build_suites();
-	if (!suites)
+	if (!suites) {
+		intlist__delete(skiplist);
 		return errno ? -errno : -ENOMEM;
+	}
 	ret = __cmd_test(suites, argc, argv, skiplist);
 	free(suites);
+	intlist__delete(skiplist);
 	return ret;
 }
-- 
2.55.0.rc0.786.g65d90a0328-goog

[PATCH v1 08/13] perf python: Check counts_values size in set_values

[Ian Rogers]

The set_values function incorrectly assumed the list contained exactly
5 elements. Add a check to prevent out-of-bounds access.

Fixes: 877108e42b1b ("perf tools: Initial python binding")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 21e8b5a23a1c..f9398bc30a80 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1913,6 +1913,11 @@ static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObje
 	}
 
 	size = PyList_Size(list);
+	if (size != 5) {
+		PyErr_SetString(PyExc_ValueError, "List must have exactly 5 entries");
+		return -1;
+	}
+
 	for (Py_ssize_t i = 0; i < size; i++) {
 		item = PyList_GetItem(list, i);
 		if (!PyLong_Check(item)) {
-- 
2.55.0.rc0.786.g65d90a0328-goog

Re: [PATCH v1 08/13] perf python: Check counts_values size in set_values

[sashiko-bot]

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

Pre-existing issues:
- [High] The Python bindings for `perf_counts_values` incorrectly use 32-bit-limited C `long` functions and types for `uint64_t` data, leading to data truncation on 32-bit platforms, incorrect negative values, and unhandled `OverflowError` exceptions causing fatal `SystemError` crashes.
- [High] Deleting the `.values` attribute from Python causes a segmentation fault due to a NULL pointer dereference in the property setter.
--

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

perf python: Check counts_values size in set_values

The set_values function incorrectly assumed the list contained exactly
5 elements. Add a check to prevent out-of-bounds access.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 21e8b5a23a1c9..f9398bc30a80b 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -1913,6 +1913,11 @@ static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObje
>  	}

[Severity: High]
This is a pre-existing issue, but does this function cause a NULL pointer 
dereference when the attribute is deleted?

Just above this context in pyrf_counts_values_set_values():

	if (!PyList_Check(list)) {
		PyErr_SetString(PyExc_TypeError, "Value assigned must be a list");
		return -1;
	}

If a user executes del obj.values from Python code, the Python C API calls 
the property setter with list set to NULL. Passing a NULL list directly into 
PyList_Check(list) will dereference the object to check its type, which 
causes an immediate segmentation fault.

>  
>  	size = PyList_Size(list);
> +	if (size != 5) {
> +		PyErr_SetString(PyExc_ValueError, "List must have exactly 5 entries");
> +		return -1;
> +	}
> +
>  	for (Py_ssize_t i = 0; i < size; i++) {
>  		item = PyList_GetItem(list, i);
>  		if (!PyLong_Check(item)) {

[Severity: High]
This is a pre-existing issue, but can this loop lead to data truncation or
crash the interpreter?

Just below this context in pyrf_counts_values_set_values():

		self->values.values[i] = PyLong_AsLong(item);
	}

Since self->values.values stores uint64_t data, assigning the result of
PyLong_AsLong (which returns a signed C long) can truncate data on 32-bit
platforms or produce incorrect negative values.

Additionally, if PyLong_AsLong raises an OverflowError for very large
integers, the error is silently ignored and 0 is returned to the interpreter,
which triggers a fatal SystemError crash.

The same underlying issue appears in other places where we expose uint64_t
data:

tools/perf/util/python.c:pyrf_counts_values_get_values()
	for (int i = 0; i < 5; i++)
		PyList_SetItem(vals, i, PyLong_FromLong(self->values.values[i]));

tools/perf/util/python.c:pyrf_counts_values_members
	counts_values_member_def(val, T_ULONG, "Value of event"),

Should we be using 64-bit functions like PyLong_AsUnsignedLongLong() and
PyLong_FromUnsignedLongLong(), and ensuring exceptions are caught?

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

[PATCH v1 09/13] perf python: Validate CPU and thread maps in pyrf_evsel__open

[Ian Rogers]

Add explicit Py_TYPE checks to ensure the arguments passed are
actually of the correct pyrf_thread_map and pyrf_cpu_map types.

Fixes: 877108e42b1b ("perf tools: Initial python binding")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index f9398bc30a80..62f404c40d8d 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -2096,11 +2096,21 @@ static PyObject *pyrf_evsel__open(struct pyrf_evsel *pevsel,
 					 &pcpus, &pthreads, &group, &inherit))
 		return NULL;
 
-	if (pthreads != NULL && pthreads != Py_None)
+	if (pthreads != NULL && pthreads != Py_None) {
+		if (Py_TYPE(pthreads) != &pyrf_thread_map__type) {
+			PyErr_SetString(PyExc_TypeError, "threads must be a thread_map");
+			return NULL;
+		}
 		threads = ((struct pyrf_thread_map *)pthreads)->threads;
+	}
 
-	if (pcpus != NULL && pcpus != Py_None)
+	if (pcpus != NULL && pcpus != Py_None) {
+		if (Py_TYPE(pcpus) != &pyrf_cpu_map__type) {
+			PyErr_SetString(PyExc_TypeError, "cpus must be a cpu_map");
+			return NULL;
+		}
 		cpus = ((struct pyrf_cpu_map *)pcpus)->cpus;
+	}
 
 	evsel->core.attr.inherit = inherit;
 	/*
-- 
2.55.0.rc0.786.g65d90a0328-goog

Re: [PATCH v1 09/13] perf python: Validate CPU and thread maps in pyrf_evsel__open

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The explicit `Py_TYPE()` checks added to `pyrf_evsel__open` break Python subclassing for `perf.thread_map` and `perf.cpu_map` objects.
--

commit 909b91c5a1d04af18afad351b78395740e856d0b
Author: Ian Rogers <irogers@google.com>

perf python: Validate CPU and thread maps in pyrf_evsel__open

This commit adds explicit Py_TYPE checks to ensure the cpu and thread map
arguments passed to pyrf_evsel__open are of the correct types.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index f9398bc30a80b..62f404c40d8dc 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> @@ -2096,11 +2096,21 @@ static PyObject *pyrf_evsel__open(struct pyrf_evsel *pevsel,
>  					 &pcpus, &pthreads, &group, &inherit))
>  		return NULL;
>  
> -	if (pthreads != NULL && pthreads != Py_None)
> +	if (pthreads != NULL && pthreads != Py_None) {
> +		if (Py_TYPE(pthreads) != &pyrf_thread_map__type) {

[Severity: Medium]
Will this strict equality check break valid Python subclasses of thread_map?

Because pyrf_thread_map__type sets Py_TPFLAGS_BASETYPE, Python scripts are
permitted to subclass it, but Py_TYPE() will point to the subclass type and
fail this exact-type check.

Should this use PyObject_TypeCheck() instead to safely support subclassing?

> +			PyErr_SetString(PyExc_TypeError, "threads must be a thread_map");
> +			return NULL;
> +		}
>  		threads = ((struct pyrf_thread_map *)pthreads)->threads;
> +	}
>  
> -	if (pcpus != NULL && pcpus != Py_None)
> +	if (pcpus != NULL && pcpus != Py_None) {
> +		if (Py_TYPE(pcpus) != &pyrf_cpu_map__type) {

[Severity: Medium]
Similarly, does this strict equality block valid Python subclasses of cpu_map?

> +			PyErr_SetString(PyExc_TypeError, "cpus must be a cpu_map");
> +			return NULL;
> +		}
>  		cpus = ((struct pyrf_cpu_map *)pcpus)->cpus;
> +	}

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

[PATCH v1 10/13] perf python: Validate attribute setters in pyrf_evsel

[Ian Rogers]

If val is NULL when setting an attribute, PyErr_SetString should be
called as deleting the attribute isn't supported. In addition, ensure
PyErr_Occurred is checked before setting the attribute to avoid setting
a garbage value.

Fixes: 877108e42b1b ("perf tools: Initial python binding")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 89 ++++++++++++++++++++++++++++++++++------
 1 file changed, 77 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 62f404c40d8d..e277c89b8347 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -2284,6 +2284,11 @@ static int pyrf_evsel__set_tracking(PyObject *self, PyObject *val, void *closure
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
 	is_true = PyObject_IsTrue(val);
 	if (is_true < 0)
 		return -1;
@@ -2295,11 +2300,21 @@ static int pyrf_evsel__set_tracking(PyObject *self, PyObject *val, void *closure
 static int pyrf_evsel__set_attr_config(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.config = PyLong_AsUnsignedLongLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLongLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.config = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_config(PyObject *self, void *closure __maybe_unused)
@@ -2314,11 +2329,21 @@ static PyObject *pyrf_evsel__get_attr_config(PyObject *self, void *closure __may
 static int pyrf_evsel__set_attr_read_format(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.read_format = PyLong_AsUnsignedLongLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLongLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.read_format = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_read_format(PyObject *self, void *closure __maybe_unused)
@@ -2333,11 +2358,21 @@ static PyObject *pyrf_evsel__get_attr_read_format(PyObject *self, void *closure
 static int pyrf_evsel__set_attr_sample_period(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.sample_period = PyLong_AsUnsignedLongLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLongLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.sample_period = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_sample_period(PyObject *self, void *closure __maybe_unused)
@@ -2352,11 +2387,21 @@ static PyObject *pyrf_evsel__get_attr_sample_period(PyObject *self, void *closur
 static int pyrf_evsel__set_attr_sample_type(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.sample_type = PyLong_AsUnsignedLongLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLongLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.sample_type = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_sample_type(PyObject *self, void *closure __maybe_unused)
@@ -2380,11 +2425,21 @@ static PyObject *pyrf_evsel__get_attr_size(PyObject *self, void *closure __maybe
 static int pyrf_evsel__set_attr_type(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.type = PyLong_AsUnsignedLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.type = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_type(PyObject *self, void *closure __maybe_unused)
@@ -2399,11 +2454,21 @@ static PyObject *pyrf_evsel__get_attr_type(PyObject *self, void *closure __maybe
 static int pyrf_evsel__set_attr_wakeup_events(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.wakeup_events = PyLong_AsUnsignedLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.wakeup_events = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_wakeup_events(PyObject *self, void *closure __maybe_unused)
-- 
2.55.0.rc0.786.g65d90a0328-goog

[PATCH v1 11/13] perf python: Zero initialize perf_data in pyrf_data__init

[Ian Rogers]

Replace path clearing with memset so the entire struct is zeroed,
preventing uninitialized fields from causing errors later.

Fixes: 4cd0142f7dec ("perf python: Add wrapper for perf_data file abstraction")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index e277c89b8347..03b4e8809107 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -3607,7 +3607,7 @@ static int pyrf_data__init(struct pyrf_data *pdata, PyObject *args, PyObject *kw
 	if (pdata->data.open)
 		perf_data__close(&pdata->data);
 	free((char *)pdata->data.path);
-	pdata->data.path = NULL;
+	memset(&pdata->data, 0, sizeof(pdata->data));
 
 	if (fd != -1) {
 		struct stat st;
-- 
2.55.0.rc0.786.g65d90a0328-goog

Re: [PATCH v1 11/13] perf python: Zero initialize perf_data in pyrf_data__init

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing explicit inclusion of POSIX standard header `<string.h>` for newly added `memset`.
--

commit 84d9f7832c422ee6951987ed0cdd0d5717fcc7d2
Author: Ian Rogers <irogers@google.com>

perf python: Zero initialize perf_data in pyrf_data__init

This commit replaces clearing the path with memset to zero out the entire
perf_data struct in pyrf_data__init.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index e277c89b83477..03b4e88091074 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -3607,7 +3607,7 @@ static int pyrf_data__init(struct pyrf_data *pdata, PyObject *args, PyObject *kw
>  	if (pdata->data.open)
>  		perf_data__close(&pdata->data);
>  	free((char *)pdata->data.path);
> -	pdata->data.path = NULL;
> +	memset(&pdata->data, 0, sizeof(pdata->data));

[Severity: Medium]
Will this cause a build failure on musl libc systems? It looks like
memset() is added here but <string.h> is not explicitly included in
tools/perf/util/python.c.

Musl libc enforces strict POSIX namespace separation and does not implicitly
include headers. Could we add an explicit #include <string.h> at the top
of the file to ensure the build succeeds in environments using musl?

>  
>  	if (fd != -1) {
>  		struct stat st;

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

[PATCH v1 12/13] perf python: Add thread uninitialized checks

[Ian Rogers]

Add CHECK_INITIALIZED checks to the thread attribute getters
(get_pid, get_tid, get_ppid) to prevent crashes if they are accessed
before being properly initialized.

Fixes: 3b96bf7af60d ("perf python: Add python session abstraction wrapping perf's session")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 03b4e8809107..70f985ad3dd2 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -3743,16 +3743,19 @@ static PyMethodDef pyrf_thread__methods[] = {
 
 static PyObject *pyrf_thread__get_pid(struct pyrf_thread *pthread, void *closure __maybe_unused)
 {
+	CHECK_INITIALIZED(pthread->thread, "thread");
 	return PyLong_FromLong(thread__pid(pthread->thread));
 }
 
 static PyObject *pyrf_thread__get_tid(struct pyrf_thread *pthread, void *closure __maybe_unused)
 {
+	CHECK_INITIALIZED(pthread->thread, "thread");
 	return PyLong_FromLong(thread__tid(pthread->thread));
 }
 
 static PyObject *pyrf_thread__get_ppid(struct pyrf_thread *pthread, void *closure __maybe_unused)
 {
+	CHECK_INITIALIZED(pthread->thread, "thread");
 	return PyLong_FromLong(thread__ppid(pthread->thread));
 }
 
-- 
2.55.0.rc0.786.g65d90a0328-goog

Re: [PATCH v1 12/13] perf python: Add thread uninitialized checks

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch incompletely fixes the missing initialization checks by omitting `pyrf_thread__get_cpu`, leaving a NULL pointer dereference vulnerability.
- [High] The `perf.pmu` type methods lack initialization guards, allowing uninitialized objects to cause NULL pointer dereferences.
- [Medium] The Python object `count_values` is leaked in multiple error paths within `pyrf_evsel__read`.
--

commit 95a2c7473298664fd72c7086443470ff19559438
Author: Ian Rogers <irogers@google.com>

perf python: Add thread uninitialized checks

The patch adds CHECK_INITIALIZED checks to the thread attribute getters
(get_pid, get_tid, get_ppid) in the perf python module to prevent crashes
when accessing uninitialized thread objects.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 03b4e88091074..70f985ad3dd20 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -3743,16 +3743,19 @@ static PyMethodDef pyrf_thread__methods[] = {
>  
>  static PyObject *pyrf_thread__get_pid(struct pyrf_thread *pthread, void *closure __maybe_unused)
>  {
> +	CHECK_INITIALIZED(pthread->thread, "thread");
>  	return PyLong_FromLong(thread__pid(pthread->thread));
>  }
>  
>  static PyObject *pyrf_thread__get_tid(struct pyrf_thread *pthread, void *closure __maybe_unused)
>  {
> +	CHECK_INITIALIZED(pthread->thread, "thread");
>  	return PyLong_FromLong(thread__tid(pthread->thread));
>  }
>  
>  static PyObject *pyrf_thread__get_ppid(struct pyrf_thread *pthread, void *closure __maybe_unused)
>  {
> +	CHECK_INITIALIZED(pthread->thread, "thread");
>  	return PyLong_FromLong(thread__ppid(pthread->thread));
>  }
>  

[Severity: High]
This isn't a bug introduced by this patch, but pyrf_thread__get_cpu() is
missing a similar check:

tools/perf/util/python.c:pyrf_thread__get_cpu() {
    return PyLong_FromLong(thread__cpu(pthread->thread));
}

Could accessing the .cpu property on an uninitialized thread object cause a
NULL pointer dereference? Does it make sense to add a CHECK_INITIALIZED
guard here as well?

[Severity: High]
This isn't a bug introduced by this patch, but looking at the perf.pmu
methods, they also appear to lack initialization checks:

tools/perf/util/python.c:pyrf_pmu__name() {
    return PyUnicode_FromString(ppmu->pmu->name);
}

tools/perf/util/python.c:pyrf_pmu__events() {
    ...
    ret = perf_pmu__for_each_event(ppmu->pmu,
                                   false,
                                   py_list,
                                   pyrf_pmu__events_cb);
    ...
}

Since perf.pmu uses PyType_GenericNew, could a script instantiate an
uninitialized PMU and trigger a NULL pointer dereference when calling .name()
or .events()?

[Severity: Medium]
This isn't a bug introduced by this patch, but I noticed a leak in
pyrf_evsel__read() on its error paths:

tools/perf/util/python.c:pyrf_evsel__read() {
    count_values = PyObject_New(struct pyrf_counts_values,
                                &pyrf_counts_values__type);
    if (!count_values)
        return NULL;

    if (!PyArg_ParseTuple(args, "ii", &cpu, &thread))
        return NULL;

    cpu_idx = perf_cpu_map__idx(evsel->core.cpus, (struct perf_cpu){.cpu = cpu});
    if (cpu_idx < 0) {
        PyErr_Format(PyExc_TypeError, "CPU %d is not part of evsel's CPUs", cpu);
        return NULL;
    }
    ...
}

Does returning NULL directly on argument parsing or bounds checking failures
leak the newly allocated count_values object? Should we call
Py_DECREF(count_values) before returning?

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

[PATCH v1 13/13] perf python: Fix MetricGroup return type in perf.pyi

[Ian Rogers]

The metrics() function can return a dictionary where the value is either
a string or a list of strings, so the type signature in the stub file
should be Union[str, List[str]].

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/python/perf.pyi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/python/perf.pyi b/tools/perf/python/perf.pyi
index f266c9baffd0..58b0b3ed819d 100644
--- a/tools/perf/python/perf.pyi
+++ b/tools/perf/python/perf.pyi
@@ -1,5 +1,5 @@
 """Type stubs for the perf Python module."""
-from typing import Callable, Dict, List, Optional, Any, Iterator
+from typing import Callable, Dict, List, Optional, Any, Iterator, Union
 
 def config_get(name: str) -> Optional[str]:
     """Get a configuration value from perf config.
@@ -12,7 +12,7 @@ def config_get(name: str) -> Optional[str]:
     """
     ...
 
-def metrics() -> List[Dict[str, str]]:
+def metrics() -> List[Dict[str, Union[str, List[str]]]]:
     """Get a list of available metrics.
 
     Returns:
-- 
2.55.0.rc0.786.g65d90a0328-goog

Re: [PATCH v1 13/13] perf python: Fix MetricGroup return type in perf.pyi

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but local references to the Python objects 'key' and 'value' in `pyrf__metrics_cb` are leaked on the success path.
--

commit 0b60b233b211850d4730f46ab4faf2da85366d5f
Author: Ian Rogers <irogers@google.com>

perf python: Fix MetricGroup return type in perf.pyi

The metrics() function can return a dictionary where the value is either
a string or a list of strings, so the type signature in the stub file
should be Union[str, List[str]].

> diff --git a/tools/perf/python/perf.pyi b/tools/perf/python/perf.pyi
> index f266c9baffd01..58b0b3ed819d8 100644
> --- a/tools/perf/python/perf.pyi
> +++ b/tools/perf/python/perf.pyi
[ ... ]
> @@ -12,7 +12,7 @@ def config_get(name: str) -> Optional[str]:
>      """
>      ...
>  
> -def metrics() -> List[Dict[str, str]]:
> +def metrics() -> List[Dict[str, Union[str, List[str]]]]:

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the return
type update for metrics(), I noticed a potential resource leak in its C
implementation.

Looking at tools/perf/util/python.c:pyrf__metrics_cb():

	if (!value || PyDict_SetItem(dict, key, value) != 0) {
		Py_XDECREF(key);
		Py_XDECREF(value);
		Py_XDECREF(dict);
		return -ENOMEM;
	}

	if (!add_to_dict(dict, "MetricName", pm->metric_name) ||

Does this leak the local references to the Python objects 'key' and 'value'
on the success path?

Since PyDict_SetItem() does not steal references, it appears that 'key' and
'value' might need a Py_DECREF() after a successful insertion. Without it,
a script repeatedly calling perf.metrics() could continually leak memory.

>      """Get a list of available metrics.
>  
>      Returns:

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

[PATCH v2 00/16] perf python: Fix python extension bugs from v19 review

[Ian Rogers]

Phase 1 of splitting the v19 review patches into smaller fixes.
These patches address issues identified in the python extension during
the review of the v19 script porting series, as well as fixing several
pre-existing bugs and leaks.

v2:
- Addressed sashiko review feedback:
  - Fixed 64-bit casting for counts_values by using T_ULONGLONG and PyLong_FromUnsignedLongLong.
  - Replaced strict Py_TYPE checks with PyObject_TypeCheck for thread/cpu maps.
  - Fixed a missing CHECK_INITIALIZED for pyrf_thread__get_cpu, pyrf_pmu__name, pyrf_pmu__events.
  - Included <string.h> where memset was used.
- Fixed line splitting formatting on PyObject_New in pyrf_evsel__read.
- Added fixes for memory leaks and issues discovered during review:
  - Fix memory leak of key and value in pyrf__metrics_cb
  - Fix count_values memory leak in pyrf_evsel__read
  - Fix sb_evlist leaks in both top and record paths.
  - Fix potential divide by zero in perf_event__synthesize_threads

v1:
- Decomposed the single large "Fix python extension bugs" patch into 13 individual fixes.
- Added Fixes: tags for issues that date back to earlier commits.
- Addressed checkpatch warnings (line lengths, Signed-off-by, etc).


Ian Rogers (16):
  perf script: Fix metric_evlist leak in script_find_metrics
  perf stat: Fix evsel_list leak in cmd_stat
  perf tools: Fix sb_evlist leaks in top and record
  perf python: Fix memory leak in pyrf_evlist__get_pollfd
  perf synthetic-events: Fix uninitialized pthread_join
  perf data: Fix directory file descriptor leak in perf_data__switch
  perf test: Fix skiplist leak in cmd_test
  perf python: Check counts_values size in set_values
  perf python: Validate CPU and thread maps in pyrf_evsel__open
  perf python: Validate attribute setters in pyrf_evsel
  perf python: Zero initialize perf_data in pyrf_data__init
  perf python: Add thread and PMU uninitialized checks
  perf python: Fix MetricGroup return type in perf.pyi
  perf python: Fix count_values memory leak in pyrf_evsel__read
  perf python: Fix memory leak in pyrf__metrics_cb
  perf synthetic-events: Fix divide by zero in
    perf_event__synthesize_threads

 tools/perf/builtin-record.c        |   2 +
 tools/perf/builtin-script.c        |   2 +-
 tools/perf/builtin-stat.c          |   9 +-
 tools/perf/builtin-top.c           |   2 +
 tools/perf/python/perf.pyi         |   4 +-
 tools/perf/tests/builtin-test.c    |   5 +-
 tools/perf/util/data.c             |   3 +-
 tools/perf/util/python.c           | 160 ++++++++++++++++++++++++-----
 tools/perf/util/synthetic-events.c |   6 +-
 9 files changed, 153 insertions(+), 40 deletions(-)

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 01/16] perf script: Fix metric_evlist leak in script_find_metrics

[Ian Rogers]

Fix a memory leak in script_find_metrics() where metric_evlist is leaked
when returning early on error paths.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 3622990efaab ("perf script: Change metric format to use json metrics")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-script.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 47afd8cdc2b7..6b69e982ba62 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2275,8 +2275,8 @@ static int script_find_metrics(const struct pmu_metric *pm,
 	}
 	pr_debug("Found metric '%s' whose evsels match those of in the perf data\n",
 		 pm->metric_name);
-	evlist__put(metric_evlist);
 out:
+	evlist__put(metric_evlist);
 	return 0;
 }
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 02/16] perf stat: Fix evsel_list leak in cmd_stat

[Ian Rogers]

Fix a memory leak in cmd_stat() where evsel_list is leaked if an error
occurs while opening the output file.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 361c99a661a7 ("perf evsel: Introduce perf_evlist")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-stat.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 3f897b2e8638..7cb6ea937e87 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2718,7 +2718,8 @@ int cmd_stat(int argc, const char **argv)
 		output = fopen(output_name, mode);
 		if (!output) {
 			perror("failed to create output file");
-			return -1;
+			status = -1;
+			goto out;
 		}
 		if (!stat_config.json_output) {
 			clock_gettime(CLOCK_REALTIME, &tm);
@@ -2729,7 +2730,8 @@ int cmd_stat(int argc, const char **argv)
 		output = fdopen(output_fd, mode);
 		if (!output) {
 			perror("Failed opening logfd");
-			return -errno;
+			status = -errno;
+			goto out;
 		}
 	}
 
@@ -2738,7 +2740,8 @@ int cmd_stat(int argc, const char **argv)
 		parse_options_usage(stat_usage, stat_options, "o", 1);
 		parse_options_usage(NULL, stat_options, "log-fd", 0);
 		parse_options_usage(NULL, stat_options, "interval-clear", 0);
-		return -1;
+		status = -1;
+		goto out;
 	}
 
 	stat_config.output = output;
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 03/16] perf tools: Fix sb_evlist leaks in top and record

[Ian Rogers]

Fix a memory leak in cmd_top() where top.sb_evlist was not freed if
evlist__add_bpf_sb_event() fails. Note that evlist__start_sb_thread() and
evlist__stop_sb_thread() take ownership of the evlist and free it, so
we must only free it if we fail before starting the thread. Also set
top.sb_evlist to NULL to prevent use-after-free bugs.

Apply the same fix to builtin-record.c to avoid leaking rec->sb_evlist
and calling pthread_join on uninitialized threads in the error path.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: b38d85ef49cf ("perf bpf: Decouple creating the evlist from adding the SB event")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-record.c | 2 ++
 tools/perf/builtin-top.c    | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ebd3ed0c9b3e..b106d50f6d97 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2247,6 +2247,8 @@ static int record__setup_sb_evlist(struct record *rec)
 
 		if (evlist__add_bpf_sb_event(rec->sb_evlist, perf_session__env(rec->session))) {
 			pr_err("Couldn't ask for PERF_RECORD_BPF_EVENT side band events.\n.");
+			evlist__put(rec->sb_evlist);
+			rec->sb_evlist = NULL;
 			return -1;
 		}
 	}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 5933c46ee137..570410599f1b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1881,6 +1881,8 @@ int cmd_top(int argc, const char **argv)
 		if (evlist__add_bpf_sb_event(top.sb_evlist, &host_env)) {
 			pr_err("Couldn't ask for PERF_RECORD_BPF_EVENT side band events.\n.");
 			status = -EINVAL;
+			evlist__put(top.sb_evlist);
+			top.sb_evlist = NULL;
 			goto out_put_evlist;
 		}
 	}
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 03/16] perf top: Fix sb_evlist leak

[Ian Rogers]

Fix a memory leak in cmd_top() where top.sb_evlist was not freed if
evlist__add_bpf_sb_event() fails. Note that evlist__start_sb_thread() and
evlist__stop_sb_thread() take ownership of the evlist and free it, so
we must only free it if we fail before starting the thread.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: b38d85ef49cf ("perf bpf: Decouple creating the evlist from adding the SB event")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-top.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 5933c46ee137..abc74b8ab759 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1881,6 +1881,7 @@ int cmd_top(int argc, const char **argv)
 		if (evlist__add_bpf_sb_event(top.sb_evlist, &host_env)) {
 			pr_err("Couldn't ask for PERF_RECORD_BPF_EVENT side band events.\n.");
 			status = -EINVAL;
+			evlist__put(top.sb_evlist);
 			goto out_put_evlist;
 		}
 	}
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 04/16] perf python: Fix memory leak in pyrf_evlist__get_pollfd

[Ian Rogers]

Fix a Python list object leak in pyrf_evlist__get_pollfd() by adding
a missing Py_DECREF on the error exit path.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 877108e42b1b ("perf tools: Initial python binding")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 11e1f39c1bea..550f1e7d1449 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -2825,6 +2825,8 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist,
 
 	evlist = pevlist->evlist;
 	list = PyList_New(0);
+	if (!list)
+		return NULL;
 
 	for (i = 0; i < evlist__core(evlist)->pollfd.nr; ++i) {
 		PyObject *file;
@@ -2843,6 +2845,7 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist,
 
 	return list;
 free_list:
+	Py_XDECREF(list);
 	return PyErr_NoMemory();
 }
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH v2 04/16] perf python: Fix memory leak in pyrf_evlist__get_pollfd

[sashiko-bot]

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

Pre-existing issues:
- [Low] Missing explicit POSIX libc header inclusions for `<stdlib.h>` and `<stdio.h>`.
--

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

perf python: Fix memory leak in pyrf_evlist__get_pollfd

This commit fixes a Python list object leak in pyrf_evlist__get_pollfd() by
adding a missing Py_DECREF on the error exit path.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 11e1f39c1bea9..550f1e7d14499 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c

[Severity: Low]
This isn't a bug introduced by this patch, but there is a pre-existing issue
where the top-level includes section is missing explicit POSIX libc headers:

#include <inttypes.h>
#include <string.h>

#include <linux/err.h>

The file relies on POSIX standard functions like calloc() and free() which
require <stdlib.h>, as well as asprintf() which requires <stdio.h>. Could
these be explicitly added to ensure musl compilation compatibility?

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

[PATCH v2 05/16] perf synthetic-events: Fix uninitialized pthread_join

[Ian Rogers]

In perf_event__synthesize_threads(), fix an uninitialized pthread_join()
call when thread creation fails by only joining the successfully
created threads.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 340b47f510bb ("perf top: Implement multithreading for perf_event__synthesize_threads")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/synthetic-events.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index b75f9dcf4dbf..35de41ad14d3 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -1104,8 +1104,8 @@ int perf_event__synthesize_threads(const struct perf_tool *tool,
 	}
 	err = 0;
 out_join:
-	for (i = 0; i < thread_nr; i++)
-		pthread_join(synthesize_threads[i], NULL);
+	for (j = 0; j < i; j++)
+		pthread_join(synthesize_threads[j], NULL);
 	free(args);
 free_threads:
 	free(synthesize_threads);
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH v2 05/16] perf synthetic-events: Fix uninitialized pthread_join

[sashiko-bot]

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

Pre-existing issues:
- [Low] Missing explicit POSIX inclusions for <stdlib.h> and <pthread.h> in synthetic-events.c.
--

commit 9ead28d76c34623188955779076a3c283fe73884
Author: Ian Rogers <irogers@google.com>

perf synthetic-events: Fix uninitialized pthread_join

This commit fixes an uninitialized pthread_join() call in
perf_event__synthesize_threads() when thread creation fails. It updates the
cleanup loop to only join the threads that were successfully created.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -1104,8 +1104,8 @@ int perf_event__synthesize_threads(const struct perf_tool *tool,
>  	}
>  	err = 0;
>  out_join:
> -	for (i = 0; i < thread_nr; i++)
> -		pthread_join(synthesize_threads[i], NULL);
> +	for (j = 0; j < i; j++)
> +		pthread_join(synthesize_threads[j], NULL);

[Severity: Low]
This is a pre-existing issue, but should this file have explicit POSIX header
inclusions for <stdlib.h> and <pthread.h> to ensure compatibility with musl
libc? The code here relies on pthread_join() and free(), but appears to
depend on implicit header inclusions to resolve them.

>  	free(args);
>  free_threads:
>  	free(synthesize_threads);

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

[PATCH v2 06/16] perf data: Fix directory file descriptor leak in perf_data__switch

[Ian Rogers]

Fix a file descriptor leak in perf_data__switch() when running in
directory mode. By substituting perf_data__close() for
perf_data_file__close(), any thread-local directory files are properly
closed during file rotation.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 040f9915e99e ("perf data: Add perf_data_file__switch() helper")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/data.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
index 33fd1b82019e..32d4d908ad38 100644
--- a/tools/perf/util/data.c
+++ b/tools/perf/util/data.c
@@ -472,8 +472,7 @@ int perf_data__switch(struct perf_data *data,
 		pr_warning("Failed to rename %s to %s\n", data->path, *new_filepath);
 
 	if (!at_exit) {
-		perf_data_file__close(&data->file);
-		data->open = false;
+		perf_data__close(data);
 		ret = perf_data__open(data);
 		if (ret < 0)
 			goto out;
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH v2 06/16] perf data: Fix directory file descriptor leak in perf_data__switch

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The patch introduces a Use-After-Free during file rotation in directory mode. By changing `perf_data__switch()` to call `perf_data__close()`, the `data->dir.files` array is freed. However, thread mmaps (`map->file`) still retain pointers to this freed array and are never updated. Subsequent writes dereference the freed memory.
--

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

perf data: Fix directory file descriptor leak in perf_data__switch

This commit fixes a file descriptor leak in perf_data__switch() when running
in directory mode. It replaces perf_data_file__close() with perf_data__close()
to properly close thread-local directory files during file rotation.

> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> --- a/tools/perf/util/data.c
> +++ b/tools/perf/util/data.c
> @@ -472,8 +472,7 @@ int perf_data__switch(struct perf_data *data,
>  		pr_warning("Failed to rename %s to %s\n", data->path, *new_filepath);
>  
>  	if (!at_exit) {
> -		perf_data_file__close(&data->file);
> -		data->open = false;
> +		perf_data__close(data);

[Severity: Critical]
Could this cause a use-after-free regression during file rotation?

Calling perf_data__close(data) here frees the data->dir.files array.
However, thread mmaps (like map->file) appear to still retain pointers to this
freed memory and are not updated.

For example, in tools/perf/builtin-record.c:record__write():

    if (perf_data_file__write(file, bf, size) < 0) {

The file pointer here is aliased from map->file, which now points to the
freed memory. If someone runs perf record --switch-output with threads
enabled, the subsequent writes dereference these dangling pointers.

>  		ret = perf_data__open(data);
>  		if (ret < 0)
>  			goto out;

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

[PATCH v2 07/16] perf test: Fix skiplist leak in cmd_test

[Ian Rogers]

Fix a memory leak in cmd_test() where skiplist was not freed on
exit paths.

Assisted-by: Antigravity:gemini-3.1-pro
Fixes: 2ae828786c65 ("perf test: Allow skipping tests")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/builtin-test.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 7e75f590f225..cf5625cf4f11 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -1781,9 +1781,12 @@ int cmd_test(int argc, const char **argv)
 	rlimit__bump_memlock();
 
 	suites = build_suites();
-	if (!suites)
+	if (!suites) {
+		intlist__delete(skiplist);
 		return errno ? -errno : -ENOMEM;
+	}
 	ret = __cmd_test(suites, argc, argv, skiplist);
 	free(suites);
+	intlist__delete(skiplist);
 	return ret;
 }
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH v2 07/16] perf test: Fix skiplist leak in cmd_test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unsafe error handling: `errno` may be clobbered by cleanup functions before being evaluated.
--

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

perf test: Fix skiplist leak in cmd_test

This commit fixes a memory leak in cmd_test() by ensuring the skiplist is
properly freed on the exit paths.

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 7e75f590f225e..cf5625cf4f110 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c

[ ... ]

> @@ -1781,9 +1781,12 @@ int cmd_test(int argc, const char **argv)
>  	rlimit__bump_memlock();
>  
>  	suites = build_suites();
> -	if (!suites)
> +	if (!suites) {
> +		intlist__delete(skiplist);
>  		return errno ? -errno : -ENOMEM;

[Severity: Medium]
Could the call to intlist__delete() clobber errno before it is evaluated in
the return statement? 

intlist__delete() cascades to rblist__delete(), which ultimately issues
free() calls. On some libc implementations or under memory sanitizers, free()
and related cleanup functions do not guarantee that errno is preserved. This
could result in returning an incorrect error code from build_suites().

> +	}
>  	ret = __cmd_test(suites, argc, argv, skiplist);
>  	free(suites);
> +	intlist__delete(skiplist);
>  	return ret;
>  }

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

[PATCH v2 08/16] perf python: Check counts_values size in set_values

[Ian Rogers]

The set_values function incorrectly assumed the list contained exactly
5 elements. Add a check to prevent out-of-bounds access.

Fixes: 877108e42b1b ("perf tools: Initial python binding")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 550f1e7d1449..c6c729808a0f 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1881,11 +1881,11 @@ static void pyrf_counts_values__delete(struct pyrf_counts_values *pcounts_values
 	  0, help }
 
 static PyMemberDef pyrf_counts_values_members[] = {
-	counts_values_member_def(val, T_ULONG, "Value of event"),
-	counts_values_member_def(ena, T_ULONG, "Time for which enabled"),
-	counts_values_member_def(run, T_ULONG, "Time for which running"),
-	counts_values_member_def(id, T_ULONG, "Unique ID for an event"),
-	counts_values_member_def(lost, T_ULONG, "Num of lost samples"),
+	counts_values_member_def(val, T_ULONGLONG, "Value of event"),
+	counts_values_member_def(ena, T_ULONGLONG, "Time for which enabled"),
+	counts_values_member_def(run, T_ULONGLONG, "Time for which running"),
+	counts_values_member_def(id, T_ULONGLONG, "Unique ID for an event"),
+	counts_values_member_def(lost, T_ULONGLONG, "Num of lost samples"),
 	{ .name = NULL, },
 };
 
@@ -1895,8 +1895,15 @@ static PyObject *pyrf_counts_values_get_values(struct pyrf_counts_values *self,
 
 	if (!vals)
 		return NULL;
-	for (int i = 0; i < 5; i++)
-		PyList_SetItem(vals, i, PyLong_FromLong(self->values.values[i]));
+	for (int i = 0; i < 5; i++) {
+		PyObject *val = PyLong_FromUnsignedLongLong(self->values.values[i]);
+
+		if (!val) {
+			Py_DECREF(vals);
+			return NULL;
+		}
+		PyList_SetItem(vals, i, val);
+	}
 
 	return vals;
 }
@@ -1913,13 +1920,23 @@ static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObje
 	}
 
 	size = PyList_Size(list);
+	if (size != 5) {
+		PyErr_SetString(PyExc_ValueError, "List must have exactly 5 entries");
+		return -1;
+	}
+
 	for (Py_ssize_t i = 0; i < size; i++) {
+		unsigned long long val;
+
 		item = PyList_GetItem(list, i);
 		if (!PyLong_Check(item)) {
 			PyErr_SetString(PyExc_TypeError, "List members should be numbers");
 			return -1;
 		}
-		self->values.values[i] = PyLong_AsLong(item);
+		val = PyLong_AsUnsignedLongLong(item);
+		if (val == (unsigned long long)-1 && PyErr_Occurred())
+			return -1;
+		self->values.values[i] = val;
 	}
 
 	return 0;
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH v2 08/16] perf python: Check counts_values size in set_values

[sashiko-bot]

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

Pre-existing issues:
- [High] Deleting the `values` attribute from a `perf.counts_values` object in Python causes a segmentation fault.
- [Medium] The function `tracepoint_field` truncates 64-bit tracepoint fields on 32-bit platforms by casting them to `unsigned long` or `long`.
--

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

    perf python: Check counts_values size in set_values
    
    The set_values function incorrectly assumed the list contained exactly
    5 elements. Add a check to prevent out-of-bounds access.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 550f1e7d14499..c6c729808a0f2 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> @@ -1913,13 +1920,23 @@ static int pyrf_counts_values_set_values(struct pyrf_counts_values *self, PyObje
>  	}
>  
>  	size = PyList_Size(list);

[Severity: High]
This isn't a bug introduced by this patch, but does 
pyrf_counts_values_set_values() crash if the list parameter is NULL?

If a user deletes the attribute in Python (e.g., `del counts_values.values`),
the list parameter will be passed as NULL. The PyList_Check(list) macro called
just before this line unconditionally dereferences the pointer, which would
trigger a segmentation fault.

> +	if (size != 5) {
> +		PyErr_SetString(PyExc_ValueError, "List must have exactly 5 entries");
> +		return -1;
> +	}
> +
>  	for (Py_ssize_t i = 0; i < size; i++) {

[ ... ]

[Severity: Medium]
This is also a pre-existing issue, but could tracepoint_field() truncate 64-bit
values on 32-bit platforms?

In tools/perf/util/python.c:tracepoint_field():

		val = tep_read_number(pevent, data + field->offset,
				      field->size);
		if (field->flags & TEP_FIELD_IS_POINTER)
			ret = PyLong_FromUnsignedLong((unsigned long) val);
		else if (field->flags & TEP_FIELD_IS_SIGNED)
			ret = PyLong_FromLong((long) val);
		else
			ret = PyLong_FromUnsignedLong((unsigned long) val);

Since val is returned by tep_read_number(), which provides a 64-bit value,
casting it to unsigned long or long drops the top 32 bits on 32-bit
architectures. Does this lead to silent data corruption for 64-bit tracepoint
fields like timestamps or counters?

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

[PATCH v2 09/16] perf python: Validate CPU and thread maps in pyrf_evsel__open

[Ian Rogers]

Add explicit Py_TYPE checks to ensure the arguments passed are
actually of the correct pyrf_thread_map and pyrf_cpu_map types.

Fixes: 877108e42b1b ("perf tools: Initial python binding")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index c6c729808a0f..4bf37e2afb18 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -2108,11 +2108,21 @@ static PyObject *pyrf_evsel__open(struct pyrf_evsel *pevsel,
 					 &pcpus, &pthreads, &group, &inherit))
 		return NULL;
 
-	if (pthreads != NULL && pthreads != Py_None)
+	if (pthreads != NULL && pthreads != Py_None) {
+		if (!PyObject_TypeCheck(pthreads, &pyrf_thread_map__type)) {
+			PyErr_SetString(PyExc_TypeError, "threads must be a thread_map");
+			return NULL;
+		}
 		threads = ((struct pyrf_thread_map *)pthreads)->threads;
+	}
 
-	if (pcpus != NULL && pcpus != Py_None)
+	if (pcpus != NULL && pcpus != Py_None) {
+		if (!PyObject_TypeCheck(pcpus, &pyrf_cpu_map__type)) {
+			PyErr_SetString(PyExc_TypeError, "cpus must be a cpu_map");
+			return NULL;
+		}
 		cpus = ((struct pyrf_cpu_map *)pcpus)->cpus;
+	}
 
 	evsel->core.attr.inherit = inherit;
 	/*
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 10/16] perf python: Validate attribute setters in pyrf_evsel

[Ian Rogers]

If val is NULL when setting an attribute, PyErr_SetString should be
called as deleting the attribute isn't supported. In addition, ensure
PyErr_Occurred is checked before setting the attribute to avoid setting
a garbage value.

Fixes: 877108e42b1b ("perf tools: Initial python binding")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 89 ++++++++++++++++++++++++++++++++++------
 1 file changed, 77 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 4bf37e2afb18..657b054f5e51 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -2296,6 +2296,11 @@ static int pyrf_evsel__set_tracking(PyObject *self, PyObject *val, void *closure
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
 	is_true = PyObject_IsTrue(val);
 	if (is_true < 0)
 		return -1;
@@ -2307,11 +2312,21 @@ static int pyrf_evsel__set_tracking(PyObject *self, PyObject *val, void *closure
 static int pyrf_evsel__set_attr_config(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.config = PyLong_AsUnsignedLongLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLongLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.config = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_config(PyObject *self, void *closure __maybe_unused)
@@ -2326,11 +2341,21 @@ static PyObject *pyrf_evsel__get_attr_config(PyObject *self, void *closure __may
 static int pyrf_evsel__set_attr_read_format(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.read_format = PyLong_AsUnsignedLongLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLongLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.read_format = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_read_format(PyObject *self, void *closure __maybe_unused)
@@ -2345,11 +2370,21 @@ static PyObject *pyrf_evsel__get_attr_read_format(PyObject *self, void *closure
 static int pyrf_evsel__set_attr_sample_period(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.sample_period = PyLong_AsUnsignedLongLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLongLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.sample_period = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_sample_period(PyObject *self, void *closure __maybe_unused)
@@ -2364,11 +2399,21 @@ static PyObject *pyrf_evsel__get_attr_sample_period(PyObject *self, void *closur
 static int pyrf_evsel__set_attr_sample_type(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.sample_type = PyLong_AsUnsignedLongLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLongLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.sample_type = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_sample_type(PyObject *self, void *closure __maybe_unused)
@@ -2392,11 +2437,21 @@ static PyObject *pyrf_evsel__get_attr_size(PyObject *self, void *closure __maybe
 static int pyrf_evsel__set_attr_type(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.type = PyLong_AsUnsignedLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.type = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_type(PyObject *self, void *closure __maybe_unused)
@@ -2411,11 +2466,21 @@ static PyObject *pyrf_evsel__get_attr_type(PyObject *self, void *closure __maybe
 static int pyrf_evsel__set_attr_wakeup_events(PyObject *self, PyObject *val, void *closure __maybe_unused)
 {
 	struct pyrf_evsel *pevsel = (void *)self;
+	unsigned long new_val;
 
 	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
 
-	pevsel->evsel->core.attr.wakeup_events = PyLong_AsUnsignedLong(val);
-	return PyErr_Occurred() ? -1 : 0;
+	if (val == NULL) {
+		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
+		return -1;
+	}
+
+	new_val = PyLong_AsUnsignedLong(val);
+	if (PyErr_Occurred())
+		return -1;
+
+	pevsel->evsel->core.attr.wakeup_events = new_val;
+	return 0;
 }
 
 static PyObject *pyrf_evsel__get_attr_wakeup_events(PyObject *self, void *closure __maybe_unused)
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH v2 10/16] perf python: Validate attribute setters in pyrf_evsel

[sashiko-bot]

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

Pre-existing issues:
- [Medium] Missing NULL check for attribute deletion in `pyrf_counts_values_set_values` causes a segmentation fault.
--

commit 921733bf92d5e6634715543526c3d25b8346f3c3
Author: Ian Rogers <irogers@google.com>

perf python: Validate attribute setters in pyrf_evsel

This commit adds NULL checks to attribute setters in pyrf_evsel to
ensure an error is correctly reported instead of setting a garbage
value when attempting to delete an attribute.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 4bf37e2afb18d..657b054f5e515 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> @@ -2296,6 +2296,11 @@ static int pyrf_evsel__set_tracking(PyObject *self, PyObject *val, void *closure
>  
>  	CHECK_INITIALIZED_INT(pevsel->evsel, "evsel");
>  
> +	if (val == NULL) {
> +		PyErr_SetString(PyExc_TypeError, "cannot delete attribute");
> +		return -1;
> +	}
> +
>  	is_true = PyObject_IsTrue(val);
>  	if (is_true < 0)
>  		return -1;

[Severity: Medium]
This is a pre-existing issue, but since you are fixing these attribute
deletion issues in pyrf_evsel, should we also fix
pyrf_counts_values_set_values in the same file?

Looking at pyrf_counts_values_set_values, the list parameter is passed
directly to PyList_Check without verifying if it is NULL:

static int pyrf_counts_values_set_values(struct pyrf_counts_values *self,
					 PyObject *list, void *closure)
{
	Py_ssize_t size;
	PyObject *item = NULL;

	if (!PyList_Check(list)) {
		PyErr_SetString(PyExc_TypeError, "Value assigned must be a list");
		return -1;
	}

Could deleting the counts_values.values attribute from a Python script
trigger a segmentation fault here?

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

[PATCH v2 11/16] perf python: Zero initialize perf_data in pyrf_data__init

[Ian Rogers]

Replace path clearing with memset so the entire struct is zeroed,
preventing uninitialized fields from causing errors later.

Fixes: 4cd0142f7dec ("perf python: Add wrapper for perf_data file abstraction")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 657b054f5e51..71be130b38c5 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -3,6 +3,7 @@
 #include <Python.h>
 
 #include <inttypes.h>
+#include <string.h>
 
 #include <linux/err.h>
 #include <poll.h>
@@ -3621,7 +3622,7 @@ static int pyrf_data__init(struct pyrf_data *pdata, PyObject *args, PyObject *kw
 	if (pdata->data.open)
 		perf_data__close(&pdata->data);
 	free((char *)pdata->data.path);
-	pdata->data.path = NULL;
+	memset(&pdata->data, 0, sizeof(pdata->data));
 
 	if (fd != -1) {
 		struct stat st;
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 12/16] perf python: Add thread and PMU uninitialized checks

[Ian Rogers]

Add CHECK_INITIALIZED checks to the thread attribute getters
(get_pid, get_tid, get_ppid) to prevent crashes if they are accessed
before being properly initialized.

Fixes: 3b96bf7af60d ("perf python: Add python session abstraction wrapping perf's session")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 71be130b38c5..b17b191b1625 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -1678,6 +1678,7 @@ static PyObject *pyrf_pmu__name(PyObject *self)
 {
 	struct pyrf_pmu *ppmu = (void *)self;
 
+	CHECK_INITIALIZED(ppmu->pmu, "pmu");
 	return PyUnicode_FromString(ppmu->pmu->name);
 }
 
@@ -1730,6 +1731,8 @@ static PyObject *pyrf_pmu__events(PyObject *self)
 	PyObject *py_list = PyList_New(0);
 	int ret;
 
+	CHECK_INITIALIZED(ppmu->pmu, "pmu");
+
 	if (!py_list)
 		return NULL;
 
@@ -3758,21 +3761,25 @@ static PyMethodDef pyrf_thread__methods[] = {
 
 static PyObject *pyrf_thread__get_pid(struct pyrf_thread *pthread, void *closure __maybe_unused)
 {
+	CHECK_INITIALIZED(pthread->thread, "thread");
 	return PyLong_FromLong(thread__pid(pthread->thread));
 }
 
 static PyObject *pyrf_thread__get_tid(struct pyrf_thread *pthread, void *closure __maybe_unused)
 {
+	CHECK_INITIALIZED(pthread->thread, "thread");
 	return PyLong_FromLong(thread__tid(pthread->thread));
 }
 
 static PyObject *pyrf_thread__get_ppid(struct pyrf_thread *pthread, void *closure __maybe_unused)
 {
+	CHECK_INITIALIZED(pthread->thread, "thread");
 	return PyLong_FromLong(thread__ppid(pthread->thread));
 }
 
 static PyObject *pyrf_thread__get_cpu(struct pyrf_thread *pthread, void *closure __maybe_unused)
 {
+	CHECK_INITIALIZED(pthread->thread, "thread");
 	return PyLong_FromLong(thread__cpu(pthread->thread));
 }
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH v2 12/16] perf python: Add thread and PMU uninitialized checks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added CHECK_INITIALIZED macro in pyrf_pmu__events() leaks the py_list object when the pmu is not initialized.
--

commit 579d96b72f035d8f31919edd0270e2cfa16975d3
Author: Ian Rogers <irogers@google.com>

perf python: Add thread and PMU uninitialized checks

Add CHECK_INITIALIZED checks to the thread attribute getters
(get_pid, get_tid, get_ppid) to prevent crashes if they are accessed
before being properly initialized.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 71be130b38c53..b17b191b16255 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> @@ -1730,6 +1731,8 @@ static PyObject *pyrf_pmu__events(PyObject *self)
>  	PyObject *py_list = PyList_New(0);
>  	int ret;
>  
> +	CHECK_INITIALIZED(ppmu->pmu, "pmu");
> +

[Severity: Medium]
If ppmu->pmu is uninitialized, won't this newly added check return early
and leak the py_list object allocated just above it?

Would it make sense to move the CHECK_INITIALIZED() macro before the
PyList_New(0) allocation?

>  	if (!py_list)
>  		return NULL;

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

[PATCH v2 13/16] perf python: Fix MetricGroup return type in perf.pyi

[Ian Rogers]

The metrics() function can return a dictionary where the value is either
a string or a list of strings, so the type signature in the stub file
should be Union[str, List[str]].

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/python/perf.pyi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/python/perf.pyi b/tools/perf/python/perf.pyi
index f266c9baffd0..58b0b3ed819d 100644
--- a/tools/perf/python/perf.pyi
+++ b/tools/perf/python/perf.pyi
@@ -1,5 +1,5 @@
 """Type stubs for the perf Python module."""
-from typing import Callable, Dict, List, Optional, Any, Iterator
+from typing import Callable, Dict, List, Optional, Any, Iterator, Union
 
 def config_get(name: str) -> Optional[str]:
     """Get a configuration value from perf config.
@@ -12,7 +12,7 @@ def config_get(name: str) -> Optional[str]:
     """
     ...
 
-def metrics() -> List[Dict[str, str]]:
+def metrics() -> List[Dict[str, Union[str, List[str]]]]:
     """Get a list of available metrics.
 
     Returns:
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 14/16] perf python: Fix count_values memory leak in pyrf_evsel__read

[Ian Rogers]

In pyrf_evsel__read, if PyArg_ParseTuple fails, the allocated count_values
is leaked. Move the allocation of count_values after the PyArg_ParseTuple
call to prevent the memory leak.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index b17b191b1625..e76a407cb2bb 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -2204,11 +2204,6 @@ static PyObject *pyrf_evsel__read(struct pyrf_evsel *pevsel,
 
 	CHECK_INITIALIZED(evsel, "evsel");
 
-	count_values = PyObject_New(struct pyrf_counts_values,
-							       &pyrf_counts_values__type);
-	if (!count_values)
-		return NULL;
-
 	if (!PyArg_ParseTuple(args, "ii", &cpu, &thread))
 		return NULL;
 
@@ -2227,6 +2222,10 @@ static PyObject *pyrf_evsel__read(struct pyrf_evsel *pevsel,
 	if (evsel__ensure_counts(evsel))
 		return PyErr_NoMemory();
 
+	count_values = PyObject_New(struct pyrf_counts_values, &pyrf_counts_values__type);
+	if (!count_values)
+		return NULL;
+
 	/* Set up pointers to the old and newly read counter values. */
 	old_count = perf_counts(evsel->prev_raw_counts, cpu_idx, thread_idx);
 	new_count = perf_counts(evsel->counts, cpu_idx, thread_idx);
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH v2 14/16] perf python: Fix count_values memory leak in pyrf_evsel__read

[sashiko-bot]

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

Pre-existing issues:
- [High] A partially failed allocation in `evsel__ensure_counts()` results in a NULL pointer dereference when retrying `pyrf_evsel__read()`.
- [High] The `evsel->prev_raw_counts` array allocated for Python `perf.evsel` objects is unconditionally leaked when the object is destroyed.
- [Medium] String duplication leak in `prepare_metric` on error paths.
--

commit 0bdfb7eb04b252f781d1daa5947d46bec1d9df82
Author: Ian Rogers <irogers@google.com>

perf python: Fix count_values memory leak in pyrf_evsel__read

In pyrf_evsel__read, if PyArg_ParseTuple fails, the allocated count_values
is leaked. Move the allocation of count_values after the PyArg_ParseTuple
call to prevent the memory leak.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index b17b191b16255..e76a407cb2bb9 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -2204,11 +2204,6 @@ static PyObject *pyrf_evsel__read(struct pyrf_evsel *pevsel,
>  
>  	CHECK_INITIALIZED(evsel, "evsel");
>  
> -	count_values = PyObject_New(struct pyrf_counts_values,
> -							       &pyrf_counts_values__type);
> -	if (!count_values)
> -		return NULL;
> -
>  	if (!PyArg_ParseTuple(args, "ii", &cpu, &thread))
>  		return NULL;
>  
> @@ -2227,6 +2222,10 @@ static PyObject *pyrf_evsel__read(struct pyrf_evsel *pevsel,
>  	if (evsel__ensure_counts(evsel))
>  		return PyErr_NoMemory();
>  
> +	count_values = PyObject_New(struct pyrf_counts_values, &pyrf_counts_values__type);
> +	if (!count_values)
> +		return NULL;
> +
>  	/* Set up pointers to the old and newly read counter values. */
>  	old_count = perf_counts(evsel->prev_raw_counts, cpu_idx, thread_idx);

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential NULL
pointer dereference here.

If evsel__ensure_counts() successfully allocates evsel->counts but fails to
allocate evsel->prev_raw_counts, it returns -ENOMEM. If a user script
handles the resulting Python MemoryError and retries the read operation,
evsel__ensure_counts() will see that evsel->counts is no longer NULL and
return success immediately, without allocating evsel->prev_raw_counts.

Will this cause perf_counts() to dereference a NULL prev_raw_counts pointer
on the retry?

[Severity: High]
This is also a pre-existing issue, but does evsel->prev_raw_counts leak
when the object is destroyed?

When a standalone Python evsel object is destroyed, its destruction path
goes through pyrf_evsel__delete() -> evsel__put() -> evsel__exit(). While
evsel__exit() properly calls evsel__free_counts() to free evsel->counts,
it seems to miss freeing evsel->prev_raw_counts.

Could this unconditionally drain memory if Python evsel objects are used in
a loop?

>  	new_count = perf_counts(evsel->counts, cpu_idx, thread_idx);

[ ... ]

[Severity: Medium]
This is another pre-existing issue not caused by this patch, but looking at
prepare_metric() in the same file, does this error path leak the duplicated
string n?

tools/perf/util/python.c:prepare_metric() {
    ...
    char *n = strdup(evsel__metric_id(cur));

    if (!n)
        return -ENOMEM;
    ...
    ret = evsel__ensure_counts(cur);
    if (ret)
        return ret;
    ...
    ret = expr__add_id_val_source_count(pctx, n, val, source_count);
    if (ret)
        return ret;
    ...
}

If evsel__ensure_counts() or expr__add_id_val_source_count() returns an
error, the function returns early without freeing n. Should we ensure n is
freed on these error paths?

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

[PATCH v2 15/16] perf python: Fix memory leak in pyrf__metrics_cb

[Ian Rogers]

In pyrf__metrics_cb, PyDict_SetItem does not steal the reference of the
key and value, so they need to be decref'ed after successful insertion
to avoid memory leaks.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/python.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index e76a407cb2bb..5471037d04c5 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -3567,6 +3567,8 @@ static int pyrf__metrics_cb(const struct pmu_metric *pm,
 		Py_XDECREF(dict);
 		return -ENOMEM;
 	}
+	Py_DECREF(key);
+	Py_DECREF(value);
 
 	if (!add_to_dict(dict, "MetricName", pm->metric_name) ||
 	    !add_to_dict(dict, "PMU", pm->pmu) ||
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH v2 16/16] perf synthetic-events: Fix divide by zero in perf_event__synthesize_threads

[Ian Rogers]

If scandir() finds no matching tasks in /proc, n is 0. If thread_nr is > 1,
we bypass the single-thread fast path and then clamp thread_nr to n, making
it 0. This results in a divide by zero when calculating num_per_thread.

Handle n <= 1 early to use the single-thread fast path and prevent the
crash.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/synthetic-events.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index 35de41ad14d3..19c1f3eaf58d 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -1058,7 +1058,7 @@ int perf_event__synthesize_threads(const struct perf_tool *tool,
 	else
 		thread_nr = nr_threads_synthesize;
 
-	if (thread_nr <= 1) {
+	if (thread_nr <= 1 || n <= 1) {
 		err = __perf_event__synthesize_threads(tool, process,
 						       machine,
 						       needs_mmap, mmap_data,
-- 
2.55.0.rc0.799.gd6f94ed593-goog