perf map: Changes to reference counting

When a pointer to a map exists do a get, when that pointer is
overwritten or freed, put the map. This avoids issues with gets and
puts being inconsistently used causing, use after puts, etc. For
example, the map in struct addr_location is changed to hold a
reference count. Reference count checking and address sanitizer were
used to identify issues.

Signed-off-by: Ian Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Dmitriy Vyukov <dvyukov@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: German Gomez <german.gomez@arm.com>
Cc: Hao Luo <haoluo@google.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Miaoqian Lin <linmq006@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Riccardo Mancini <rickyman7@gmail.com>
Cc: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
Cc: Song Liu <song@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Thomas Richter <tmricht@linux.ibm.com>
Cc: Yury Norov <yury.norov@gmail.com>
Link: https://lore.kernel.org/r/20230404205954.2245628-2-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 1545fcaa95c60da1affac84aff820be3c912f1bb..efe026a350100df072c95e0e5679cdf14366b60f 100644 (file)
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -366,6 +366,7 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
        }
        pr_debug("Bytes read match those read by objdump\n");
 out:
+       map__put(al.map);
        return err;
 }
 

diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index f00ec9abdbcd3157e0c7a42c618d6e8b8bb7002f..8c0e3f33474763e4cc3d0d9bca9fb62d4b0fb7cf 100644 (file)
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -112,6 +112,7 @@ static int add_hist_entries(struct hists *hists, struct machine *machine)
                }
 
                fake_samples[i].thread = al.thread;
+               map__put(fake_samples[i].map);
                fake_samples[i].map = al.map;
                fake_samples[i].sym = al.sym;
        }
@@ -147,6 +148,14 @@ static void del_hist_entries(struct hists *hists)
        }
 }
 
+static void put_fake_samples(void)
+{
+       size_t i;
+
+       for (i = 0; i < ARRAY_SIZE(fake_samples); i++)
+               map__put(fake_samples[i].map);
+}
+
 typedef int (*test_fn_t)(struct evsel *, struct machine *);
 
 #define COMM(he)  (thread__comm_str(he->thread))
@@ -733,6 +742,7 @@ out:
        /* tear down everything */
        evlist__delete(evlist);
        machines__exit(&machines);
+       put_fake_samples();
 
        return err;
 }

diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
index 7c552549f4a42b173b909dcb88612364d7e0ec42..98eff5935a1c7e0061128d038e525e3e84661af6 100644 (file)
--- a/tools/perf/tests/hists_filter.c
+++ b/tools/perf/tests/hists_filter.c
@@ -89,6 +89,7 @@ static int add_hist_entries(struct evlist *evlist,
                        }
 
                        fake_samples[i].thread = al.thread;
+                       map__put(fake_samples[i].map);
                        fake_samples[i].map = al.map;
                        fake_samples[i].sym = al.sym;
                }
@@ -101,6 +102,14 @@ out:
        return TEST_FAIL;
 }
 
+static void put_fake_samples(void)
+{
+       size_t i;
+
+       for (i = 0; i < ARRAY_SIZE(fake_samples); i++)
+               map__put(fake_samples[i].map);
+}
+
 static int test__hists_filter(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
 {
        int err = TEST_FAIL;
@@ -322,6 +331,7 @@ out:
        evlist__delete(evlist);
        reset_output_field();
        machines__exit(&machines);
+       put_fake_samples();
 
        return err;
 }

diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index e7e4ee57ce0414fb9e1cef870a9eeaa21d36d81d..64ce8097889c35f987a24321fed9fce4416518a4 100644 (file)
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -6,6 +6,7 @@
 #include "evsel.h"
 #include "evlist.h"
 #include "machine.h"
+#include "map.h"
 #include "parse-events.h"
 #include "hists_common.h"
 #include "util/mmap.h"
@@ -94,6 +95,7 @@ static int add_hist_entries(struct evlist *evlist, struct machine *machine)
                        }
 
                        fake_common_samples[k].thread = al.thread;
+                       map__put(fake_common_samples[k].map);
                        fake_common_samples[k].map = al.map;
                        fake_common_samples[k].sym = al.sym;
                }
@@ -126,11 +128,24 @@ out:
        return -1;
 }
 
