On Thu, Aug 15, 2024 at 7:22 AM Matt Fleming matt@readmodwrite.com wrote:

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 #1 0x5650d8043744 in hist_entry__cmp util/hist.c:1286 #2 0x5650d8043951 in hists__findnew_entry util/hist.c:614 #3 0x5650d804568f in __hists__add_entry util/hist.c:754 #4 0x5650d8045bf9 in hists__add_entry util/hist.c:772 #5 0x5650d8045df1 in iter_add_single_normal_entry util/hist.c:997 #6 0x5650d8043326 in hist_entry_iter__add util/hist.c:1242 #7 0x5650d7ceeefe in perf_event__process_sample /home/matt/src/linux/tools/perf/builtin-top.c:845 #8 0x5650d7ceeefe in deliver_event /home/matt/src/linux/tools/perf/builtin-top.c:1208 #9 0x5650d7fdb51b in do_flush util/ordered-events.c:245 #10 0x5650d7fdb51b in __ordered_events__flush util/ordered-events.c:324 #11 0x5650d7ced743 in process_thread /home/matt/src/linux/tools/perf/builtin-top.c:1120 #12 0x7f757ef1f133 in start_thread nptl/pthread_create.c:442 #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 5c24b67aae72 ("perf tools: Replace map->referenced & maps->removed_maps with map->refcnt"), the symbol objects were leaked until c087e9480cf3 ("perf machine: Fix refcount usage when processing PERF_RECORD_KSYMBOL") was merged so the bug was masked.

Fixes: c087e9480cf3 ("perf machine: Fix refcount usage when processing PERF_RECORD_KSYMBOL") Signed-off-by: Matt Fleming (Cloudflare) matt@readmodwrite.com Reported-by: Yunzhao Li yunzhao@cloudflare.com Cc: stable@vger.kernel.org # v5.13+

---

tools/perf/util/hist.c | 5 +++++ 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 0f554febf9a1..0f9ce2ee2c31 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -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;
  

nit: we normally put a newline between a variable and the first non-variable line of code.

Acked-by: Ian Rogers irogers@google.com

Thanks, Ian

•                                   he->ms.sym = map__find_symbol(entry->ms.map, addr);
  

•                           }
  

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

-- 2.34.1