perf maps: Get map before returning in maps__find_by_name

Finding a map is done under a lock, returning the map without a
reference count means it can be removed without notice and causing
uses after free. Grab a reference count to the map within the lock
region and return this. Fix up locations that need a map__put
following this. Also fix some reference counted pointer comparisons.

Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: James Clark <james.clark@arm.com>
Cc: Vincent Whitchurch <vincent.whitchurch@axis.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Changbin Du <changbin.du@huawei.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Song Liu <song@kernel.org>
Cc: Leo Yan <leo.yan@linux.dev>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Liam Howlett <liam.howlett@oracle.com>
Cc: Artem Savkov <asavkov@redhat.com>
Cc: bpf@vger.kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: https://lore.kernel.org/r/20240210031746.4057262-4-irogers@google.com

diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
index e808e6f..fecbf85 100644 (file)
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -131,9 +131,10 @@ static int test__vmlinux_matches_kallsyms_cb1(struct map *map, void *data)
        struct map *pair = maps__find_by_name(args->kallsyms.kmaps,
                                        (dso->kernel ? dso->short_name : dso->name));
 
-       if (pair)
+       if (pair) {
                map__set_priv(pair, 1);
-       else {
+               map__put(pair);
+       } else {
                if (!args->header_printed) {
                        pr_info("WARN: Maps only in vmlinux:\n");
                        args->header_printed = true;

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e8eb9f0..7031f6f 100644 (file)
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1537,8 +1537,10 @@ static int maps__set_module_path(struct maps *maps, const char *path, struct kmo
                return 0;
 
        long_name = strdup(path);
-       if (long_name == NULL)
+       if (long_name == NULL) {
+               map__put(map);
                return -ENOMEM;
+       }
 
        dso = map__dso(map);
        dso__set_long_name(dso, long_name, true);
@@ -1552,7 +1554,7 @@ static int maps__set_module_path(struct maps *maps, const char *path, struct kmo
                dso->symtab_type++;
                dso->comp = m->comp;
        }
-
+       map__put(map);
        return 0;
 }
 

diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index 2547c90..ea8fa68 100644 (file)
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -905,7 +905,7 @@ struct map *maps__find_by_name(struct maps *maps, const char *name)
                        struct dso *dso = map__dso(maps__maps_by_name(maps)[i]);
 
                        if (dso && strcmp(dso->short_name, name) == 0) {
-                               result = maps__maps_by_name(maps)[i]; // TODO: map__get
+                               result = map__get(maps__maps_by_name(maps)[i]);
                                done = true;
                        }
                }
@@ -917,7 +917,7 @@ struct map *maps__find_by_name(struct maps *maps, const char *name)
                                        sizeof(*mapp), map__strcmp_name);
 
                        if (mapp) {
-                               result = *mapp; // TODO: map__get
+                               result = map__get(*mapp);
                                i = mapp - maps__maps_by_name(maps);
                                RC_CHK_ACCESS(maps)->last_search_by_name_idx = i;
                        }
@@ -942,7 +942,7 @@ struct map *maps__find_by_name(struct maps *maps, const char *name)
                                        struct dso *dso = map__dso(pos);
 
                                        if (dso && strcmp(dso->short_name, name) == 0) {
-                                               result = pos; // TODO: map__get
+                                               result = map__get(pos);
                                                break;
                                        }
                                }

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a1a7960..be71abe 100644 (file)
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -358,6 +358,7 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)
                map = maps__find_by_name(machine__kernel_maps(host_machine), module_name);
                if (map) {
                        dso = map__dso(map);
+                       map__put(map);
                        goto found;
                }
                pr_debug("Failed to find module %s.\n", module);

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 95773c3..0b91f81 100644 (file)
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1535,8 +1535,10 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
                dso__set_loaded(curr_dso);
                *curr_mapp = curr_map;
                *curr_dsop = curr_dso;
-       } else
+       } else {
                *curr_dsop = map__dso(curr_map);
+               map__put(curr_map);
+       }
 
        return 0;
 }

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 1710b89..0785a54 100644 (file)
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -814,7 +814,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
                                struct map *initial_map)
 {
        struct machine *machine;
-       struct map *curr_map = initial_map;
+       struct map *curr_map = map__get(initial_map);
        struct symbol *pos;
        int count = 0, moved = 0;
        struct rb_root_cached *root = &dso->symbols;
@@ -858,13 +858,14 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
                                        dso__set_loaded(curr_map_dso);
                                }
 
+                               map__zput(curr_map);
                                curr_map = maps__find_by_name(kmaps, module);
                                if (curr_map == NULL) {
                                        pr_debug("%s/proc/{kallsyms,modules} "
                                                 "inconsistency while looking "
                                                 "for \"%s\" module!\n",
                                                 machine->root_dir, module);
-                                       curr_map = initial_map;
+                                       curr_map = map__get(initial_map);
                                        goto discard_symbol;
                                }
                                curr_map_dso = map__dso(curr_map);
@@ -888,7 +889,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
                         * symbols at this point.
                         */
                        goto discard_symbol;
-               } else if (curr_map != initial_map) {
+               } else if (!RC_CHK_EQUAL(curr_map, initial_map)) {
                        char dso_name[PATH_MAX];
                        struct dso *ndso;
 
@@ -899,7 +900,8 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
                        }
 
                        if (count == 0) {
-                               curr_map = initial_map;
+                               map__zput(curr_map);
+                               curr_map = map__get(initial_map);
                                goto add_symbol;
                        }
 
@@ -913,6 +915,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
                                        kernel_range++);
 
                        ndso = dso__new(dso_name);
+                       map__zput(curr_map);
                        if (ndso == NULL)
                                return -1;
 
@@ -926,6 +929,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
 
                        map__set_mapping_type(curr_map, MAPPING_TYPE__IDENTITY);
                        if (maps__insert(kmaps, curr_map)) {
+                               map__zput(curr_map);
                                dso__put(ndso);
                                return -1;
                        }
@@ -936,7 +940,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
                        pos->end -= delta;
                }
 add_symbol:
-               if (curr_map != initial_map) {
+               if (!RC_CHK_EQUAL(curr_map, initial_map)) {
                        struct dso *curr_map_dso = map__dso(curr_map);
 
                        rb_erase_cached(&pos->rb_node, root);
@@ -951,12 +955,12 @@ discard_symbol:
                symbol__delete(pos);
        }
 
-       if (curr_map != initial_map &&
+       if (!RC_CHK_EQUAL(curr_map, initial_map) &&
            dso->kernel == DSO_SPACE__KERNEL_GUEST &&
            machine__is_default_guest(maps__machine(kmaps))) {
                dso__set_loaded(map__dso(curr_map));
        }
-
+       map__put(curr_map);
        return count + moved;
 }