lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250312160651.280817-4-acme@kernel.org>
Date: Wed, 12 Mar 2025 13:06:51 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	James Clark <james.clark@...aro.org>,
	Jiri Olsa <jolsa@...nel.org>,
	Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	Clark Williams <williams@...hat.com>,
	linux-kernel@...r.kernel.org,
	linux-perf-users@...r.kernel.org,
	Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: [PATCH 3/3] perf python: Don't keep a raw_data pointer to consumed ring buffer space

From: Arnaldo Carvalho de Melo <acme@...hat.com>

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.

So if the event is a tracepoint, copy the raw_data before consuming the
ring buffer for the event.

This is similar to what was done in e8968e654191390a ("perf python: Fix
pyrf_evlist__read_on_cpu event consuming"), but wasn't when adding
support for tracepoints in bae57e3825a3dded ("perf python: Add support
to resolve tracepoint fields"), fix it.

Fixes: bae57e3825a3dded ("perf python: Add support to resolve tracepoint fields")
Cc: Adrian Hunter <adrian.hunter@...el.com>
Cc: Ian Rogers <irogers@...gle.com>
Cc: James Clark <james.clark@...aro.org>
Cc: Jiri Olsa <jolsa@...nel.org>
Cc: Kan Liang <kan.liang@...ux.intel.com>
Cc: Namhyung Kim <namhyung@...nel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
---
 tools/perf/util/python.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index f9491b6699764fbc..e1bd8143296385fc 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -4,6 +4,7 @@
 #include <inttypes.h>
 #include <poll.h>
 #include <linux/err.h>
+#include <linux/string.h>
 #include <perf/cpumap.h>
 #ifdef HAVE_LIBTRACEEVENT
 #include <event-parse.h>
@@ -38,9 +39,16 @@ struct pyrf_event {
 	PyObject_HEAD
 	struct evsel *evsel;
 	struct perf_sample sample;
+	/** @raw_data_copy: read_on_cpu consumes rb, may be overwritten at get_tracepoint_field */
+	void *raw_data_copy;
 	union perf_event   event;
 };
 
+static void pyrf_event__delete(struct pyrf_event *pevent)
+{
+	zfree(&pevent->raw_data_copy);
+}
+
 #define sample_members \
 	sample_member_def(sample_ip, ip, T_ULONGLONG, "event ip"),			 \
 	sample_member_def(sample_pid, pid, T_INT, "event pid"),			 \
@@ -94,6 +102,7 @@ static PyTypeObject pyrf_mmap_event__type = {
 	.tp_doc		= pyrf_mmap_event__doc,
 	.tp_members	= pyrf_mmap_event__members,
 	.tp_repr	= (reprfunc)pyrf_mmap_event__repr,
+	.tp_dealloc	= (destructor)pyrf_event__delete,
 };
 
 static const char pyrf_task_event__doc[] = PyDoc_STR("perf task (fork/exit) event object.");
@@ -129,6 +138,7 @@ static PyTypeObject pyrf_task_event__type = {
 	.tp_doc		= pyrf_task_event__doc,
 	.tp_members	= pyrf_task_event__members,
 	.tp_repr	= (reprfunc)pyrf_task_event__repr,
+	.tp_dealloc	= (destructor)pyrf_event__delete,
 };
 
 static const char pyrf_comm_event__doc[] = PyDoc_STR("perf comm event object.");
@@ -158,6 +168,7 @@ static PyTypeObject pyrf_comm_event__type = {
 	.tp_doc		= pyrf_comm_event__doc,
 	.tp_members	= pyrf_comm_event__members,
 	.tp_repr	= (reprfunc)pyrf_comm_event__repr,
+	.tp_dealloc	= (destructor)pyrf_event__delete,
 };
 
 static const char pyrf_throttle_event__doc[] = PyDoc_STR("perf throttle event object.");
@@ -190,6 +201,7 @@ static PyTypeObject pyrf_throttle_event__type = {
 	.tp_doc		= pyrf_throttle_event__doc,
 	.tp_members	= pyrf_throttle_event__members,
 	.tp_repr	= (reprfunc)pyrf_throttle_event__repr,
+	.tp_dealloc	= (destructor)pyrf_event__delete,
 };
 
 static const char pyrf_lost_event__doc[] = PyDoc_STR("perf lost event object.");
@@ -225,6 +237,7 @@ static PyTypeObject pyrf_lost_event__type = {
 	.tp_doc		= pyrf_lost_event__doc,
 	.tp_members	= pyrf_lost_event__members,
 	.tp_repr	= (reprfunc)pyrf_lost_event__repr,
+	.tp_dealloc	= (destructor)pyrf_event__delete,
 };
 
 static const char pyrf_read_event__doc[] = PyDoc_STR("perf read event object.");
@@ -255,6 +268,7 @@ static PyTypeObject pyrf_read_event__type = {
 	.tp_doc		= pyrf_read_event__doc,
 	.tp_members	= pyrf_read_event__members,
 	.tp_repr	= (reprfunc)pyrf_read_event__repr,
+	.tp_dealloc	= (destructor)pyrf_event__delete,
 };
 
 static const char pyrf_sample_event__doc[] = PyDoc_STR("perf sample event object.");
@@ -295,11 +309,14 @@ static PyObject*
 tracepoint_field(const struct pyrf_event *pe, struct tep_format_field *field)
 {
 	struct tep_handle *pevent = field->event->tep;
-	void *data = pe->sample.raw_data;
+	void *data = pe->raw_data_copy;
 	PyObject *ret = NULL;
 	unsigned long long val;
 	unsigned int offset, len;
 
+	if (data == NULL)
+		return ret;
+
 	if (field->flags & TEP_FIELD_IS_ARRAY) {
 		offset = field->offset;
 		len    = field->size;
@@ -346,6 +363,11 @@ get_tracepoint_field(struct pyrf_event *pevent, PyObject *attr_name)
 	field = tep_find_any_field(tp_format, str);
 	return field ? tracepoint_field(pevent, field) : NULL;
 }
+#else // HAVE_LIBTRACEEVENT
+static bool is_tracepoint(const struct pyrf_event *pevent __maybe_unused)
+{
+	return false;
+}
 #endif /* HAVE_LIBTRACEEVENT */
 
 static PyObject*
@@ -369,6 +391,7 @@ static PyTypeObject pyrf_sample_event__type = {
 	.tp_doc		= pyrf_sample_event__doc,
 	.tp_members	= pyrf_sample_event__members,
 	.tp_repr	= (reprfunc)pyrf_sample_event__repr,
+	.tp_dealloc	= (destructor)pyrf_event__delete,
 	.tp_getattro	= (getattrofunc) pyrf_sample_event__getattro,
 };
 
@@ -407,6 +430,7 @@ static PyTypeObject pyrf_context_switch_event__type = {
 	.tp_doc		= pyrf_context_switch_event__doc,
 	.tp_members	= pyrf_context_switch_event__members,
 	.tp_repr	= (reprfunc)pyrf_context_switch_event__repr,
+	.tp_dealloc	= (destructor)pyrf_event__delete,
 };
 
 static int pyrf_event__setup_types(void)
@@ -478,8 +502,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->raw_data_copy = NULL;
+	}
 	return (PyObject *)pevent;
 }
 
@@ -1020,6 +1046,14 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 
 		err = evsel__parse_sample(evsel, event, &pevent->sample);
 
+		if (is_tracepoint(pevent) &&
+		    pevent->sample.raw_size > 0 && !pevent->raw_data_copy) {
+			// No need to check here, only when get_tracepoint_field gets
+			// called, if ever, then it can fail getting the value for
+			// a tracepoint field.
+			pevent->raw_data_copy = memdup(pevent->sample.raw_data, pevent->sample.raw_size);
+		}
+
 		/* Consume the even only after we parsed it out. */
 		perf_mmap__consume(&md->core);
 
-- 
2.48.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