Em Tue, Apr 07, 2015 at 05:31:11PM +0800, He Kuang escreveu:
> After perf_evlist__filter_pollfd() filters out fds and releases
> perf_mmap by using perf_evlist__mmap_put(), refcnt of perf_mmap hits 1
> then perf_evlist__mmap_consume() will do the final unmap. In this
> condition, perf_evlist__mmap_read() will crash by referencing invalid
> mmap. Put refcnt check before use.
>
> Can be reproduced as following:
After applying 1/2 in this series and trying to reproduce I couldn't, it
works, looking at the code...
Let me get my head around this, idea was that after all fds associated
with a mmap would be closed, i.e. the perf_mmap->refcnt hits zero, then
we would have to drain whatever was left in the mmap, but looking again
that doesn't look like that is what is doing, becaue in filter_pollfd we
will munmap it before being able to "drain" it, as all mmaps were
closed, thus filter_pollfd returned zero...
Reading on, thanks for the patch!
- Arnaldo
> $ perf trace --duration 1.0 ls
> ...
> perf: Segmentation fault
> Obtained 14 stack frames.
> ./perf(dump_stack+0x2e) [0x503c2d]
> ./perf(sighandler_dump_stack+0x2e)
> [0x503d0c]
> /lib64/libc.so.6(+0x34df0) [0x7f5fd9a4adf0]
> ./perf() [0x4a8fda]
> ./perf(perf_evlist__mmap_read+0x56)
> [0x4aae93]
> ./perf() [0x470b28]
> ./perf(cmd_trace+0xada) [0x4727bd]
> ./perf() [0x49c4f4]
> ./perf() [0x49c74d]
> ./perf() [0x49c899]
> ./perf(main+0x23b)
> [0x49cbfa]
> /lib64/libc.so.6(__libc_start_main+0xf5)
> [0x7f5fd9a377b5]
> ./perf() [0x434ea5]
> [(nil)]
>
> Signed-off-by: He Kuang <heku...@huawei.com>
> ---
> tools/perf/util/evlist.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 76ef7ee..9d36433 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -634,11 +634,18 @@ static struct perf_evsel
> *perf_evlist__event2evsel(struct perf_evlist *evlist,
> union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
> {
> struct perf_mmap *md = &evlist->mmap[idx];
> - unsigned int head = perf_mmap__read_head(md);
> - unsigned int old = md->prev;
> - unsigned char *data = md->base + page_size;
> + unsigned int head;
> + unsigned int old;
> + unsigned char *data;
> union perf_event *event = NULL;
>
> + if (md == NULL || md->refcnt == 0)
> + return NULL;
> +
> + head = perf_mmap__read_head(md);
> + old = md->prev;
> + data = md->base + page_size;
> +
> if (evlist->overwrite) {
> /*
> * If we're further behind than half the buffer, there's a
> chance
> --
> 2.3.3.220.g9ab698f
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
