Messages in this thread | | | Date | Thu, 9 Aug 2018 09:39:45 +0200 | From | Jiri Olsa <> | Subject | Re: [PATCH] perf ordered_events: fix crash in free_dup_event() |
| |
On Wed, Aug 08, 2018 at 02:47:42PM -0700, Stephane Eranian wrote: > Hi, > > Ok, I found the problem. It still exists upstream , just very tricky to trigger. > Took me lots of time with gdb + watchpoints to track this down, where > in fact it was just in front of me. > > From the crashdump: > Program received signal SIGSEGV, Segmentation fault. > free_dup_event (oe=0x26a39a0, event=0xffffffff00000000) > > I was puzzled by the 0xffffffff00000000. I tracked down where this > value was coming from using watchpoints. > In my case the memory was used before by elfutils to back the struct > Elf. The -1 in the upper bits came from: > > file_read_elf () at third_party/elfutils/libelf/elf_begin.c:451 > elf->state.elf64.scns.data[cnt].shndx_index = -1; > > And yet the next access to that memory location was in the crash. That > meant the memory was released by > elfutils and reused by ordered_events, yet without any initialization. > But looking at alloc_event(), it was > not obvious to figure out how a new_event->event could be uninitialized. > > Well, it turns out there is this little hack where the code > commandeers the first element in the oe->buffer to > use as a list_head for the oe->to_free freelist. The problem is that > this entry also gets freed, but its > event->event field is NEVER initialized. So depending on how the > memory was previously used, you > could get a on NULL value and crash in free_dup_event(). This is what > happened to me. I am glad > I pursued this further because the bug is still in the upstream > version. The patch is a one-liner fixing > the initialization of the event->event = NULL. For the other elements > in the list, the initialization is > already done at the end of alloc_event(). > > I will send the patch separately.
nice ;-) thanks
jirka
| |