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

sashiko-bot
2026-04-25T23:15:03Z

Sashiko AI review found 4 potential issue(s):
- [High] Unhandled Python exception leak in `pyrf_session_tool__stat_round` causes interpreter crashes.
- [High] Callback argument mismatch causes immediate `TypeError` in `STAT_ROUND` events.
- [Medium] Unreachable size checks for new event types in `pyrf_event__new` due to restrictive bounds check.
- [Medium] Out-of-bounds read in `pyrf_session_tool__stat` before event size validation.
--

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

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

This commit adds stat information to the session and introduces callbacks for
stat events in the perf python extension.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 5478561ca62c..17d0ee15336f 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> @@ -1026,7 +1105,9 @@ static PyObject *pyrf_event__new(const union perf_event *event, struct evsel *ev
> if ((event->header.type < PERF_RECORD_MMAP ||
> event->header.type > PERF_RECORD_SAMPLE) &&
> !(event->header.type == PERF_RECORD_SWITCH ||
> - event->header.type == PERF_RECORD_SWITCH_CPU_WIDE)) {
> + event->header.type == PERF_RECORD_SWITCH_CPU_WIDE ||
> + event->header.type == PERF_RECORD_STAT ||
> + event->header.type == PERF_RECORD_STAT_ROUND)) {
> PyErr_Format(PyExc_TypeError, "Unexpected header type %u",
> event->header.type);
> return NULL;
> @@ -1038,6 +1119,9 @@ static PyObject *pyrf_event__new(const union perf_event *event, struct evsel *ev
> case PERF_RECORD_MMAP:
> min_size = sizeof(struct perf_record_mmap);
> break;
> + case PERF_RECORD_MMAP2:
> + min_size = sizeof(struct perf_record_mmap2);
> + break;

Are the new switch cases reachable for events like PERF_RECORD_MMAP2 and
PERF_RECORD_AUX?

The if condition at the start of the function restricts event->header.type to
be <= PERF_RECORD_SAMPLE, or one of a few explicitly allowed types like
SWITCH or STAT.

Since MMAP2 and other newly added types fall outside this whitelist, it seems
they will be rejected early with a TypeError before reaching this switch
statement.

[ ... ]
> @@ -3117,6 +3326,47 @@ static int pyrf_session_tool__sample(const struct perf_tool *tool,
> return 0;
> }
>
> +static int pyrf_session_tool__stat(const struct perf_tool *tool,
> + struct perf_session *session,
> + union perf_event *event)
> +{
> + struct pyrf_session *psession = container_of(tool, struct pyrf_session, tool);
> + struct evsel *evsel = evlist__id2evsel(session->evlist, event->stat.id);

Does reading event->stat.id here bypass size bounds checking?

This reads the event structure before passing it to pyrf_event__new(),
which performs the size validation against min_size.

If a truncated event is smaller than sizeof(struct perf_record_stat), will
this read past the end of the event buffer?

> + PyObject *pyevent = pyrf_event__new(event, evsel, psession->session);
> + const char *name = evsel ? evsel__name(evsel) : "unknown";
> + PyObject *ret;
> +
> + if (pyevent == NULL)
> + return -ENOMEM;
> +
> + ret = PyObject_CallFunction(psession->stat, "Os", pyevent, name);
> + if (!ret) {
> + PyErr_Print();
> + Py_DECREF(pyevent);
> + return -1;
> + }
> + Py_DECREF(ret);
> + Py_DECREF(pyevent);
> + return 0;
> +}
> +
> +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, /*evsel=*/NULL, psession->session);
> + PyObject *ret;
> +
> + if (pyevent == NULL)
> + return -ENOMEM;
> +
> + ret = PyObject_CallFunction(psession->stat, "O", pyevent);

Does this mismatch in argument counts cause problems for the Python callback?

In pyrf_session__init(), the identical user-provided callback (psession->stat)
is assigned for both PERF_RECORD_STAT and PERF_RECORD_STAT_ROUND events.

However, pyrf_session_tool__stat() calls it with two arguments ("Os"), while
this function calls it with just one ("O").

If the user's Python callback is defined to accept two arguments, will it
raise a TypeError when a STAT_ROUND event arrives?

> + Py_XDECREF(ret);
> + Py_DECREF(pyevent);
> + return 0;

If the Python callback raises an exception, PyObject_CallFunction() returns
NULL.

Since this function doesn't clear the exception (via PyErr_Clear() or
PyErr_Print()) and returns 0, does this leak an unhandled Python exception?

Returning to the C-API caller with an active exception usually causes a
SystemError in the interpreter.

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