Messages in this thread | | | Subject | Re: [PATCH v2] fix use-after-free in perf_sched__lat | From | "liwei (GF)" <> | Date | Thu, 4 Jul 2019 19:21:28 +0800 |
| |
Hi Arnaldo, I found this issue has not been fixed in mainline now, please take a glance at this.
On 2019/5/23 10:50, Namhyung Kim wrote: > On Wed, May 22, 2019 at 08:08:23AM -0300, Arnaldo Carvalho de Melo wrote: >> Em Wed, May 22, 2019 at 03:56:10PM +0900, Namhyung Kim escreveu: >>> On Wed, May 08, 2019 at 10:36:48PM +0800, Wei Li wrote: >>>> After thread is added to machine->threads[i].dead in >>>> __machine__remove_thread, the machine->threads[i].dead is freed >>>> when calling free(session) in perf_session__delete(). So it get a >>>> Segmentation fault when accessing it in thread__put(). >>>> >>>> In this patch, we delay the perf_session__delete until all threads >>>> have been deleted. >>>> >>>> This can be reproduced by following steps: >>>> ulimit -c unlimited >>>> export MALLOC_MMAP_THRESHOLD_=0 >>>> perf sched record sleep 10 >>>> perf sched latency --sort max >>>> Segmentation fault (core dumped) >>>> >>>> Signed-off-by: Zhipeng Xie <xiezhipeng1@huawei.com> >>>> Signed-off-by: Wei Li <liwei391@huawei.com> >>> >>> Acked-by: Namhyung Kim <namhyung@kernel.org> >> >> I'll try to analyse this one soon, but my first impression was that we >> should just grab reference counts when keeping a pointer to those >> threads instead of keeping _all_ threads alive when supposedly we could >> trow away unreferenced data structures. >> >> But this is just a first impression from just reading the patch >> description, probably I'm missing something. > > No, thread refcounting is fine. We already did it and threads with the > refcount will be accessed only. > > But the problem is the head of the list. After using the thread, the > refcount is gone and thread is removed from the list and destroyed. > However the head of list is in a struct machine which was freed with > session already. > > Thanks, > Namhyung > > >> >> Thanks for providing instructions on readily triggering the segfault. >> >> - Arnaldo > > . >
Thanks, Wei
| |