+static void put_fake_samples(void)
+{
+       size_t i, j;
+
+       for (i = 0; i < ARRAY_SIZE(fake_common_samples); i++)
+               map__put(fake_common_samples[i].map);
+       for (i = 0; i < ARRAY_SIZE(fake_samples); i++) {
+               for (j = 0; j < ARRAY_SIZE(fake_samples[0]); j++)
+                       map__put(fake_samples[i][j].map);
+       }
+}
+
 static int find_sample(struct sample *samples, size_t nr_samples,
                       struct thread *t, struct map *m, struct symbol *s)
 {
        while (nr_samples--) {
-               if (samples->thread == t && samples->map == m &&
+               if (samples->thread == t &&
+                   samples->map == m &&
                    samples->sym == s)
                        return 1;
                samples++;
@@ -336,6 +351,7 @@ out:
        evlist__delete(evlist);
        reset_output_field();
        machines__exit(&machines);
+       put_fake_samples();
 
        return err;
 }

diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c
index 428d11a938f28306eb4bdcc9977d6cdc23312922..cebd5226bb12bbdfaf55156e0187001f30be50d8 100644 (file)
--- a/tools/perf/tests/hists_output.c
+++ b/tools/perf/tests/hists_output.c
@@ -78,6 +78,7 @@ static int add_hist_entries(struct hists *hists, struct machine *machine)
                }
 
                fake_samples[i].thread = al.thread;
+               map__put(fake_samples[i].map);
                fake_samples[i].map = al.map;
                fake_samples[i].sym = al.sym;
        }
@@ -113,6 +114,14 @@ static void del_hist_entries(struct hists *hists)
        }
 }
 
+static void put_fake_samples(void)
+{
+       size_t i;
+
+       for (i = 0; i < ARRAY_SIZE(fake_samples); i++)
+               map__put(fake_samples[i].map);
+}
+
 typedef int (*test_fn_t)(struct evsel *, struct machine *);
 
 #define COMM(he)  (thread__comm_str(he->thread))
@@ -620,6 +629,7 @@ out:
        /* tear down everything */
        evlist__delete(evlist);
        machines__exit(&machines);
+       put_fake_samples();
 
        return err;
 }

diff --git a/tools/perf/tests/mmap-thread-lookup.c b/tools/perf/tests/mmap-thread-lookup.c
index 5cc4644e353dc4e8a9f6a313f3e7b22078ae5250..898eda55b7a8169af91fc798fbfa636c44178060 100644 (file)
--- a/tools/perf/tests/mmap-thread-lookup.c
+++ b/tools/perf/tests/mmap-thread-lookup.c
@@ -203,6 +203,7 @@ static int mmap_events(synth_cb synth)
                }
 
                pr_debug("map %p, addr %" PRIx64 "\n", al.map, map__start(al.map));
+               map__put(al.map);
        }
 
        machine__delete_threads(machine);

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 8e7c29836765d164cf19674dfcfdad1e1266853a..b0dafc758173ada4fe61ad4584549919bcf01c61 100644 (file)
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -589,7 +589,7 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor)
                }
                call->ip = cursor_node->ip;
                call->ms = cursor_node->ms;
