Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1387483
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 1/5] perf tools: Enforce ring buffer reading |
| Date | 2016-04-26 15:40 +0200 |
| Message-ID | <rsbnB-9L-29@gated-at.bofh.it> (permalink) |
| References | <rs14R-8ie-3@gated-at.bofh.it> <rs14R-8ie-1@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
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
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[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
csiph-web