Messages in this thread | | | Date | Mon, 07 Oct 2013 09:48:31 -0600 | From | David Ahern <> | Subject | Re: [PATCH] perf tools: Fix perf_evlist__mmap_read event overflow |
| |
On 10/2/13 7:46 AM, Jiri Olsa wrote: > The perf_evlist__mmap_read used 'union perf_event' > as a placeholder for event crossing the mmap boundary. > > This is ok for sample shorter than ~PATH_MAX. However > we could grow up to the maximum sample size which is > 16 bits max. > > I hit this overflow issue when using 'perf top -G dwarf' > which produces sample with the size around 8192 bytes. > We could configure any valid sample size here using: > '-G dwarf,size'. > > Using array with sample max size instead for the event > placeholder. Also adding another safe check for the > dynamic size of the user stack. > > TODO: The 'struct perf_mmap' is quite big now, maybe we > could use some lazy allocation for event_copy size.
I was going to suggest just that -- dynamically handle the use of event_copy. The below adds ~60k per mmap. Otherwise...
Acked-by: David Ahern <dsahern@gmail.com>
> > Signed-off-by: Jiri Olsa <jolsa@redhat.com> > Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com> > Cc: David Ahern <dsahern@gmail.com> > Cc: Ingo Molnar <mingo@elte.hu> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > Cc: Frederic Weisbecker <fweisbec@gmail.com> > --- > tools/perf/util/event.h | 3 +++ > tools/perf/util/evlist.c | 4 ++-- > tools/perf/util/evlist.h | 2 +- > tools/perf/util/evsel.c | 3 +++ > 4 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h > index 9f50f88..2b8032a 100644 > --- a/tools/perf/util/event.h > +++ b/tools/perf/util/event.h > @@ -75,6 +75,9 @@ struct throttle_event { > PERF_SAMPLE_CPU | PERF_SAMPLE_PERIOD | \ > PERF_SAMPLE_IDENTIFIER) > > +/* perf sample has 16 bits size limit */ > +#define PERF_SAMPLE_MAX_SIZE (1 << 16) > + > struct sample_event { > struct perf_event_header header; > u64 array[]; > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index f0d71a9..cb9523f 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -540,7 +540,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx) > if ((old & md->mask) + size != ((old + size) & md->mask)) { > unsigned int offset = old; > unsigned int len = min(sizeof(*event), size), cpy; > - void *dst = &md->event_copy; > + void *dst = md->event_copy; > > do { > cpy = min(md->mask + 1 - (offset & md->mask), len); > @@ -550,7 +550,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx) > len -= cpy; > } while (len); > > - event = &md->event_copy; > + event = (union perf_event *) md->event_copy; > } > > old += size; > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h > index 871b55a..722618f 100644 > --- a/tools/perf/util/evlist.h > +++ b/tools/perf/util/evlist.h > @@ -21,7 +21,7 @@ struct perf_mmap { > void *base; > int mask; > unsigned int prev; > - union perf_event event_copy; > + char event_copy[PERF_SAMPLE_MAX_SIZE]; > }; > > struct perf_evlist { > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 0ce9feb..aa20ee2 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -1453,6 +1453,9 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event, > array = (void *)array + sz; > OVERFLOW_CHECK_u64(array); > data->user_stack.size = *array++; > + if (WARN_ONCE(data->user_stack.size > sz, > + "user stack dump failure\n")) > + return -EFAULT; > } > } > >
| |