perf map: Add reference count checking

There's no strict get/put policy with map that leads to leaks or use
after free. Reference count checking identifies correct pairing of gets
and puts.

Committer notes:

Extracted from a larger patch removing bits that were covered by the use
of pre-existing map__ accessors (e.g. maps__nr_maps()) and new ones
added (map__refcnt() and the maps__set_ ones) to reduce
RC_CHK_ACCESS(maps)-> source code pollution.

Signed-off-by: Ian Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
Cc: Dmitriy Vyukov <dvyukov@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Riccardo Mancini <rickyman7@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
Link: https://lore.kernel.org/lkml/20230407230405.2931830-6-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 64ce8097889c35f987a24321fed9fce4416518a4..141e2972e34f2087efd4e3ef7b6329a88f64461a 100644 (file)
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -145,7 +145,7 @@ static int find_sample(struct sample *samples, size_t nr_samples,
 {
        while (nr_samples--) {
                if (samples->thread == t &&
-                   samples->map == m &&
+                   RC_CHK_ACCESS(samples->map) == RC_CHK_ACCESS(m) &&
                    samples->sym == s)
                        return 1;
                samples++;

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 8ccbe48e23bdef0e0f794cfbdf42b57c69bc950c..9e02e19c1b7a9ab6c850e0256110e1a8db3c80bb 100644 (file)
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -953,7 +953,7 @@ static int machine__process_ksymbol_unregister(struct machine *machine,
        if (!map)
                return 0;
 
-       if (map != machine->vmlinux_map)
+       if (RC_CHK_ACCESS(map) != RC_CHK_ACCESS(machine->vmlinux_map))
                maps__remove(machine__kernel_maps(machine), map);
        else {
                struct dso *dso = map__dso(map);

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index bdd2742fa35b33e8c6f72e0ef6588a1369f80314..b7f890950909e22167eceef7cf75714f789d61d0 100644 (file)
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -120,11 +120,13 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
                     u32 prot, u32 flags, struct build_id *bid,
                     char *filename, struct thread *thread)
 {
-       struct map *map = malloc(sizeof(*map));
+       struct map *result;
+       RC_STRUCT(map) *map;
        struct nsinfo *nsi = NULL;
        struct nsinfo *nnsi;
 
-       if (map != NULL) {
+       map = malloc(sizeof(*map));
+       if (ADD_RC_CHK(result, map)) {
                char newfilename[PATH_MAX];
                struct dso *dso, *header_bid_dso;
                int anon, no_dso, vdso, android;
@@ -167,7 +169,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
                if (dso == NULL)
                        goto out_delete;
 
-               map__init(map, start, start + len, pgoff, dso);
+               map__init(result, start, start + len, pgoff, dso);
 
                if (anon || no_dso) {
                        map->map_ip = map->unmap_ip = identity__map_ip;
@@ -204,10 +206,10 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
                }
                dso__put(dso);
        }
-       return map;
+       return result;
 out_delete:
        nsinfo__put(nsi);
-       free(map);
+       RC_CHK_FREE(result);
        return NULL;
 }
 
@@ -218,16 +220,18 @@ out_delete:
  */
 struct map *map__new2(u64 start, struct dso *dso)
 {
-       struct map *map = calloc(1, (sizeof(*map) +
-                                    (dso->kernel ? sizeof(struct kmap) : 0)));
-       if (map != NULL) {
+       struct map *result;
+       RC_STRUCT(map) *map;
+
+       map = calloc(1, sizeof(*map) + (dso->kernel ? sizeof(struct kmap) : 0));
+       if (ADD_RC_CHK(result, map)) {
                /*
                 * ->end will be filled after we load all the symbols
                 */
-               map__init(map, start, 0, 0, dso);
+               map__init(result, start, 0, 0, dso);
        }
 
-       return map;
+       return result;
 }
 
 bool __map__is_kernel(const struct map *map)
@@ -293,19 +297,21 @@ bool map__has_symbols(const struct map *map)
 static void map__exit(struct map *map)
 {
        BUG_ON(refcount_read(map__refcnt(map)) != 0);
-       dso__zput(map->dso);
+       dso__zput(RC_CHK_ACCESS(map)->dso);
 }
 
 void map__delete(struct map *map)
 {
        map__exit(map);
-       free(map);
+       RC_CHK_FREE(map);
 }
 
 void map__put(struct map *map)
 {
        if (map && refcount_dec_and_test(map__refcnt(map)))
                map__delete(map);
+       else
+               RC_CHK_PUT(map);
 }
 
 void map__fixup_start(struct map *map)
@@ -400,20 +406,21 @@ struct symbol *map__find_symbol_by_name(struct map *map, const char *name)
 
 struct map *map__clone(struct map *from)
 {
-       size_t size = sizeof(struct map);
-       struct map *map;
+       struct map *result;
+       RC_STRUCT(map) *map;
+       size_t size = sizeof(RC_STRUCT(map));
        struct dso *dso = map__dso(from);
 
        if (dso && dso->kernel)
                size += sizeof(struct kmap);
 
-       map = memdup(from, size);
-       if (map != NULL) {
+       map = memdup(RC_CHK_ACCESS(from), size);
+       if (ADD_RC_CHK(result, map)) {
                refcount_set(&map->refcnt, 1);
                map->dso = dso__get(dso);
        }
 
-       return map;
+       return result;
 }
 
 size_t map__fprintf(struct map *map, FILE *fp)
@@ -567,7 +574,7 @@ struct kmap *__map__kmap(struct map *map)
 
        if (!dso || !dso->kernel)
                return NULL;
-       return (struct kmap *)(map + 1);
+       return (struct kmap *)(&RC_CHK_ACCESS(map)[1]);
 }
 
 struct kmap *map__kmap(struct map *map)

diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 0760c671314dc72370af2d4fd4fd5ed29dbfbffa..823ab7fc0acf0833b0cddcd71791660d4e689443 100644 (file)
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -10,12 +10,13 @@
 #include <string.h>
 #include <stdbool.h>
 #include <linux/types.h>
+#include <internal/rc_check.h>
 
 struct dso;
 struct maps;
 struct machine;
 
-struct map {
+DECLARE_RC_STRUCT(map) {
        u64                     start;
        u64                     end;
        bool                    erange_warned:1;
@@ -49,72 +50,72 @@ u64 identity__map_ip(const struct map *map __maybe_unused, u64 ip);
 
 static inline struct dso *map__dso(const struct map *map)
 {
-       return map->dso;
+       return RC_CHK_ACCESS(map)->dso;
 }
 
 static inline u64 map__map_ip(const struct map *map, u64 ip)
 {
-       return map->map_ip(map, ip);
+       return RC_CHK_ACCESS(map)->map_ip(map, ip);
 }
 
 static inline u64 map__unmap_ip(const struct map *map, u64 ip)
 {
-       return map->unmap_ip(map, ip);
+       return RC_CHK_ACCESS(map)->unmap_ip(map, ip);
 }
 
 static inline void *map__map_ip_ptr(struct map *map)
 {
-       return map->map_ip;
+       return RC_CHK_ACCESS(map)->map_ip;
 }
 
 static inline void* map__unmap_ip_ptr(struct map *map)
 {
-       return map->unmap_ip;
+       return RC_CHK_ACCESS(map)->unmap_ip;
 }
 
 static inline u64 map__start(const struct map *map)
 {
-       return map->start;
+       return RC_CHK_ACCESS(map)->start;
 }
 
 static inline u64 map__end(const struct map *map)
 {
-       return map->end;
+       return RC_CHK_ACCESS(map)->end;
 }
 
 static inline u64 map__pgoff(const struct map *map)
 {
-       return map->pgoff;
+       return RC_CHK_ACCESS(map)->pgoff;
 }
 
 static inline u64 map__reloc(const struct map *map)
 {
-       return map->reloc;
+       return RC_CHK_ACCESS(map)->reloc;
 }
 
 static inline u32 map__flags(const struct map *map)
 {
-       return map->flags;
+       return RC_CHK_ACCESS(map)->flags;
 }
 
 static inline u32 map__prot(const struct map *map)
 {
-       return map->prot;
+       return RC_CHK_ACCESS(map)->prot;
 }
 
 static inline bool map__priv(const struct map *map)
 {
-       return map->priv;
+       return RC_CHK_ACCESS(map)->priv;
 }
 
 static inline refcount_t *map__refcnt(struct map *map)
 {
-       return &map->refcnt;
+       return &RC_CHK_ACCESS(map)->refcnt;
 }
 
 static inline bool map__erange_warned(struct map *map)
 {
-       return map->erange_warned;
+       return RC_CHK_ACCESS(map)->erange_warned;
 }
 
 static inline size_t map__size(const struct map *map)
@@ -173,9 +174,12 @@ struct map *map__clone(struct map *map);
 
 static inline struct map *map__get(struct map *map)
 {
-       if (map)
+       struct map *result;
+
+       if (RC_CHK_GET(result, map))
                refcount_inc(map__refcnt(map));
-       return map;
+
+       return result;
 }
 
 void map__put(struct map *map);
@@ -249,51 +253,51 @@ static inline int is_no_dso_memory(const char *filename)
 
 static inline void map__set_start(struct map *map, u64 start)
 {
-       map->start = start;
+       RC_CHK_ACCESS(map)->start = start;
 }
 
 static inline void map__set_end(struct map *map, u64 end)
 {
-       map->end = end;
+       RC_CHK_ACCESS(map)->end = end;
 }
 
 static inline void map__set_pgoff(struct map *map, u64 pgoff)
 {
-       map->pgoff = pgoff;
+       RC_CHK_ACCESS(map)->pgoff = pgoff;
 }
 
 static inline void map__add_pgoff(struct map *map, u64 inc)
 {
-       map->pgoff += inc;
+       RC_CHK_ACCESS(map)->pgoff += inc;
 }
 
 static inline void map__set_reloc(struct map *map, u64 reloc)
 {
-       map->reloc = reloc;
+       RC_CHK_ACCESS(map)->reloc = reloc;
 }
 
 static inline void map__set_priv(struct map *map, int priv)
 {
-       map->priv = priv;
+       RC_CHK_ACCESS(map)->priv = priv;
 }
 
 static inline void map__set_erange_warned(struct map *map, bool erange_warned)
 {
-       map->erange_warned = erange_warned;
+       RC_CHK_ACCESS(map)->erange_warned = erange_warned;
 }
 
 static inline void map__set_dso(struct map *map, struct dso *dso)
 {
-       map->dso = dso;
+       RC_CHK_ACCESS(map)->dso = dso;
 }
 
 static inline void map__set_map_ip(struct map *map, u64 (*map_ip)(const struct map *map, u64 ip))
 {
-       map->map_ip = map_ip;
+       RC_CHK_ACCESS(map)->map_ip = map_ip;
 }
 
 static inline void map__set_unmap_ip(struct map *map, u64 (*unmap_ip)(const struct map *map, u64 rip))
 {
-       map->unmap_ip = unmap_ip;
+       RC_CHK_ACCESS(map)->unmap_ip = unmap_ip;
 }
 #endif /* __PERF_MAP_H */

diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index df2fc8221f3c698699ad9463a240ed1e40399d19..1aeb1db58fe5902e1dcfb3fe40fbf916cc318e6e 100644 (file)
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -126,7 +126,7 @@ void maps__remove(struct maps *maps, struct map *map)
                RC_CHK_ACCESS(maps)->last_search_by_name = NULL;
 
        rb_node = maps__find_node(maps, map);
-       assert(rb_node->map == map);
+       assert(rb_node->RC_CHK_ACCESS(map) == RC_CHK_ACCESS(map));
        __maps__remove(maps, rb_node);
        if (maps__maps_by_name(maps))
                __maps__free_maps_by_name(maps);
@@ -420,7 +420,7 @@ struct map_rb_node *maps__find_node(struct maps *maps, struct map *map)
        struct map_rb_node *rb_node;
 
        maps__for_each_entry(maps, rb_node) {
-               if (rb_node->map == map)
+               if (rb_node->RC_CHK_ACCESS(map) == RC_CHK_ACCESS(map))
                        return rb_node;
        }
        return NULL;

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 35d860f95b18d73c67f8e1df1f0c5a809bd623df..6b9c55784b56a4be3db2a79680f235ee293ce568 100644 (file)
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -865,7 +865,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
                        *module++ = '\0';
                        curr_map_dso = map__dso(curr_map);
                        if (strcmp(curr_map_dso->short_name, module)) {
-                               if (curr_map != initial_map &&
+                               if (RC_CHK_ACCESS(curr_map) != RC_CHK_ACCESS(initial_map) &&
                                    dso->kernel == DSO_SPACE__KERNEL_GUEST &&
                                    machine__is_default_guest(machine)) {
                                        /*
@@ -1457,7 +1457,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 
                list_del_init(&new_node->node);
 
-               if (new_map == replacement_map) {
+               if (RC_CHK_ACCESS(new_map) == RC_CHK_ACCESS(replacement_map)) {
                        map__set_start(map, map__start(new_map));
                        map__set_end(map, map__end(new_map));
                        map__set_pgoff(map, map__pgoff(new_map));