Re: [PATCH v7 11/59] perf evsel: Add reference count

sashiko-bot
2026-04-25T23:18:06Z

Sashiko AI review found 5 potential issue(s):
- [Critical] `pyrf_evlist__add` blindly casts arbitrary Python objects, allowing type confusion and arbitrary memory writes.
- [High] Premature `evsel__put()` on a borrowed pointer in `evlist__deliver_deferred_callchain()` causes a use-after-free.
- [High] Uninitialized Python `perf.evsel` objects bypass `tp_getattro` protection, causing NULL pointer dereferences.
- [High] Early deallocation of `pyrf_event` passes uninitialized memory to `perf_sample__exit()`, risking a crash.
- [Medium] Python wrapper leaks `evsel` reference counts for non-sample events.
--

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

perf evsel: Add reference count

This commit adds reference counting to evsel to prevent assertion failures
in the Python code when events are cloned. It initializes the reference count
to 1 at allocation and decrements it on deletion. Getset functions are added
for the evsel members so the Python API remains unchanged.

> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> @@ -506,8 +507,10 @@ static PyObject *pyrf_event__new(const union perf_event *event)
>
> ptype = pyrf_event__type[event->header.type];
> pevent = PyObject_New(struct pyrf_event, ptype);
> - if (pevent != NULL)
> + if (pevent != NULL) {
> memcpy(&pevent->event, event, event->header.size);
> + pevent->evsel = NULL;
> + }
> return (PyObject *)pevent;
> }

Does this pass uninitialized memory to perf_sample__exit()?

When pyrf_event__new() creates a new object, pevent->sample is left
uninitialized. If event parsing fails shortly after creation, the object
is deallocated:

> static void pyrf_sample_event__delete(struct pyrf_event *pevent)
> {
> + evsel__put(pevent->evsel);
> perf_sample__exit(&pevent->sample);
> Py_TYPE(pevent)->tp_free((PyObject*)pevent);
> }

Calling perf_sample__exit() on garbage memory will attempt to free invalid
pointers, such as zfree(&sample->user_regs).

I see this was fixed later in the patch series (perf python: Use evsel
in sample in pyrf_event) by adding perf_sample__init(&pevent->sample, false)
to the initialization, but I wanted to mention it here.

[ ... ]
> +static PyObject *pyrf_evsel__str(PyObject *self)
> +{
> + struct pyrf_evsel *pevsel = (void *)self;
> + struct evsel *evsel = pevsel->evsel;
> +
> + return PyUnicode_FromFormat("evsel(%s/%s/)", evsel__pmu_name(evsel), evsel__name(evsel));
> +}

Could an uninitialized perf.evsel object cause a NULL pointer dereference
here?

The patch adds a tp_getattro handler to protect against uninitialized
objects (where pevsel->evsel is NULL). However, tp_getattro only intercepts
attribute reads.

Direct method calls like str(e) or repr(e) invoke tp_str, which passes the
NULL pointer directly to evsel__pmu_name(), leading to a segmentation fault.
Similarly, property setters (like e.tracking = True) use the C setters
directly, bypassing tp_getattro and dereferencing the NULL pointer.

[ ... ]
> @@ -1566,9 +1738,9 @@ static PyObject *pyrf_evlist__add(struct pyrf_evlist *pevlist,
> return NULL;
>
> Py_INCREF(pevsel);
> - evsel = &((struct pyrf_evsel *)pevsel)->evsel;
> + evsel = ((struct pyrf_evsel *)pevsel)->evsel;
> evsel->core.idx = evlist->core.nr_entries;
> - evlist__add(evlist, evsel);
> + evlist__add(evlist, evsel__get(evsel));
>
> return Py_BuildValue("i", evlist->core.nr_entries);
> }

Can this lead to an arbitrary memory write if a Python user passes the wrong
object type?

PyArg_ParseTuple(args, "O", &pevsel) accepts any Python object without
checking if it is actually a pyrf_evsel. It is then blindly cast to
struct pyrf_evsel *, and its evsel pointer is dereferenced to write the
index.

While this type confusion existed before this patch, changing it from an
address-of operator to a direct pointer dereference turns it into an
arbitrary memory write. Should this use "O!" with &pyrf_evsel__type to
enforce type safety?

[ ... ]
> @@ -1626,7 +1798,7 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
> return Py_None;
> }
>
> - pevent->evsel = evsel;
> + pevent->evsel = evsel__get(evsel);
>
> perf_mmap__consume(&md->core);

Does this leak reference counts for non-sample events?

Here, evsel__get() is called for all parsed events. However, only
pyrf_sample_event__type overrides the destructor to call evsel__put():

> static void pyrf_sample_event__delete(struct pyrf_event *pevent)
> {
> + evsel__put(pevent->evsel);
> perf_sample__exit(&pevent->sample);
> Py_TYPE(pevent)->tp_free((PyObject*)pevent);
> }

Other event types (like mmap or comm) fall back to the default deallocator,
meaning their evsel reference counts are never decremented.

I note this is addressed in subsequent commits by unifying the event
destructors, but wanted to bring it up.

> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
[ ... ]
> @@ -1373,6 +1373,7 @@ static int evlist__deliver_deferred_callchain(struct evlist *evlist,
> sample->evsel = evlist__id2evsel(evlist, sample->id);
> ret = tool->callchain_deferred(tool, event, sample,
> sample->evsel, machine);
> + evsel__put(sample->evsel);
> sample->evsel = saved_evsel;
> return ret;
> }

Does this cause a use-after-free? evlist__id2evsel() returns a borrowed
pointer to the evsel without incrementing its reference count. Calling
evsel__put() here drops the reference count incorrectly.

I see this was fixed later in the patch series (in perf python: Use evsel
in sample in pyrf_event) by adding the missing evsel__get(), but I am
pointing it out for completeness.

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