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

sashiko-bot
2026-04-25T18:19:48Z

Sashiko AI review found 3 potential issue(s):
- [High] An unbalanced `evsel__put()` on a borrowed reference in `evlist__deliver_deferred_callchain` leads to a Use-After-Free.
- [Medium] The python bindings for `pyrf_evsel` are vulnerable to NULL pointer dereferences via property setters and methods when operating on uninitialized objects.
- [Medium] The Python `pevsel` object is permanently leaked in `pyrf_evlist__add` because `Py_INCREF` is used without a corresponding `Py_DECREF`.
--

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

perf evsel: Add reference count

This commit adds reference counting to evsel to resolve issues with the
python code cloning evsels. It initializes the count to 1 at allocation
and updates the Python bindings to use getset functions and reference
counting instead of cloning.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> +static int pyrf_evsel__set_attr_config(PyObject *self, PyObject *val, void */*closure*/)
> +{
> + struct pyrf_evsel *pevsel = (void *)self;
> +
> + pevsel->evsel->core.attr.config = PyLong_AsUnsignedLongLong(val);
> + return PyErr_Occurred() ? -1 : 0;
> +}

Does this code allow a NULL pointer dereference if the python object is
instantiated without calling __init__?

While tp_getattro was added to check for initialization during attribute
reads, it seems tp_setattro is not overridden.

When setting an attribute, the setter is called directly and blindly
dereferences pevsel->evsel, which would be NULL and crash the interpreter.
The same issue appears to exist in pyrf_evlist__add below.

[ ... ]
> @@ -1566,9 +1728,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);
> }

Does this code leak the Python pevsel object?

The patch changes the C evsel to be reference counted, and evsel__get takes
a reference for the evlist.

However, Py_INCREF is still used here without a corresponding Py_DECREF
because pevlist does not track it internally, leading to a memory leak every
time evlist.add is called.

Additionally, as mentioned above, if pevsel was created without __init__,
evsel would be NULL here, leading to a crash when dereferencing
evsel->core.idx.

> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> --- 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;
> }

Can this cause a use-after-free?

evlist__id2evsel returns a borrowed reference from the evlist without
incrementing the refcount. Calling evsel__put here drops a reference it never
acquired, which might prematurely free the evsel while it remains in the
evlist.

Note that I see this was addressed in a later commit 7675a76ec470 ("perf
evlist: Add reference count checking") which adds the missing evsel__get.

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