Re: [PATCH v5 20/58] perf python: Extend API for stat events in python.c

sashiko-bot
2026-04-24T17:17:04Z

Sashiko AI review found 4 potential issue(s):
- [Critical] The `stat_round_type` field has an incorrect memory offset, leading to memory corruption.
- [High] The `pyrf_session_tool__stat_round` function leaks memory and fails to handle Python exceptions.
- [High] A single Python callback is overloaded for `stat` and `stat_round` events with incompatible argument signatures.
- [High] Exposing `sample_members` for stat events without initialization leaks garbage memory data to Python.
--

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

perf python: Extend API for stat events in python.c

Add stat information to the session. Add call backs for stat events.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index dc10d8a42d9247..c4f0e01b64f364 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -299,6 +299,76 @@ static PyTypeObject pyrf_lost_event__type = {

[ ... ]

> +static PyMemberDef pyrf_stat_event__members[] = {
> + sample_members

Do the sample_members macros expose uninitialized memory here? Unlike
sample events where evsel__parse_sample() initializes the pevent->sample
structure, stat events are created with PyObject_New() which does not
zero-initialize the memory.

If a Python script accesses any sample_* attribute on a stat event, could
this read uninitialized C heap memory?

> + member_def(perf_event_header, type, T_UINT, "event type"),
> + member_def(perf_record_stat, id, T_ULONGLONG, "event id"),

[ ... ]

> +static PyMemberDef pyrf_stat_round_event__members[] = {
> + sample_members
> + member_def(perf_event_header, type, T_UINT, "event type"),
> + { .name = "stat_round_type", .type = T_ULONGLONG,
> + .offset = offsetof(struct perf_record_stat_round, type), .doc = "round type" },

Does this offset calculation corrupt the Python object header?

The member_def macro correctly calculates offsets from the base of the
allocated Python object by adding offsetof(struct pyrf_event, event).

By omitting the base offset for struct pyrf_event, reading or writing
stat_round_type in Python might write directly into the PyObject_HEAD fields.

> + member_def(perf_record_stat_round, time, T_ULONGLONG, "round time"),
> + { .name = NULL, },
> +};

[ ... ]

> +static int pyrf_session_tool__stat_round(const struct perf_tool *tool,
> + struct perf_session *session __maybe_unused,
> + union perf_event *event)
> +{
> + struct pyrf_session *psession = container_of(tool, struct pyrf_session, tool);
> + PyObject *pyevent = pyrf_event__new(event);
> +
> + if (pyevent == NULL)
> + return -ENOMEM;
> +
> + PyObject_CallFunction(psession->stat, "O", pyevent);

Since psession->stat is used here with one argument ("O"), but
pyrf_session_tool__stat calls it with two arguments ("Os"), will this cause
a TypeError in Python when a single callback function is registered for
both events?

Also, should the return value of PyObject_CallFunction() be checked and
decremented?

It returns a new reference on success which seems to leak since Py_DECREF()
is not called.

Additionally, if the Python callback raises an exception, the function
returns NULL. Without a check and a call to PyErr_Print(), the Python
interpreter might remain in an error state.

> + Py_DECREF(pyevent);
> + return 0;
> +}

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