Messages in this thread | | | Date | Mon, 10 Jan 2022 15:51:07 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH] perf tui: avoid crash when annotating recursive functions |
| |
Em Mon, Jan 10, 2022 at 12:44:41AM +0100, Dario Petrillo escreveu: > In perf report, entering a recursive function from inside of itself > (either directly of indirectly through some other function) results in > calling symbol__annotate2 multiple times, and freeing the whole disassembly > when exiting from the innermost instance. The first issue causes the > function's disassembly to be duplicated, and the latter a heap > use-after-free (and crash) when trying to access the disassembly again. > > I reproduced the bug on perf 5.11.22 (Ubuntu 20.04.3 LTS) and 5.16.rc8 > with the following testcase (compile with gcc recursive.c -o recursive). > To reproduce: > - perf record ./recursive > - perf report > - enter fibonacci and annotate it > - move the cursor on one of the "callq fibonacci" instructions and press enter > - at this point there will be two copies of the function in the disassembly > - go back by pressing q, and perf will crash > > #include <stdio.h> > > int fibonacci(int n) > { > if(n <= 2) return 1; > return fibonacci(n-1) + fibonacci(n-2); > } > > int main() > { > printf("%d\n", fibonacci(40)); > } > > This patch addresses the issue by annotating a function and freeing the > associated memory on exit only if no annotation is already present, > so that a recursive function is only annotated on entry.
Problem reproduced, patch applied, problem fixed.
Thanks, applied.
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Signed-off-by: Dario Petrillo <dario.pk1@gmail.com> > > --- > tools/perf/ui/browsers/annotate.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c > index e81c2493efdf..44ba900828f6 100644 > --- a/tools/perf/ui/browsers/annotate.c > +++ b/tools/perf/ui/browsers/annotate.c > @@ -966,6 +966,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel, > .opts = opts, > }; > int ret = -1, err; > + int not_annotated = list_empty(¬es->src->source); > > if (sym == NULL) > return -1; > @@ -973,13 +974,15 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel, > if (ms->map->dso->annotate_warned) > return -1; > > - err = symbol__annotate2(ms, evsel, opts, &browser.arch); > - if (err) { > - char msg[BUFSIZ]; > - ms->map->dso->annotate_warned = true; > - symbol__strerror_disassemble(ms, err, msg, sizeof(msg)); > - ui__error("Couldn't annotate %s:\n%s", sym->name, msg); > - goto out_free_offsets; > + if (not_annotated) { > + err = symbol__annotate2(ms, evsel, opts, &browser.arch); > + if (err) { > + char msg[BUFSIZ]; > + ms->map->dso->annotate_warned = true; > + symbol__strerror_disassemble(ms, err, msg, sizeof(msg)); > + ui__error("Couldn't annotate %s:\n%s", sym->name, msg); > + goto out_free_offsets; > + } > } > > ui_helpline__push("Press ESC to exit"); > @@ -994,9 +997,11 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel, > > ret = annotate_browser__run(&browser, evsel, hbt); > > - annotated_source__purge(notes->src); > + if(not_annotated) > + annotated_source__purge(notes->src); > > out_free_offsets: > - zfree(¬es->offsets); > + if(not_annotated) > + zfree(¬es->offsets); > return ret; > } > -- > 2.25.1
--
- Arnaldo
| |