[GIT PULL] perf tools updates

[GIT PULL] perf tools updates

[Frederic Weisbecker]

Ingo, Arnaldo,

Please pull the perf/core branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
	perf/core

Thanks,
	Frederic
---

Frederic Weisbecker (6):
      perf tools: Check we are able to read the event size on mmap
      perf tools: Remove junk code in mmap size handling
      perf tools: Move evlist sample helpers to evlist area
      perf tools: Pre-check sample size before parsing
      perf tools: Robustify dynamic sample content fetch
      perf tools: Propagate event parse error handling


 tools/perf/builtin-test.c |    9 ++++++++-
 tools/perf/builtin-top.c  |    7 ++++++-
 tools/perf/util/event.c   |   16 ++++++++++++++++
 tools/perf/util/event.h   |   12 +++++++++++-
 tools/perf/util/evlist.c  |   31 +++++++++++++++++++++++++++++++
 tools/perf/util/evlist.h  |    3 +++
 tools/perf/util/evsel.c   |   32 +++++++++++++++++++++++++++++++-
 tools/perf/util/header.c  |   31 -------------------------------
 tools/perf/util/header.h  |    2 --
 tools/perf/util/python.c  |   13 ++++++++++---
 tools/perf/util/session.c |   25 ++++++++++++++++++-------
 tools/perf/util/session.h |    2 ++
 12 files changed, 136 insertions(+), 47 deletions(-)

[PATCH 1/6] perf tools: Check we are able to read the event size on mmap

[Frederic Weisbecker]

Check we have enough mmaped space to read the current event
size from its headers, otherwise we may dereference some
hell there.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/session.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index fff6674..61746b5 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1007,6 +1007,13 @@ remap:
 	file_pos = file_offset + head;
 
 more:
+	/*
+	 * Ensure we have enough space remaining to read
+	 * the size of the event in the headers.
+	 */
+	if (head + sizeof(event->header) > mmap_size)
+		goto remap;
+
 	event = (union perf_event *)(buf + head);
 
 	if (session->header.needs_swap)
-- 
1.7.3.2

[PATCH 2/6] perf tools: Remove junk code in mmap size handling

[Frederic Weisbecker]

size is overriden later and used only then. Those
lines are only junk, probably a leftover.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/session.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 61746b5..db65206 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1018,9 +1018,6 @@ more:
 
 	if (session->header.needs_swap)
 		perf_event_header__bswap(&event->header);
