* Re: perf: perf report stuck in an infinite loop
2019-07-26 21:14 ` Arnaldo Carvalho de Melo
@ 2019-07-29 15:03 ` Vince Weaver
2019-08-15 9:16 ` [tip:perf/core] perf session: Avoid infinite loop when seeing invalid header.size tip-bot for Arnaldo Carvalho de Melo
1 sibling, 0 replies; 4+ messages in thread
From: Vince Weaver @ 2019-07-29 15:03 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Jiri Olsa, Namhyung Kim
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>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip:perf/core] perf session: Avoid infinite loop when seeing invalid header.size
2019-07-26 21:14 ` Arnaldo Carvalho de Melo
2019-07-29 15:03 ` Vince Weaver
@ 2019-08-15 9:16 ` tip-bot for Arnaldo Carvalho de Melo
1 sibling, 0 replies; 4+ messages in thread
From: tip-bot for Arnaldo Carvalho de Melo @ 2019-08-15 9:16 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, acme, hpa, namhyung, mingo, jolsa, vincent.weaver,
tglx, peterz, alexander.shishkin
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);
^ permalink raw reply related [flat|nested] 4+ messages in thread