| From 8c926b249823faa6da8e74ac7ea9214ae0ec4b75 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 12 Mar 2025 17:31:40 -0300 |
| Subject: perf python: Don't keep a raw_data pointer to consumed ring buffer |
| space |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Arnaldo Carvalho de Melo <acme@redhat.com> |
| |
| [ Upstream commit f3fed3ae34d606819d87a63d970cc3092a5be7ab ] |
| |
| When processing tracepoints the perf python binding was parsing the |
| event before calling perf_mmap__consume(&md->core) in |
| pyrf_evlist__read_on_cpu(). |
| |
| But part of this event parsing was to set the perf_sample->raw_data |
| pointer to the payload of the event, which then could be overwritten by |
| other event before tracepoint fields were asked for via event.prev_comm |
| in a python program, for instance. |
| |
| This also happened with other fields, but strings were were problems |
| were surfacing, as there is UTF-8 validation for the potentially garbled |
| data. |
| |
| This ended up showing up as (with some added debugging messages): |
| |
| ( field 'prev_comm' ret=0x7f7c31f65110, raw_size=68 ) ( field 'prev_pid' ret=0x7f7c23b1bed0, raw_size=68 ) ( field 'prev_prio' ret=0x7f7c239c0030, raw_size=68 ) ( field 'prev_state' ret=0x7f7c239c0250, raw_size=68 ) time 14771421785867 prev_comm= prev_pid=1919907691 prev_prio=796026219 prev_state=0x303a32313175 ==> |
| ( XXX '��' len=16, raw_size=68) ( field 'next_comm' ret=(nil), raw_size=68 ) Traceback (most recent call last): |
| File "/home/acme/git/perf-tools-next/tools/perf/python/tracepoint.py", line 51, in <module> |
| main() |
| File "/home/acme/git/perf-tools-next/tools/perf/python/tracepoint.py", line 46, in main |
| event.next_comm, |
| ^^^^^^^^^^^^^^^ |
| AttributeError: 'perf.sample_event' object has no attribute 'next_comm' |
| |
| When event.next_comm was asked for, the PyUnicode_FromString() python |
| API would fail and that tracepoint field wouldn't be available, stopping |
| the tools/perf/python/tracepoint.py test tool. |
| |
| But, since we already do a copy of the whole event in pyrf_event__new, |
| just use it and while at it remove what was done in in e8968e654191390a |
| ("perf python: Fix pyrf_evlist__read_on_cpu event consuming") because we |
| don't really need to wait for parsing the sample before declaring the |
| event as consumed. |
| |
| This copy is questionable as is now, as it limits the maximum event + |
| sample_type and tracepoint payload to sizeof(union perf_event), this all |
| has been "working" because 'struct perf_event_mmap2', the largest entry |
| in 'union perf_event' is: |
| |
| $ pahole -C perf_event ~/bin/perf | grep mmap2 |
| struct perf_record_mmap2 mmap2; /* 0 4168 */ |
| $ |
| |
| Fixes: bae57e3825a3dded ("perf python: Add support to resolve tracepoint fields") |
| Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> |
| Reviewed-by: Ian Rogers <irogers@google.com> |
| Link: https://lore.kernel.org/r/20250312203141.285263-6-acme@kernel.org |
| Signed-off-by: Namhyung Kim <namhyung@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| tools/perf/util/python.c | 4 +--- |
| 1 file changed, 1 insertion(+), 3 deletions(-) |
| |
| diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c |
| index 894a9966599fd..efc8f88db5cff 100644 |
| --- a/tools/perf/util/python.c |
| +++ b/tools/perf/util/python.c |
| @@ -1116,11 +1116,9 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist, |
| |
| pevent->evsel = evsel; |
| |
| - err = evsel__parse_sample(evsel, event, &pevent->sample); |
| - |
| - /* Consume the even only after we parsed it out. */ |
| perf_mmap__consume(&md->core); |
| |
| + err = evsel__parse_sample(evsel, &pevent->event, &pevent->sample); |
| if (err) { |
| Py_DECREF(pyevent); |
| return PyErr_Format(PyExc_OSError, |
| -- |
| 2.39.5 |
| |