-	size = event->header.size;
-	if (size == 0)
-		size = 8;
 
 	if (head + event->header.size > mmap_size) {
 		if (mmaps[map_idx]) {
-- 
1.7.3.2

[PATCH 4/6] perf tools: Pre-check sample size before parsing

[Frederic Weisbecker]

Check that the total size of the sample fields having a fixed
size do not exceed the one of the whole event. This robustifies
the sample parsing.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/builtin-test.c |    4 +++-
 tools/perf/util/event.c   |   16 ++++++++++++++++
 tools/perf/util/event.h   |   12 +++++++++++-
 tools/perf/util/evsel.c   |    6 +++++-
 tools/perf/util/python.c  |    5 +++--
 tools/perf/util/session.c |    1 +
 tools/perf/util/session.h |    2 ++
 7 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 11e3c84..44d7df2 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -474,6 +474,7 @@ static int test__basic_mmap(void)
 	unsigned int nr_events[nsyscalls],
 		     expected_nr_events[nsyscalls], i, j;
 	struct perf_evsel *evsels[nsyscalls], *evsel;
+	int sample_size = perf_sample_size(attr.sample_type);
 
 	for (i = 0; i < nsyscalls; ++i) {
 		char name[64];
@@ -558,7 +559,8 @@ static int test__basic_mmap(void)
 			goto out_munmap;
 		}
 
-		perf_event__parse_sample(event, attr.sample_type, false, &sample);
+		perf_event__parse_sample(event, attr.sample_type, sample_size,
+					 false, &sample);
 		evsel = perf_evlist__id2evsel(evlist, sample.id);
 		if (evsel == NULL) {
 			pr_debug("event with id %" PRIu64
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 1023f67..17c1c3c 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -35,6 +35,22 @@ const char *perf_event__name(unsigned int id)
 	return perf_event__names[id];
 }
 
+int perf_sample_size(u64 sample_type)
+{
+	u64 mask = sample_type & PERF_SAMPLE_MASK;
+	int size = 0;
+	int i;
+
+	for (i = 0; i < 64; i++) {
+		if ((mask << i) & 1)
+			size++;
+	}
+
+	size *= sizeof(u64);
+
+	return size;
+}
+
 static struct perf_sample synth_sample = {
 	.pid	   = -1,
 	.tid	   = -1,
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 9c35170..c083328 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -56,6 +56,13 @@ struct read_event {
 	u64 id;
 };
 
+
+#define PERF_SAMPLE_MASK				\
+	(PERF_SAMPLE_IP | PERF_SAMPLE_TID |		\
+	 PERF_SAMPLE_TIME | PERF_SAMPLE_ADDR |		\
+	PERF_SAMPLE_ID | PERF_SAMPLE_STREAM_ID |	\
+	 PERF_SAMPLE_CPU | PERF_SAMPLE_PERIOD)
+
 struct sample_event {
 	struct perf_event_header        header;
 	u64 array[];
@@ -75,6 +82,8 @@ struct perf_sample {
 	struct ip_callchain *callchain;
 };
 
+int perf_sample_size(u64 sample_type);
+
 #define BUILD_ID_SIZE 20
 
 struct build_id_event {
@@ -178,6 +187,7 @@ int perf_event__preprocess_sample(const union perf_event *self,
 const char *perf_event__name(unsigned int id);
 
 int perf_event__parse_sample(const union perf_event *event, u64 type,
-			     bool sample_id_all, struct perf_sample *sample);
+			     int sample_size, bool sample_id_all,
+			     struct perf_sample *sample);
 
 #endif /* __PERF_RECORD_H */
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d6fd59b..bfce8bf 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -304,7 +304,8 @@ static int perf_event__parse_id_sample(const union perf_event *event, u64 type,
 }
 
 int perf_event__parse_sample(const union perf_event *event, u64 type,
-			     bool sample_id_all, struct perf_sample *data)
+			     int sample_size, bool sample_id_all,
+			     struct perf_sample *data)
 {
 	const u64 *array;
 
@@ -319,6 +320,9 @@ int perf_event__parse_sample(const union perf_event *event, u64 type,
 
 	array = event->sample.array;
 
+	if (sample_size + sizeof(event->header) > event->header.size)
+		return -EFAULT;
+
 	if (type & PERF_SAMPLE_IP) {
 		data->ip = event->ip.ip;
 		array++;
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 8b0eff8..4174c09 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -690,8 +690,9 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 			return PyErr_NoMemory();
 
 		first = list_entry(evlist->entries.next, struct perf_evsel, node);
-		perf_event__parse_sample(event, first->attr.sample_type, sample_id_all,
-					 &pevent->sample);
+		perf_event__parse_sample(event, first->attr.sample_type,
+					 perf_sample_size(first->attr.sample_type),
+					 sample_id_all, &pevent->sample);
 		return pyevent;
 	}
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index db65206..8940fd8 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -97,6 +97,7 @@ out:
 void perf_session__update_sample_type(struct perf_session *self)
 {
 	self->sample_type = perf_evlist__sample_type(self->evlist);
+	self->sample_size = perf_sample_size(self->sample_type);
 	self->sample_id_all = perf_evlist__sample_id_all(self->evlist);
 	perf_session__id_header_size(self);
 }
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 8daaa2d..66d4e14 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -43,6 +43,7 @@ struct perf_session {
 	 */
 	struct hists		hists;
 	u64			sample_type;
+	int			sample_size;
 	int			fd;
 	bool			fd_pipe;
 	bool			repipe;
@@ -159,6 +160,7 @@ static inline int perf_session__parse_sample(struct perf_session *session,
 					     struct perf_sample *sample)
 {
 	return perf_event__parse_sample(event, session->sample_type,
+					session->sample_size,
 					session->sample_id_all, sample);
 }
 
-- 
1.7.3.2

[PATCH 5/6] perf tools: Robustify dynamic sample content fetch

[Frederic Weisbecker]

Ensure the size of the dynamic fields such as callchains
or raw events don't overlap the whole event boundaries.

This prevents from dereferencing junk if the given size of
the callchain goes too eager.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/util/evsel.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index bfce8bf..ee0fe0d 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -303,6 +303,17 @@ static int perf_event__parse_id_sample(const union perf_event *event, u64 type,
 	return 0;
 }
 
+static bool sample_overlap(const union perf_event *event,
+			   const void *offset, u64 size)
+{
+	const void *base = event;
+
+	if (offset + size > base + event->header.size)
+		return true;
+
+	return false;
+}
+
 int perf_event__parse_sample(const union perf_event *event, u64 type,
 			     int sample_size, bool sample_id_all,
 			     struct perf_sample *data)
@@ -373,14 +384,29 @@ int perf_event__parse_sample(const union perf_event *event, u64 type,
 	}
 
 	if (type & PERF_SAMPLE_CALLCHAIN) {
+		if (sample_overlap(event, array, sizeof(data->callchain->nr)))
+			return -EFAULT;
+
 		data->callchain = (struct ip_callchain *)array;
+
+		if (sample_overlap(event, array, data->callchain->nr))
+			return -EFAULT;
+
 		array += 1 + data->callchain->nr;
 	}
 
 	if (type & PERF_SAMPLE_RAW) {
 		u32 *p = (u32 *)array;
+
+		if (sample_overlap(event, array, sizeof(u32)))
+			return -EFAULT;
+
 		data->raw_size = *p;
 		p++;
+
+		if (sample_overlap(event, p, data->raw_size))
+			return -EFAULT;
+
 		data->raw_data = p;
 	}
 
-- 
1.7.3.2

[PATCH 6/6] perf tools: Propagate event parse error handling

[Frederic Weisbecker]

Better handle event parsing error by propagating the details
in upper layers or by dumping some failure message. So that
the user knows he has some crazy events in the batch.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
 tools/perf/builtin-test.c |    9 +++++++--
 tools/perf/builtin-top.c  |    7 ++++++-
 tools/perf/util/python.c  |   14 ++++++++++----
 tools/perf/util/session.c |   14 ++++++++++----
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 44d7df2..1fa9f58 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -559,8 +559,13 @@ static int test__basic_mmap(void)
 			goto out_munmap;
 		}
 
-		perf_event__parse_sample(event, attr.sample_type, sample_size,
-					 false, &sample);
+		err = perf_event__parse_sample(event, attr.sample_type, sample_size,
+					       false, &sample);
+		if (err) {
+			pr_err("Can't parse sample, err = %d\n", err);
+			goto out_munmap;
+		}
+
 		evsel = perf_evlist__id2evsel(evlist, sample.id);
 		if (evsel == NULL) {
 			pr_debug("event with id %" PRIu64
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 7e3d6e3..74f533c 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -805,9 +805,14 @@ static void perf_session__mmap_read_cpu(struct perf_session *self, int cpu)
 {
 	struct perf_sample sample;
 	union perf_event *event;
+	int ret;
 
 	while ((event = perf_evlist__read_on_cpu(top.evlist, cpu)) != NULL) {
-		perf_session__parse_sample(self, event, &sample);
+		ret = perf_session__parse_sample(self, event, &sample);
+		if (ret) {
+			pr_err("Can't parse sample, err = %d\n", ret);
+			continue;
+		}
 
 		if (event->header.type == PERF_RECORD_SAMPLE)
 			perf_event__process_sample(event, &sample, self);
diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
index 4174c09..3344d6e 100644
--- a/tools/perf/util/python.c
+++ b/tools/perf/util/python.c
@@ -675,6 +675,7 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 	union perf_event *event;
 	int sample_id_all = 1, cpu;
 	static char *kwlist[] = {"sample_id_all", NULL, NULL};
+	int err;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "i|i", kwlist,
 					 &cpu, &sample_id_all))
@@ -690,12 +691,17 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
 			return PyErr_NoMemory();
 
 		first = list_entry(evlist->entries.next, struct perf_evsel, node);
-		perf_event__parse_sample(event, first->attr.sample_type,
-					 perf_sample_size(first->attr.sample_type),
-					 sample_id_all, &pevent->sample);
+		err = perf_event__parse_sample(event, first->attr.sample_type,
+					       perf_sample_size(first->attr.sample_type),
+					       sample_id_all, &pevent->sample);
+		if (err) {
+			pr_err("Can't parse sample, err = %d\n", err);
+			goto end;
+		}
+
 		return pyevent;
 	}
-
+end:
 	Py_INCREF(Py_None);
 	return Py_None;
 }
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 8940fd8..948327d 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -480,6 +480,7 @@ static void flush_sample_queue(struct perf_session *s,
 	struct perf_sample sample;
 	u64 limit = os->next_flush;
 	u64 last_ts = os->last_sample ? os->last_sample->timestamp : 0ULL;
+	int ret;
 
 	if (!ops->ordered_samples || !limit)
 		return;
@@ -488,9 +489,12 @@ static void flush_sample_queue(struct perf_session *s,
 		if (iter->timestamp > limit)
 			break;
 
-		perf_session__parse_sample(s, iter->event, &sample);
-		perf_session_deliver_event(s, iter->event, &sample, ops,
-					   iter->file_offset);
+		ret = perf_session__parse_sample(s, iter->event, &sample);
+		if (ret)
+			pr_err("Can't parse sample, err = %d\n", ret);
+		else
+			perf_session_deliver_event(s, iter->event, &sample, ops,
+						   iter->file_offset);
 
 		os->last_flush = iter->timestamp;
 		list_del(&iter->list);
@@ -806,7 +810,9 @@ static int perf_session__process_event(struct perf_session *session,
 	/*
 	 * For all kernel events we get the sample data
 	 */
-	perf_session__parse_sample(session, event, &sample);
+	ret = perf_session__parse_sample(session, event, &sample);
+	if (ret)
+		return ret;
 
 	/* Preprocess sample records - precheck callchains */
 	if (perf_session__preprocess_sample(session, event, &sample))
-- 
1.7.3.2

Re: [GIT PULL] perf tools updates

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> Ingo, Arnaldo,
> 
> Please pull the perf/core branch that can be found at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> 	perf/core
> 
> Thanks,
> 	Frederic
> ---
> 
> Frederic Weisbecker (6):
>       perf tools: Check we are able to read the event size on mmap
>       perf tools: Remove junk code in mmap size handling
>       perf tools: Move evlist sample helpers to evlist area
>       perf tools: Pre-check sample size before parsing
>       perf tools: Robustify dynamic sample content fetch
>       perf tools: Propagate event parse error handling
> 
> 
>  tools/perf/builtin-test.c |    9 ++++++++-
>  tools/perf/builtin-top.c  |    7 ++++++-
>  tools/perf/util/event.c   |   16 ++++++++++++++++
>  tools/perf/util/event.h   |   12 +++++++++++-
>  tools/perf/util/evlist.c  |   31 +++++++++++++++++++++++++++++++
>  tools/perf/util/evlist.h  |    3 +++
>  tools/perf/util/evsel.c   |   32 +++++++++++++++++++++++++++++++-
>  tools/perf/util/header.c  |   31 -------------------------------
>  tools/perf/util/header.h  |    2 --
>  tools/perf/util/python.c  |   13 ++++++++++---
>  tools/perf/util/session.c |   25 ++++++++++++++++++-------
>  tools/perf/util/session.h |    2 ++
>  12 files changed, 136 insertions(+), 47 deletions(-)

To get this upstream ASAP i pulled them into perf/urgent and resolved two 
conflicts there (a contextual and a semantic one) - please double check my 
resolutions.

Thanks,

	Ingo

Re: [GIT PULL] perf tools updates

[Frederic Weisbecker]

On Sun, May 22, 2011 at 10:49:34AM +0200, Ingo Molnar wrote:
> 
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> 
> > Ingo, Arnaldo,
> > 
> > Please pull the perf/core branch that can be found at:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> > 	perf/core
> > 
> > Thanks,
> > 	Frederic
> > ---
> > 
> > Frederic Weisbecker (6):
> >       perf tools: Check we are able to read the event size on mmap
> >       perf tools: Remove junk code in mmap size handling
> >       perf tools: Move evlist sample helpers to evlist area
> >       perf tools: Pre-check sample size before parsing
> >       perf tools: Robustify dynamic sample content fetch
> >       perf tools: Propagate event parse error handling
> > 
> > 
> >  tools/perf/builtin-test.c |    9 ++++++++-
> >  tools/perf/builtin-top.c  |    7 ++++++-
> >  tools/perf/util/event.c   |   16 ++++++++++++++++
> >  tools/perf/util/event.h   |   12 +++++++++++-
> >  tools/perf/util/evlist.c  |   31 +++++++++++++++++++++++++++++++
> >  tools/perf/util/evlist.h  |    3 +++
> >  tools/perf/util/evsel.c   |   32 +++++++++++++++++++++++++++++++-
> >  tools/perf/util/header.c  |   31 -------------------------------
> >  tools/perf/util/header.h  |    2 --
> >  tools/perf/util/python.c  |   13 ++++++++++---
> >  tools/perf/util/session.c |   25 ++++++++++++++++++-------
> >  tools/perf/util/session.h |    2 ++
> >  12 files changed, 136 insertions(+), 47 deletions(-)
> 
> To get this upstream ASAP i pulled them into perf/urgent and resolved two 
> conflicts there (a contextual and a semantic one) - please double check my 
> resolutions.

Yeah looks good.

Thanks!