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

sashiko-bot
2026-04-23T04:19:50Z

Sashiko AI review found 2 potential issue(s):
- [Critical] Incorrect struct offset in `pyrf_stat_round_event__members` exposes Python internal object metadata to memory corruption.
- [High] Unhandled Python exceptions, reference leaks, and mismatched callback signatures in `pyrf_session_tool__stat_round`.
--

commit b0070e631de8a0052c29623f25930efb7c6511b2
Author: Ian Rogers <irogers@google.com>
Subject: perf python: Extend API for stat events in python.c

This commit extends the Python API for the perf tool by adding stat
information to the session and introducing callbacks for stat events.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index c707adf4785cd..b7f877189f8cf 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c

[ ... ]

> +static const char pyrf_stat_round_event__doc[] = PyDoc_STR("perf stat round event object.");
> +
> +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" },

The offset for the stat_round_type attribute is manually specified as
offsetof(struct perf_record_stat_round, type).

Does this need to include the base offset of the event union within the
struct pyrf_event wrapper object?

Without the base offset, this evaluates to 8 bytes, which on 64-bit systems
points directly to the ob_type pointer within the PyObject_HEAD struct.

Could reading or writing this attribute from a Python script inadvertently
manipulate the object's type pointer?

> + 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);

This invokes the user-provided Python stat callback with a single argument,
but pyrf_session_tool__stat() invokes the same callback with two arguments
("Os", pyevent, name).

Will this mismatched signature cause a TypeError when a typical Python
callback expecting two arguments is called from here?

Additionally, is it safe to ignore the return value of
PyObject_CallFunction()?

If the call succeeds, it looks like the returned Python object reference
might leak. If it fails, the code doesn't check for NULL or call
PyErr_Print(), which might leave an active exception uncleared on the
Python thread state.

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

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