Messages in this thread | | | Date | Tue, 4 Apr 2017 12:51:03 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v2] perf: fix double free at function perf_hpp__reset_output_field |
| |
Em Wed, Apr 05, 2017 at 12:34:59AM +0900, Namhyung Kim escreveu: > Hi Arnaldo, > > On Wed, Apr 5, 2017 at 12:19 AM, Arnaldo Carvalho de Melo > <acme@kernel.org> wrote: > > Em Mon, Mar 27, 2017 at 02:22:55PM +0800, changbin.du@intel.com escreveu: > >> From: Changbin Du <changbin.du@intel.com> > >> > >> 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
I assumed that he had seen it, in some self-test code, Changbin, can you please show command output or further describe when this patch would be necessary?
- Arnaldo
> 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 <changbin.du@intel.com> > >> --- > >> 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
| |