Re: [PATCH v2 13/58] perf python: Use evsel in sample in pyrf_event

sashiko-bot
2026-04-23T06:01:00Z

Sashiko AI review found 2 potential issue(s):
- [Medium] Double-put of `evsel` reference count on `evsel__parse_sample()` error paths.
- [Medium] `evsel` reference leak in the Python perf extension for non-sample events.
--

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

perf python: Use evsel in sample in pyrf_event

This commit avoids a duplicated evsel by using the one present in the sample.
It adds evsel__get() and evsel__put() to the evsel stored in perf_sample.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 0162d8a625de3..0424290f8b77f 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> @@ -274,7 +273,6 @@ static PyMemberDef pyrf_sample_event__members[] = {
>
> 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);
> }

Does this leak the evsel reference for non-sample events?

In pyrf_evlist__read_on_cpu(), evsel__parse_sample() is called for all
event types, which acquires an evsel reference and stores it in the Python
event object:

pyrf_evlist__read_on_cpu()
err = evsel__parse_sample(evsel, &pevent->event, &pevent->sample);

However, only pyrf_sample_event__type has a custom tp_dealloc method
(pyrf_sample_event__delete) that calls perf_sample__exit().

Other event types fall back to the default Python deallocator and bypass
perf_sample__exit(), which skips dropping the refcount and leaks the evsel
reference along with any dynamic structures in perf_sample.

I see this regression is fixed later in the patch series by commit
ce9c27681f1b (perf python: Refactor and add accessors to sample event),
which adds a universal tp_dealloc for all event types.

[ ... ]
> diff --git a/tools/perf/util/sample.c b/tools/perf/util/sample.c
> index cf73329326d72..106848404fdf0 100644
> --- a/tools/perf/util/sample.c
> +++ b/tools/perf/util/sample.c
[ ... ]
> @@ -29,6 +34,7 @@ void perf_sample__init(struct perf_sample *sample, bool all)
>
> void perf_sample__exit(struct perf_sample *sample)
> {
> + evsel__put(sample->evsel);
> zfree(&sample->user_regs);
> zfree(&sample->intr_regs);

Does this cause a double-put on error paths?

If evsel__parse_sample() encounters an error and jumps to out_efault, it
calls perf_sample__exit() internally and drops the refcount.

If callers like do_test() in tools/perf/tests/sample-parsing.c or
pyrf_evlist__read_on_cpu() catch the error, they execute their own
cleanup paths and call perf_sample__exit() again on the same sample object.

Since sample->evsel is not set to NULL after evsel__put() here, could the
second call lead to a reference count underflow and a use-after-free?

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