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>

5 files changed, 61 insertions, 50 deletions

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 8ccbe48e23bd..9e02e19c1b7a 100644
--- 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 bdd2742fa35b..b7f890950909 100644
--- 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 0760c671314d..823ab7fc0acf 100644
--- 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 df2fc8221f3c..1aeb1db58fe5 100644
--- 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 35d860f95b18..6b9c55784b56 100644
--- 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));