Commit 2832ef81 authored by Ian Rogers's avatar Ian Rogers Committed by Arnaldo Carvalho de Melo
Browse files

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: default avatarIan 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: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent e6a9efce

tools/perf/tests/hists_link.c

+1 −1
Original line number Diff line number Diff line
@@ -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++;

tools/perf/util/machine.c

+1 −1
Original line number Diff line number Diff line
@@ -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);

tools/perf/util/map.c

+25 −18
Original line number Diff line number Diff line
@@ -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 @@ struct map *map__new(struct machine *machine, u64 start, u64 len, 
 */

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)

tools/perf/util/map.h

+31 −27
Original line number Diff line number Diff line
@@ -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 */

tools/perf/util/maps.c

+2 −2
Original line number Diff line number Diff line
@@ -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;