* [PATCH] perf tools: Fix perf_evlist__mmap_read event overflow
@ 2013-10-02 13:46 Jiri Olsa
2013-10-07 15:48 ` David Ahern
2013-10-15 5:29 ` [tip:perf/core] perf evlist: " tip-bot for Jiri Olsa
0 siblings, 2 replies; 3+ messages in thread
From: Jiri Olsa @ 2013-10-02 13:46 UTC (permalink / raw)
To: linux-kernel
Cc: Jiri Olsa, Corey Ashford, David Ahern, Ingo Molnar, Namhyung Kim,
Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo,
Frederic Weisbecker
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.
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;
}
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] perf tools: Fix perf_evlist__mmap_read event overflow
2013-10-02 13:46 [PATCH] perf tools: Fix perf_evlist__mmap_read event overflow Jiri Olsa
@ 2013-10-07 15:48 ` David Ahern
2013-10-15 5:29 ` [tip:perf/core] perf evlist: " tip-bot for Jiri Olsa
1 sibling, 0 replies; 3+ messages in thread
From: David Ahern @ 2013-10-07 15:48 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Corey Ashford, Ingo Molnar, Namhyung Kim,
Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo,
Frederic Weisbecker
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;
> }
> }
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [tip:perf/core] perf evlist: Fix perf_evlist__mmap_read event overflow
2013-10-02 13:46 [PATCH] perf tools: Fix perf_evlist__mmap_read event overflow Jiri Olsa
2013-10-07 15:48 ` David Ahern
@ 2013-10-15 5:29 ` tip-bot for Jiri Olsa
1 sibling, 0 replies; 3+ messages in thread
From: tip-bot for Jiri Olsa @ 2013-10-15 5:29 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, namhyung,
jolsa, fweisbec, dsahern, tglx, cjashfor, mingo
Commit-ID: a65cb4b9f8a777a715371c63c0525408048cea3e
Gitweb: http://git.kernel.org/tip/a65cb4b9f8a777a715371c63c0525408048cea3e
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Wed, 2 Oct 2013 15:46:39 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 11 Oct 2013 12:17:40 -0300
perf evlist: Fix perf_evlist__mmap_read event overflow
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.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: David Ahern <dsahern@gmail.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@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>
Link: http://lkml.kernel.org/r/1380721599-24285-1-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.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 9b7d4d3..752709c 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 abe69af..bfebc1e 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1456,6 +1456,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;
}
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-10-15 5:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-02 13:46 [PATCH] perf tools: Fix perf_evlist__mmap_read event overflow Jiri Olsa
2013-10-07 15:48 ` David Ahern
2013-10-15 5:29 ` [tip:perf/core] perf evlist: " tip-bot for Jiri Olsa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.