Messages in this thread | | | Date | Tue, 7 Aug 2018 09:20:29 +0200 | From | Jiri Olsa <> | Subject | Re: [PATCH] perf ordered_events: fix crash in free_dup_event() |
| |
On Mon, Aug 06, 2018 at 06:23:35PM -0700, Stephane Eranian wrote: > Depending on memory allocations, it was possible to get a SEGFAULT in > free_dup_event() because the event pointer was bogus: > > perf[1354]: segfault at ffffffff00000006 ip 00000000004b7fc7
is there any reproducer?
> > Initially, I thought it was some double free. But it turns out > it looked more like a buffer overrun. Adding padding before > the union perf_event field in struct ordered_event avoided the > problem. But it was not obvious where this could be coming from > given the accesses to the struct, i.e., no internal array. > > Then, it struck me that the following was bogus in __dup_event(): > > __dup_event(struct ordered_events *oe, union perf_event *event) > { > ... > union perf_event *new_event; > new_event = memdup(event, event->header.size); > ... > } > > The problem here is that header.size <= sizeof(*new_event). The code > was trying to copy only what was necessary, but then, the allocation
hum, and I think we should continue to do so.. we can't allocate ~4000 bytes space for 30 bytes sample
> was also only partial. In other words if the event was not the largest > possible for the union, it would not be fully backed by memory, likely > causing troubles.
how? using that event allocated space for another type of event?
> > This patch fixes the problem by passing the size of the union and not > that of the actual event.
I think that's just bypassing the real issue, please share more details on how to reproduce this
thanks, jirka
> > Signed-off-by: Stephane Eranian <eranian@google.com> > > --- > tools/perf/util/ordered-events.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c > index bad9e0296e9a..a90dbe5df019 100644 > --- a/tools/perf/util/ordered-events.c > +++ b/tools/perf/util/ordered-events.c > @@ -66,9 +66,9 @@ static union perf_event *__dup_event(struct ordered_events *oe, > union perf_event *new_event = NULL; > > if (oe->cur_alloc_size < oe->max_alloc_size) { > - new_event = memdup(event, event->header.size); > + new_event = memdup(event, sizeof(*event)); > if (new_event) > - oe->cur_alloc_size += event->header.size; > + oe->cur_alloc_size += sizeof(*event); > } > > return new_event; > -- > 2.18.0.597.ga71716f1ad-goog >
| |