tools/perf/util/hist.c | 5 +++++ 1 file changed, 5 insertions(+)
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;
+ 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.1On 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
>
On Thu, Aug 15, 2024 at 03:22:12PM +0100, Matt Fleming 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.
Great analysis, thanks a lot, applied.
- Arnaldo
> 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;
> + 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
>
© 2016 - 2026 Red Hat, Inc.