Skip to content

Commit

Permalink
perf hist: Update hist symbol when updating maps
Browse files Browse the repository at this point in the history
AddressSanitizer found a use-after-free bug in the symbol code which
manifested as 'perf top' segfaulting.

  ==1238389==ERROR: AddressSanitizer: heap-use-after-free on address 0x60b00c48844b at pc 0x5650d8035961 bp 0x7f751aaecc90 sp 0x7f751aaecc80
  READ of size 1 at 0x60b00c48844b thread T193
      #0 0x5650d8035960 in _sort__sym_cmp util/sort.c:310
      frank-w#1 0x5650d8043744 in hist_entry__cmp util/hist.c:1286
      frank-w#2 0x5650d8043951 in hists__findnew_entry util/hist.c:614
      frank-w#3 0x5650d804568f in __hists__add_entry util/hist.c:754
      frank-w#4 0x5650d8045bf9 in hists__add_entry util/hist.c:772
      frank-w#5 0x5650d8045df1 in iter_add_single_normal_entry util/hist.c:997
      frank-w#6 0x5650d8043326 in hist_entry_iter__add util/hist.c:1242
      frank-w#7 0x5650d7ceeefe in perf_event__process_sample /home/matt/src/linux/tools/perf/builtin-top.c:845
      frank-w#8 0x5650d7ceeefe in deliver_event /home/matt/src/linux/tools/perf/builtin-top.c:1208
      frank-w#9 0x5650d7fdb51b in do_flush util/ordered-events.c:245
      frank-w#10 0x5650d7fdb51b in __ordered_events__flush util/ordered-events.c:324
      frank-w#11 0x5650d7ced743 in process_thread /home/matt/src/linux/tools/perf/builtin-top.c:1120
      frank-w#12 0x7f757ef1f133 in start_thread nptl/pthread_create.c:442
      frank-w#13 0x7f757ef9f7db in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81

When updating hist maps it's also necessary to update the hist symbol
reference because the old one gets freed in map__put().

While this bug was probably introduced with 5c24b67 ("perf
tools: Replace map->referenced & maps->removed_maps with map->refcnt"),
the symbol objects were leaked until c087e94 ("perf machine:
Fix refcount usage when processing PERF_RECORD_KSYMBOL") was merged so
the bug was masked.

Fixes: c087e94 ("perf machine: Fix refcount usage when processing PERF_RECORD_KSYMBOL")
Reported-by: Yunzhao Li <[email protected]>
Signed-off-by: Matt Fleming (Cloudflare) <[email protected]>
Cc: Ian Rogers <[email protected]>
Cc: [email protected]
Cc: Namhyung Kim <[email protected]>
Cc: Riccardo Mancini <[email protected]>
Cc: [email protected] # v5.13+
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
  • Loading branch information
mfleming authored and acmel committed Aug 15, 2024
1 parent 27ac597 commit ac01c8c
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions tools/perf/util/hist.c
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,11 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
* the history counter to increment.
*/
if (he->ms.map != entry->ms.map) {
if (he->ms.sym) {
u64 addr = he->ms.sym->start;
he->ms.sym = map__find_symbol(entry->ms.map, addr);
}

map__put(he->ms.map);
he->ms.map = map__get(entry->ms.map);
}
Expand Down

0 comments on commit ac01c8c

Please sign in to comment.