Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1387091 > unrolled thread
| Started by | Wang Nan <wangnan0@huawei.com> |
|---|---|
| First post | 2016-04-26 04:40 +0200 |
| Last post | 2016-04-27 17:40 +0200 |
| Articles | 3 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 1/5] perf tools: Enforce ring buffer reading Wang Nan <wangnan0@huawei.com> - 2016-04-26 04:40 +0200
Re: [PATCH 1/5] perf tools: Enforce ring buffer reading Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-26 15:40 +0200
[tip:perf/core] perf evlist: Enforce ring buffer reading tip-bot for Wang Nan <tipbot@zytor.com> - 2016-04-27 17:40 +0200
| From | Wang Nan <wangnan0@huawei.com> |
|---|---|
| Date | 2016-04-26 04:40 +0200 |
| Subject | [PATCH 1/5] perf tools: Enforce ring buffer reading |
| Message-ID | <rs14R-8ie-1@gated-at.bofh.it> |
Don't read broken data after 'head' pointer.
Following commits will feed perf_evlist__mmap_read() with some 'head'
pointers not maintained by kernel. If 'head' pointer breaks an event,
we should avoid reading from the broken event. This can happen in
backward ring buffer.
For example:
old head
| |
V V
+---+------+----------+----+-----+--+
|..E|D....D|C........C|B..B|A....|E.|
+---+------+----------+----+-----+--+
'old' pointer points to the beginning of 'A' and trying read from it,
but 'A' has been overwritten. In this case, don't try to read from 'A',
simply return NULL.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
tools/perf/util/evlist.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 6fb5725..85271e5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -684,6 +684,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
struct perf_mmap *md = &evlist->mmap[idx];
u64 head;
u64 old = md->prev;
+ int diff;
unsigned char *data = md->base + page_size;
union perf_event *event = NULL;
@@ -694,6 +695,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
return NULL;
head = perf_mmap__read_head(md);
+ diff = head - old;
if (evlist->overwrite) {
/*
* If we're further behind than half the buffer, there's a chance
@@ -703,7 +705,6 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
*
* In either case, truncate and restart at head.
*/
- int diff = head - old;
if (diff > md->mask / 2 || diff < 0) {
fprintf(stderr, "WARNING: failed to keep up with mmap data.\n");
@@ -711,15 +712,21 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
* head points to a known good entry, start there.
*/
old = head;
+ diff = 0;
}
}
- if (old != head) {
+ if (diff >= (int)sizeof(event->header)) {
size_t size;
event = (union perf_event *)&data[old & md->mask];
size = event->header.size;
+ if (size < sizeof(event->header) || diff < (int)size) {
+ event = NULL;
+ goto broken_event;
+ }
+
/*
* Event straddles the mmap boundary -- header should always
* be inside due to u64 alignment of output.
@@ -743,6 +750,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
old += size;
}
+broken_event:
md->prev = old;
return event;
--
1.8.3.4
[toc] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-04-26 15:40 +0200 |
| Message-ID | <rsbnB-9L-29@gated-at.bofh.it> |
| In reply to | #1387091 |
Em Tue, Apr 26, 2016 at 02:28:54AM +0000, Wang Nan escreveu:
> Don't read broken data after 'head' pointer.
>
> Following commits will feed perf_evlist__mmap_read() with some 'head'
> pointers not maintained by kernel. If 'head' pointer breaks an event,
> we should avoid reading from the broken event. This can happen in
> backward ring buffer.
Looks good, applied.
- Arnaldo
> For example:
>
> old head
> | |
> V V
> +---+------+----------+----+-----+--+
> |..E|D....D|C........C|B..B|A....|E.|
> +---+------+----------+----+-----+--+
>
> 'old' pointer points to the beginning of 'A' and trying read from it,
> but 'A' has been overwritten. In this case, don't try to read from 'A',
> simply return NULL.
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Zefan Li <lizefan@huawei.com>
> Cc: pi3orama@163.com
> ---
> tools/perf/util/evlist.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 6fb5725..85271e5 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -684,6 +684,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
> struct perf_mmap *md = &evlist->mmap[idx];
> u64 head;
> u64 old = md->prev;
> + int diff;
> unsigned char *data = md->base + page_size;
> union perf_event *event = NULL;
>
> @@ -694,6 +695,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
> return NULL;
>
> head = perf_mmap__read_head(md);
> + diff = head - old;
> if (evlist->overwrite) {
> /*
> * If we're further behind than half the buffer, there's a chance
> @@ -703,7 +705,6 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
> *
> * In either case, truncate and restart at head.
> */
> - int diff = head - old;
> if (diff > md->mask / 2 || diff < 0) {
> fprintf(stderr, "WARNING: failed to keep up with mmap data.\n");
>
> @@ -711,15 +712,21 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
> * head points to a known good entry, start there.
> */
> old = head;
> + diff = 0;
> }
> }
>
> - if (old != head) {
> + if (diff >= (int)sizeof(event->header)) {
> size_t size;
>
> event = (union perf_event *)&data[old & md->mask];
> size = event->header.size;
>
> + if (size < sizeof(event->header) || diff < (int)size) {
> + event = NULL;
> + goto broken_event;
> + }
> +
> /*
> * Event straddles the mmap boundary -- header should always
> * be inside due to u64 alignment of output.
> @@ -743,6 +750,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
> old += size;
> }
>
> +broken_event:
> md->prev = old;
>
> return event;
> --
> 1.8.3.4
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Wang Nan <tipbot@zytor.com> |
|---|---|
| Date | 2016-04-27 17:40 +0200 |
| Subject | [tip:perf/core] perf evlist: Enforce ring buffer reading |
| Message-ID | <rszJi-3tA-73@gated-at.bofh.it> |
| In reply to | #1387091 |
Commit-ID: b04b7023751bf6519eee64467b6477f0e7fb82a1
Gitweb: http://git.kernel.org/tip/b04b7023751bf6519eee64467b6477f0e7fb82a1
Author: Wang Nan <wangnan0@huawei.com>
AuthorDate: Tue, 26 Apr 2016 02:28:54 +0000
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 26 Apr 2016 10:56:08 -0300
perf evlist: Enforce ring buffer reading
Don't read broken data after 'head' pointer.
Following commits will feed perf_evlist__mmap_read() with some 'head'
pointers not maintained by kernel. If 'head' pointer breaks an event, we
should avoid reading from the broken event. This can happen in backward
ring buffer.
For example:
old head
| |
V V
+---+------+----------+----+-----+--+
|..E|D....D|C........C|B..B|A....|E.|
+---+------+----------+----+-----+--+
'old' pointer points to the beginning of 'A' and trying read from it,
but 'A' has been overwritten. In this case, don't try to read from 'A',
simply return NULL.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1461637738-62722-2-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/evlist.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 6fb5725..85271e5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -684,6 +684,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
struct perf_mmap *md = &evlist->mmap[idx];
u64 head;
u64 old = md->prev;
+ int diff;
unsigned char *data = md->base + page_size;
union perf_event *event = NULL;
@@ -694,6 +695,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
return NULL;
head = perf_mmap__read_head(md);
+ diff = head - old;
if (evlist->overwrite) {
/*
* If we're further behind than half the buffer, there's a chance
@@ -703,7 +705,6 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
*
* In either case, truncate and restart at head.
*/
- int diff = head - old;
if (diff > md->mask / 2 || diff < 0) {
fprintf(stderr, "WARNING: failed to keep up with mmap data.\n");
@@ -711,15 +712,21 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
* head points to a known good entry, start there.
*/
old = head;
+ diff = 0;
}
}
- if (old != head) {
+ if (diff >= (int)sizeof(event->header)) {
size_t size;
event = (union perf_event *)&data[old & md->mask];
size = event->header.size;
+ if (size < sizeof(event->header) || diff < (int)size) {
+ event = NULL;
+ goto broken_event;
+ }
+
/*
* Event straddles the mmap boundary -- header should always
* be inside due to u64 alignment of output.
@@ -743,6 +750,7 @@ union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
old += size;
}
+broken_event:
md->prev = old;
return event;
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web