lkml.org 
[lkml]   [2013]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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;
> }
> }
>
>



\
 
 \ /
  Last update: 2013-10-07 18:01    [W:0.061 / U:0.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site