Re: perf segfault in in ordered_events__free()

2019-01-17 Thread Jiri Olsa
On Wed, Jan 16, 2019 at 11:33:55PM -0800, Song Liu wrote:
> On Wed, Jan 16, 2019 at 1:53 PM Song Liu  wrote:
> >
> > Thanks Jiri!
> >
> > On Wed, Jan 16, 2019 at 1:34 PM Jiri Olsa  wrote:
> > >
> > > On Wed, Jan 16, 2019 at 10:57:49AM -0800, Song Liu wrote:
> > > > Hi,
> > > >
> > > > We are debugging a segfault of perf in ordered_events__free().
> > >
> > > hi,
> > > any backtrace or info on how to reproduce it?
> >
> > Here is the backtrace:
> >
> > #0  0x00500055 in ordered_events(float, long double,...)(...) ()
> > #1  0x00500196 in ordered_events.reinit ()
> > #2  0x004fe413 in perf_session.process_events ()
> > #3  0x00440431 in cmd_record ()
> > #4  0x004a439f in run_builtin ()
> > #5  0x0042b3e5 in main ()"
> >
> > >
> > > > Disassemble shows the segfault was caused by oe->buff == NULL
> > > > in the following line:
> > > >
> > > > /*
> > > >  * Current buffer might not have all the events allocated
> > > >  * yet, we need to free only allocated ones ...
> > > >  */
> > > > list_del(>buffer->list);
> > > >
> > > > After poking around the code, I suspect it is caused by the following
> > > > condition in alloc_event():
> > > >
> > > > } else if (oe->buffer) {
> > > > new = >buffer->event[oe->buffer_idx];
> > > > if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
> > > > oe->buffer = NULL;
> > >
> > >
> > > argh.. yea, we need to check oe->buffer in ordered_events__free
> > >
> > > would attached change fix it for you?
> >
> > Let me try roll a fixed version to confirm.
> 
> Yes, the patch fixes this segfault. Please CC me on the official patch to
> back port the official version.

thanks for testing, will post it today

jirka


Re: perf segfault in in ordered_events__free()

2019-01-16 Thread Song Liu
On Wed, Jan 16, 2019 at 1:53 PM Song Liu  wrote:
>
> Thanks Jiri!
>
> On Wed, Jan 16, 2019 at 1:34 PM Jiri Olsa  wrote:
> >
> > On Wed, Jan 16, 2019 at 10:57:49AM -0800, Song Liu wrote:
> > > Hi,
> > >
> > > We are debugging a segfault of perf in ordered_events__free().
> >
> > hi,
> > any backtrace or info on how to reproduce it?
>
> Here is the backtrace:
>
> #0  0x00500055 in ordered_events(float, long double,...)(...) ()
> #1  0x00500196 in ordered_events.reinit ()
> #2  0x004fe413 in perf_session.process_events ()
> #3  0x00440431 in cmd_record ()
> #4  0x004a439f in run_builtin ()
> #5  0x0042b3e5 in main ()"
>
> >
> > > Disassemble shows the segfault was caused by oe->buff == NULL
> > > in the following line:
> > >
> > > /*
> > >  * Current buffer might not have all the events allocated
> > >  * yet, we need to free only allocated ones ...
> > >  */
> > > list_del(>buffer->list);
> > >
> > > After poking around the code, I suspect it is caused by the following
> > > condition in alloc_event():
> > >
> > > } else if (oe->buffer) {
> > > new = >buffer->event[oe->buffer_idx];
> > > if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
> > > oe->buffer = NULL;
> >
> >
> > argh.. yea, we need to check oe->buffer in ordered_events__free
> >
> > would attached change fix it for you?
>
> Let me try roll a fixed version to confirm.

Yes, the patch fixes this segfault. Please CC me on the official patch to
back port the official version.

Thanks,
Song


Re: perf segfault in in ordered_events__free()

2019-01-16 Thread Song Liu
Thanks Jiri!

On Wed, Jan 16, 2019 at 1:34 PM Jiri Olsa  wrote:
>
> On Wed, Jan 16, 2019 at 10:57:49AM -0800, Song Liu wrote:
> > Hi,
> >
> > We are debugging a segfault of perf in ordered_events__free().
>
> hi,
> any backtrace or info on how to reproduce it?

Here is the backtrace:

#0  0x00500055 in ordered_events(float, long double,...)(...) ()
#1  0x00500196 in ordered_events.reinit ()
#2  0x004fe413 in perf_session.process_events ()
#3  0x00440431 in cmd_record ()
#4  0x004a439f in run_builtin ()
#5  0x0042b3e5 in main ()"

>
> > Disassemble shows the segfault was caused by oe->buff == NULL
> > in the following line:
> >
> > /*
> >  * Current buffer might not have all the events allocated
> >  * yet, we need to free only allocated ones ...
> >  */
> > list_del(>buffer->list);
> >
> > After poking around the code, I suspect it is caused by the following
> > condition in alloc_event():
> >
> > } else if (oe->buffer) {
> > new = >buffer->event[oe->buffer_idx];
> > if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
> > oe->buffer = NULL;
>
>
> argh.. yea, we need to check oe->buffer in ordered_events__free
>
> would attached change fix it for you?

Let me try roll a fixed version to confirm.

Thanks again!
Song

>
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/util/ordered-events.c 
> b/tools/perf/util/ordered-events.c
> index 897589507d97..ea523d3b248f 100644
> --- a/tools/perf/util/ordered-events.c
> +++ b/tools/perf/util/ordered-events.c
> @@ -391,8 +391,10 @@ void ordered_events__free(struct ordered_events *oe)
>  * Current buffer might not have all the events allocated
>  * yet, we need to free only allocated ones ...
>  */
> -   list_del(>buffer->list);
> -   ordered_events_buffer__free(oe->buffer, oe->buffer_idx, oe);
> +   if (oe->buffer) {
> +   list_del(>buffer->list);
> +   ordered_events_buffer__free(oe->buffer, oe->buffer_idx, oe);
> +   }
>
> /* ... and continue with the rest */
> list_for_each_entry_safe(buffer, tmp, >to_free, list) {


Re: perf segfault in in ordered_events__free()

2019-01-16 Thread Jiri Olsa
On Wed, Jan 16, 2019 at 10:57:49AM -0800, Song Liu wrote:
> Hi,
> 
> We are debugging a segfault of perf in ordered_events__free().

hi,
any backtrace or info on how to reproduce it?

> Disassemble shows the segfault was caused by oe->buff == NULL
> in the following line:
> 
> /*
>  * Current buffer might not have all the events allocated
>  * yet, we need to free only allocated ones ...
>  */
> list_del(>buffer->list);
> 
> After poking around the code, I suspect it is caused by the following
> condition in alloc_event():
> 
> } else if (oe->buffer) {
> new = >buffer->event[oe->buffer_idx];
> if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
> oe->buffer = NULL;


argh.. yea, we need to check oe->buffer in ordered_events__free

would attached change fix it for you?

thanks,
jirka


---
diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
index 897589507d97..ea523d3b248f 100644
--- a/tools/perf/util/ordered-events.c
+++ b/tools/perf/util/ordered-events.c
@@ -391,8 +391,10 @@ void ordered_events__free(struct ordered_events *oe)
 * Current buffer might not have all the events allocated
 * yet, we need to free only allocated ones ...
 */
-   list_del(>buffer->list);
-   ordered_events_buffer__free(oe->buffer, oe->buffer_idx, oe);
+   if (oe->buffer) {
+   list_del(>buffer->list);
+   ordered_events_buffer__free(oe->buffer, oe->buffer_idx, oe);
+   }
 
/* ... and continue with the rest */
list_for_each_entry_safe(buffer, tmp, >to_free, list) {


perf segfault in in ordered_events__free()

2019-01-16 Thread Song Liu
Hi,

We are debugging a segfault of perf in ordered_events__free().
Disassemble shows the segfault was caused by oe->buff == NULL
in the following line:

/*
 * Current buffer might not have all the events allocated
 * yet, we need to free only allocated ones ...
 */
list_del(>buffer->list);

After poking around the code, I suspect it is caused by the following
condition in alloc_event():

} else if (oe->buffer) {
new = >buffer->event[oe->buffer_idx];
if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
oe->buffer = NULL;

Does this theory make sense? If so, what would be the best fix?

Thanks in advance!
Song