lkml.org 
[lkml]   [2015]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] perf trace: Fix segmentfault on perf trace
Em Mon, May 11, 2015 at 08:11:14PM +0800, He Kuang escreveu:
> Hi, Arnaldo
>
> On 2015/4/8 11:15, He Kuang wrote:
> >Hi, Arnaldo
> >On 2015/4/7 20:36, Arnaldo Carvalho de Melo wrote:
> >>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...
> >
> >In function __perf_evlist__mmap(), refcnt is initialized to 2, see commit:
> > 823969860329 ("perf evlist: Refcount mmaps")
> >
> >After filter_pollfd, perf_mmap->refcnt is 1 not 0.
> >
> > perf_evlist__filter_pollfd() -- refcnt=1
> > draining = true
> > if (perf_evlist__mmap_read() != NULL)
> > perf_evlist__mmap_consume() -- unmap, refcnt = 0
> > perf_evlist__mmap_read() -- segfault
> >else
> >exit
> >
> >I noticed that this issue also exists in builtin-record.c, but it
> >checks before mmap_read():
> >
> >if (rec->evlist->mmap[i].base) {
> > if (record__mmap_read(rec, i, draining) != 0) {
> >
> >So we can either do the check outside
> >builtin-trace.c:perf_evlist__mmap_read() like what
> >builtin-record.c do or inside. What's your opinion?
>
> I found the issue is still there, so ping...

Right, I noticed it as well sometimes, will apply the bandaid and leave
properly researching it for later.

- Arnaldo


\
 
 \ /
  Last update: 2015-05-11 16:01    [W:0.052 / U:1.184 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site