perf: perf report stuck in an infinite loop

perf: perf report stuck in an infinite loop

[Vince Weaver]

Currently the perf_data_fuzzer causes perf report to get stuck in an 
infinite loop.

From what I can tell, the issue happens in reader__process_events()
when an event is mapped using mmap(), but when it goes to process the
event finds out the internal event header has the size (invalidly) set to 
something much larger than the mmap buffer size.  This means 
fetch_mmaped_event() fails, which gotos remap: which tries again with
the exact same mmap size, and this will loop forever.

I haven't been able to puzzle out how to fix this, but maybe you have a 
better feel for what's going on here.

Vince

Re: perf: perf report stuck in an infinite loop

[Arnaldo Carvalho de Melo]

Em Fri, Jul 26, 2019 at 04:46:51PM -0400, Vince Weaver escreveu:
> 
> Currently the perf_data_fuzzer causes perf report to get stuck in an 
> infinite loop.
> 
> >From what I can tell, the issue happens in reader__process_events()
> when an event is mapped using mmap(), but when it goes to process the
> event finds out the internal event header has the size (invalidly) set to 
> something much larger than the mmap buffer size.  This means 
> fetch_mmaped_event() fails, which gotos remap: which tries again with
> the exact same mmap size, and this will loop forever.
> 
> I haven't been able to puzzle out how to fix this, but maybe you have a 
> better feel for what's going on here.

Perhaps the patch below?

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 37efa1f43d8b..f670c028f84b 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <errno.h>
 #include <inttypes.h>
+#include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/zalloc.h>
 #include <traceevent/event-parse.h>
@@ -1954,7 +1955,7 @@ fetch_mmaped_event(struct perf_session *session,
 		/* We're not fetching the event so swap back again */
 		if (session->header.needs_swap)
 			perf_event_header__bswap(&event->header);
-		return NULL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	return event;
@@ -1972,6 +1973,9 @@ static int __perf_session__process_decomp_events(struct perf_session *session)
 	while (decomp->head < decomp->size && !session_done()) {
 		union perf_event *event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data);
 
+		if (IS_ERR(event))
+			return PTR_ERR(event);
+
 		if (!event)
 			break;
 
@@ -2071,6 +2075,9 @@ reader__process_events(struct reader *rd, struct perf_session *session,
 
 more:
 	event = fetch_mmaped_event(session, head, mmap_size, buf);
+	if (IS_ERR(event))
+		return PTR_ERR(event);
+
 	if (!event) {
 		if (mmaps[map_idx]) {
 			munmap(mmaps[map_idx], mmap_size);

Re: perf: perf report stuck in an infinite loop

[Vince Weaver]

On Fri, 26 Jul 2019, Arnaldo Carvalho de Melo wrote:

> Em Fri, Jul 26, 2019 at 04:46:51PM -0400, Vince Weaver escreveu:
> > 
> > Currently the perf_data_fuzzer causes perf report to get stuck in an 
> > infinite loop.
> > 
> > >From what I can tell, the issue happens in reader__process_events()
> > when an event is mapped using mmap(), but when it goes to process the
> > event finds out the internal event header has the size (invalidly) set to 
> > something much larger than the mmap buffer size.  This means 
> > fetch_mmaped_event() fails, which gotos remap: which tries again with
> > the exact same mmap size, and this will loop forever.
> > 
> > I haven't been able to puzzle out how to fix this, but maybe you have a 
> > better feel for what's going on here.
> 
> Perhaps the patch below?

yes, with the patch you provided I can no longer trigger the infinite 
loop.

Tested-by: Vince Weaver <vincent.weaver@maine.edu>

[tip:perf/core] perf session: Avoid infinite loop when seeing invalid header.size

[tip-bot for Arnaldo Carvalho de Melo]

Commit-ID:  57fc032ad643ffd018d66bd4c1bd3a91de4841e8
Gitweb:     https://git.kernel.org/tip/57fc032ad643ffd018d66bd4c1bd3a91de4841e8
Author:     Arnaldo Carvalho de Melo <acme@redhat.com>
AuthorDate: Tue, 30 Jul 2019 10:58:41 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 12 Aug 2019 16:26:02 -0300

perf session: Avoid infinite loop when seeing invalid header.size

Vince reported that when fuzzing the userland perf tool with a bogus
perf.data file he got into a infinite loop in 'perf report'.

Changing the return of fetch_mmaped_event() to ERR_PTR(-EINVAL) for that
case gets us out of that infinite loop.

Reported-by: Vince Weaver <vincent.weaver@maine.edu>
Tested-by: Vince Weaver <vincent.weaver@maine.edu>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20190726211415.GE24867@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 11e6093c941b..b9fe71d11bf6 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <errno.h>
 #include <inttypes.h>
+#include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/zalloc.h>
 #include <traceevent/event-parse.h>
@@ -1955,7 +1956,9 @@ fetch_mmaped_event(struct perf_session *session,
 		/* We're not fetching the event so swap back again */
 		if (session->header.needs_swap)
 			perf_event_header__bswap(&event->header);
-		return NULL;
+		pr_debug("%s: head=%#" PRIx64 " event->header_size=%#x, mmap_size=%#zx: fuzzed perf.data?\n",
+			 __func__, head, event->header.size, mmap_size);
+		return ERR_PTR(-EINVAL);
 	}
 
 	return event;
@@ -1973,6 +1976,9 @@ static int __perf_session__process_decomp_events(struct perf_session *session)
 	while (decomp->head < decomp->size && !session_done()) {
 		union perf_event *event = fetch_mmaped_event(session, decomp->head, decomp->size, decomp->data);
 
+		if (IS_ERR(event))
+			return PTR_ERR(event);
+
 		if (!event)
 			break;
 
@@ -2072,6 +2078,9 @@ remap:
 
 more:
 	event = fetch_mmaped_event(session, head, mmap_size, buf);
+	if (IS_ERR(event))
+		return PTR_ERR(event);
+
 	if (!event) {
 		if (mmaps[map_idx]) {
 			munmap(mmaps[map_idx], mmap_size);