-               map__get(call->ms.map);
+               call->ms.map = map__get(call->ms.map);
                call->srcline = cursor_node->srcline;
 
                if (cursor_node->branch) {
@@ -1067,7 +1067,7 @@ int callchain_cursor_append(struct callchain_cursor *cursor,
        node->ip = ip;
        map__zput(node->ms.map);
        node->ms = *ms;
-       map__get(node->ms.map);
+       node->ms.map = map__get(node->ms.map);
        node->branch = branch;
        node->nr_loop_iter = nr_loop_iter;
        node->iter_cycles = iter_cycles;
@@ -1115,7 +1115,8 @@ int fill_callchain_info(struct addr_location *al, struct callchain_cursor_node *
        struct machine *machine = maps__machine(node->ms.maps);
 
        al->maps = node->ms.maps;
-       al->map = node->ms.map;
+       map__put(al->map);
+       al->map = map__get(node->ms.map);
        al->sym = node->ms.sym;
        al->srcline = node->srcline;
        al->addr = node->ip;
@@ -1528,7 +1529,7 @@ int callchain_node__make_parent_list(struct callchain_node *node)
                                goto out;
                        *new = *chain;
                        new->has_children = false;
-                       map__get(new->ms.map);
+                       new->ms.map = map__get(new->ms.map);
                        list_add_tail(&new->list, &head);
                }
                parent = parent->parent;

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 2712d1a8264e256ddf410f093cddafed80fe05f9..13f7f85e92e10a23f7345885dc3a65e65189b3ee 100644 (file)
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -485,13 +485,14 @@ size_t perf_event__fprintf_text_poke(union perf_event *event, struct machine *ma
        if (machine) {
                struct addr_location al;
 
-               al.map = maps__find(machine__kernel_maps(machine), tp->addr);
+               al.map = map__get(maps__find(machine__kernel_maps(machine), tp->addr));
                if (al.map && map__load(al.map) >= 0) {
                        al.addr = map__map_ip(al.map, tp->addr);
                        al.sym = map__find_symbol(al.map, al.addr);
                        if (al.sym)
                                ret += symbol__fprintf_symname_offs(al.sym, &al, fp);
                }
+               map__put(al.map);
        }
        ret += fprintf(fp, " old len %u new len %u\n", tp->old_len, tp->new_len);
        old = true;
@@ -614,7 +615,7 @@ struct map *thread__find_map(struct thread *thread, u8 cpumode, u64 addr,
                return NULL;
        }
 
-       al->map = maps__find(maps, al->addr);
+       al->map = map__get(maps__find(maps, al->addr));
        if (al->map != NULL) {
                /*
                 * Kernel maps might be changed when loading symbols so loading
@@ -773,6 +774,7 @@ int machine__resolve(struct machine *machine, struct addr_location *al,
  */
 void addr_location__put(struct addr_location *al)
 {
+       map__zput(al->map);
        thread__zput(al->thread);
 }
 

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index e494425cad060bf84fb6a7fb49a513e7f386a12a..51020da15ffcc5c9370390f9154468a35e0174d6 100644 (file)
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -450,7 +450,7 @@ static int hist_entry__init(struct hist_entry *he,
                        memset(&he->stat, 0, sizeof(he->stat));
        }
 
-       map__get(he->ms.map);
+       he->ms.map = map__get(he->ms.map);
 
        if (he->branch_info) {
                /*
@@ -465,13 +465,13 @@ static int hist_entry__init(struct hist_entry *he,
                memcpy(he->branch_info, template->branch_info,
                       sizeof(*he->branch_info));
 
-               map__get(he->branch_info->from.ms.map);
-               map__get(he->branch_info->to.ms.map);
+               he->branch_info->from.ms.map = map__get(he->branch_info->from.ms.map);
+               he->branch_info->to.ms.map = map__get(he->branch_info->to.ms.map);
        }
 
        if (he->mem_info) {
-               map__get(he->mem_info->iaddr.ms.map);
-               map__get(he->mem_info->daddr.ms.map);
+               he->mem_info->iaddr.ms.map = map__get(he->mem_info->iaddr.ms.map);
+               he->mem_info->daddr.ms.map = map__get(he->mem_info->daddr.ms.map);
        }
 
        if (hist_entry__has_callchains(he) && symbol_conf.use_callchain)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 1ea6f6c06bbb3c99fa2acbed083070b97750aaea..25738775834e9e1ef3a02a397344c30f23749ca2 100644 (file)
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -881,21 +881,29 @@ static int machine__process_ksymbol_register(struct machine *machine,
        struct symbol *sym;
        struct dso *dso;
        struct map *map = maps__find(machine__kernel_maps(machine), event->ksymbol.addr);
+       bool put_map = false;
+       int err = 0;
 
        if (!map) {
-               int err;
-
                dso = dso__new(event->ksymbol.name);
-               if (dso) {
-                       dso->kernel = DSO_SPACE__KERNEL;
-                       map = map__new2(0, dso);
-                       dso__put(dso);
-               }
 
-               if (!dso || !map) {
-                       return -ENOMEM;
+               if (!dso) {
+                       err = -ENOMEM;
+                       goto out;
                }
-
+               dso->kernel = DSO_SPACE__KERNEL;
+               map = map__new2(0, dso);
+               dso__put(dso);
+               if (!map) {
+                       err = -ENOMEM;
+                       goto out;
+               }
+               /*
+                * The inserted map has a get on it, we need to put to release
+                * the reference count here, but do it after all accesses are
+                * done.
+                */
+               put_map = true;
                if (event->ksymbol.ksym_type == PERF_RECORD_KSYMBOL_TYPE_OOL) {
                        dso->binary_type = DSO_BINARY_TYPE__OOL;
                        dso->data.file_size = event->ksymbol.len;
@@ -905,9 +913,10 @@ static int machine__process_ksymbol_register(struct machine *machine,
                map->start = event->ksymbol.addr;
                map->end = map__start(map) + event->ksymbol.len;
                err = maps__insert(machine__kernel_maps(machine), map);
-               map__put(map);
-               if (err)
-                       return err;
+               if (err) {
+                       err = -ENOMEM;
+                       goto out;
+               }
 
                dso__set_loaded(dso);
 
@@ -922,10 +931,15 @@ static int machine__process_ksymbol_register(struct machine *machine,
        sym = symbol__new(map__map_ip(map, map__start(map)),
                          event->ksymbol.len,
                          0, 0, event->ksymbol.name);
-       if (!sym)
-               return -ENOMEM;
+       if (!sym) {
+               err = -ENOMEM;
+               goto out;
+       }
        dso__insert_symbol(dso, sym);
-       return 0;
+out:
+       if (put_map)
+               map__put(map);
+       return err;
 }
 
 static int machine__process_ksymbol_unregister(struct machine *machine,
@@ -1027,13 +1041,11 @@ static struct map *machine__addnew_module_map(struct machine *machine, u64 start
                goto out;
 
        err = maps__insert(machine__kernel_maps(machine), map);
-
-       /* Put the map here because maps__insert already got it */
-       map__put(map);
-
        /* If maps__insert failed, return NULL. */
-       if (err)
+       if (err) {
+               map__put(map);
                map = NULL;
+       }
 out:
        /* put the dso here, corresponding to  machine__findnew_module_dso */
        dso__put(dso);
@@ -1325,6 +1337,7 @@ __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
        /* In case of renewal the kernel map, destroy previous one */
        machine__destroy_kernel_maps(machine);
 
+       map__put(machine->vmlinux_map);
        machine->vmlinux_map = map__new2(0, kernel);
        if (machine->vmlinux_map == NULL)
                return -ENOMEM;
@@ -1613,7 +1626,7 @@ static int machine__create_module(void *arg, const char *name, u64 start,
        map->end = start + size;
 
        dso__kernel_module_get_build_id(map__dso(map), machine->root_dir);
-
+       map__put(map);
        return 0;
 }
 
@@ -1659,16 +1672,18 @@ static void machine__set_kernel_mmap(struct machine *machine,
 static int machine__update_kernel_mmap(struct machine *machine,
                                     u64 start, u64 end)
 {
-       struct map *map = machine__kernel_map(machine);
+       struct map *orig, *updated;
        int err;
 
-       map__get(map);
-       maps__remove(machine__kernel_maps(machine), map);
+       orig = machine->vmlinux_map;
+       updated = map__get(orig);
 
+       machine->vmlinux_map = updated;
        machine__set_kernel_mmap(machine, start, end);
+       maps__remove(machine__kernel_maps(machine), orig);
+       err = maps__insert(machine__kernel_maps(machine), updated);
+       map__put(orig);
 
-       err = maps__insert(machine__kernel_maps(machine), map);
-       map__put(map);
        return err;
 }
 
@@ -2295,7 +2310,7 @@ static int add_callchain_ip(struct thread *thread,
 {
        struct map_symbol ms;
        struct addr_location al;
-       int nr_loop_iter = 0;
+       int nr_loop_iter = 0, err;
        u64 iter_cycles = 0;
        const char *srcline = NULL;
 
@@ -2360,9 +2375,11 @@ static int add_callchain_ip(struct thread *thread,
                return 0;
 
        srcline = callchain_srcline(&ms, al.addr);
-       return callchain_cursor_append(cursor, ip, &ms,
-                                      branch, flags, nr_loop_iter,
-                                      iter_cycles, branch_from, srcline);
+       err = callchain_cursor_append(cursor, ip, &ms,
+                                     branch, flags, nr_loop_iter,
+                                     iter_cycles, branch_from, srcline);
+       map__put(al.map);
+       return err;
 }
 
 struct branch_info *sample__resolve_bstack(struct perf_sample *sample,

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 6426af6882c207a55382b9fb3245faae8824fd01..d81b6ca18ee9aed08b08ccaf77bc4a0359407f9e 100644 (file)
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -410,7 +410,7 @@ struct map *map__clone(struct map *from)
        map = memdup(from, size);
        if (map != NULL) {
                refcount_set(&map->refcnt, 1);
-               dso__get(dso);
+               map->dso = dso__get(dso);
        }
 
        return map;