From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754596AbdDDPfW (ORCPT ); Tue, 4 Apr 2017 11:35:22 -0400 Received: from mail-wr0-f176.google.com ([209.85.128.176]:35153 "EHLO mail-wr0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753096AbdDDPfV (ORCPT ); Tue, 4 Apr 2017 11:35:21 -0400 MIME-Version: 1.0 In-Reply-To: <20170404151940.GD12903@kernel.org> References: <20170315021631.31980-1-changbin.du@intel.com> <20170327062255.27309-1-changbin.du@intel.com> <20170404151940.GD12903@kernel.org> From: Namhyung Kim Date: Wed, 5 Apr 2017 00:34:59 +0900 X-Google-Sender-Auth: cV1Re8-AyJHXOli7fqL5waXLMDI Message-ID: Subject: Re: [PATCH v2] perf: fix double free at function perf_hpp__reset_output_field To: Arnaldo Carvalho de Melo Cc: Jiri Olsa , changbin.du@intel.com, Peter Zijlstra , Ingo Molnar , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Arnaldo, On Wed, Apr 5, 2017 at 12:19 AM, Arnaldo Carvalho de Melo wrote: > Em Mon, Mar 27, 2017 at 02:22:55PM +0800, changbin.du@intel.com escreveu: >> From: Changbin Du >> >> Some perf_hpp_fmt both registered at field and sort list. For such >> instance, we only can free it when removed from the both lists. This >> function currently only used by self-test code, but still should fix >> it. > > Looks sane, applying, > > Jiri, Namhyung, please holler (or ack) if needed, Did you actually see the double free problem? AFAICS the old code removed a fmt from both list before free it. In the first loop, fmt that was linked to both output list and sort list will be remove. And the second loop frees fmt that was linked only to the sort list (IOW, it frees fmt that was not freed in the first loop). Thanks, Namhyung > > - Arnaldo > >> Signed-off-by: Changbin Du >> --- >> v2: removed redundant Signed-off. >> >> --- >> tools/perf/ui/hist.c | 25 +++++++++++++++---------- >> 1 file changed, 15 insertions(+), 10 deletions(-) >> >> diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c >> index 5d632dc..f94b301 100644 >> --- a/tools/perf/ui/hist.c >> +++ b/tools/perf/ui/hist.c >> @@ -609,20 +609,25 @@ static void fmt_free(struct perf_hpp_fmt *fmt) >> >> void perf_hpp__reset_output_field(struct perf_hpp_list *list) >> { >> - struct perf_hpp_fmt *fmt, *tmp; >> + struct perf_hpp_fmt *field_fmt, *sort_fmt, *tmp1, *tmp2; >> >> /* reset output fields */ >> - perf_hpp_list__for_each_format_safe(list, fmt, tmp) { >> - list_del_init(&fmt->list); >> - list_del_init(&fmt->sort_list); >> - fmt_free(fmt); >> + perf_hpp_list__for_each_format_safe(list, field_fmt, tmp1) { >> + list_del_init(&field_fmt->list); >> + /* reset sort keys */ >> + perf_hpp_list__for_each_sort_list_safe(list, sort_fmt, tmp2) { >> + if (field_fmt == sort_fmt) { >> + list_del_init(&field_fmt->sort_list); >> + break; >> + } >> + } >> + fmt_free(field_fmt); >> } >> >> - /* reset sort keys */ >> - perf_hpp_list__for_each_sort_list_safe(list, fmt, tmp) { >> - list_del_init(&fmt->list); >> - list_del_init(&fmt->sort_list); >> - fmt_free(fmt); >> + /* reset remaining sort keys */ >> + perf_hpp_list__for_each_sort_list_safe(list, sort_fmt, tmp1) { >> + list_del_init(&sort_fmt->sort_list); >> + fmt_free(sort_fmt); >> } >> } >> >> -- >> 2.7.4