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
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);
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 <[email protected]>
Commit-ID: 57fc032ad643ffd018d66bd4c1bd3a91de4841e8
Gitweb: https://git.kernel.org/tip/57fc032ad643ffd018d66bd4c1bd3a91de4841e8
Author: Arnaldo Carvalho de Melo <[email protected]>
AuthorDate: Tue, 30 Jul 2019 10:58:41 -0300
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Tested-by: Vince Weaver <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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);