Messages in this thread Patch in this message | | | Date | Mon, 17 Apr 2023 12:49:09 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v7 2/5] perf cpumap: Add reference count checking |
| |
Em Fri, Apr 07, 2023 at 04:04:02PM -0700, Ian Rogers escreveu: > Enabled when REFCNT_CHECKING is defined. The change adds a memory > allocated pointer that is interposed between the reference counted > cpu map at a get and freed by a put. The pointer replaces the original > perf_cpu_map struct, so use of the perf_cpu_map via APIs remains > unchanged. Any use of the cpu map without the API requires two versions, > handled via the RC_CHK_ACCESS macro. > > This change is intended to catch: > - use after put: using a cpumap after you have put it will cause a > segv. > - unbalanced puts: two puts for a get will result in a double free > that can be captured and reported by tools like address sanitizer, > including with the associated stack traces of allocation and frees. > - missing puts: if a put is missing then the get turns into a memory > leak that can be reported by leak sanitizer, including the stack > trace at the point the get occurs. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/lib/perf/Makefile | 2 +- > tools/lib/perf/cpumap.c | 94 +++++++++++++----------- > tools/lib/perf/include/internal/cpumap.h | 4 +- > tools/perf/tests/cpumap.c | 4 +- > tools/perf/util/cpumap.c | 40 +++++----- > tools/perf/util/pmu.c | 8 +- > 6 files changed, 81 insertions(+), 71 deletions(-) > > diff --git a/tools/lib/perf/Makefile b/tools/lib/perf/Makefile > index d8cad124e4c5..3a9b2140aa04 100644 > --- a/tools/lib/perf/Makefile > +++ b/tools/lib/perf/Makefile > @@ -188,7 +188,7 @@ install_lib: libs > cp -fpR $(LIBPERF_ALL) $(DESTDIR)$(libdir_SQ) > > HDRS := bpf_perf.h core.h cpumap.h threadmap.h evlist.h evsel.h event.h mmap.h > -INTERNAL_HDRS := cpumap.h evlist.h evsel.h lib.h mmap.h threadmap.h xyarray.h > +INTERNAL_HDRS := cpumap.h evlist.h evsel.h lib.h mmap.h rc_check.h threadmap.h xyarray.h > > INSTALL_HDRS_PFX := $(DESTDIR)$(prefix)/include/perf > INSTALL_HDRS := $(addprefix $(INSTALL_HDRS_PFX)/, $(HDRS)) > diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c > index 6cd0be7c1bb4..56eed1ac80d9 100644 > --- a/tools/lib/perf/cpumap.c > +++ b/tools/lib/perf/cpumap.c > @@ -10,16 +10,16 @@ > #include <ctype.h> > #include <limits.h> > > -static struct perf_cpu_map *perf_cpu_map__alloc(int nr_cpus) > +struct perf_cpu_map *perf_cpu_map__alloc(int nr_cpus) > { > - struct perf_cpu_map *cpus = malloc(sizeof(*cpus) + sizeof(struct perf_cpu) * nr_cpus); > - > - if (cpus != NULL) { > + struct perf_cpu_map *result; > + RC_STRUCT(perf_cpu_map) *cpus = > + malloc(sizeof(*cpus) + sizeof(struct perf_cpu) * nr_cpus); > + if (ADD_RC_CHK(result, cpus)) { > cpus->nr = nr_cpus; > refcount_set(&cpus->refcnt, 1); > - > } > - return cpus; > + return result; > } > > struct perf_cpu_map *perf_cpu_map__dummy_new(void) > @@ -27,7 +27,7 @@ struct perf_cpu_map *perf_cpu_map__dummy_new(void) > struct perf_cpu_map *cpus = perf_cpu_map__alloc(1); > > if (cpus) > - cpus->map[0].cpu = -1; > + RC_CHK_ACCESS(cpus)->map[0].cpu = -1;
One more:
From 93bbe2e90194f70df537e0ea4836277c316b3050 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo <acme@redhat.com> Date: Mon, 17 Apr 2023 12:47:49 -0300 Subject: [PATCH 1/1] libperf: Add perf_cpu_map__refcnt() interanl accessor to use in the maps test
To remove one more direct access to 'struct perf_cpu_map' so that we can intercept accesses to its instantiations and refcount check it to catch use after free, etc.
Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com> Cc: Dmitriy Vyukov <dvyukov@google.com> Cc: Ian Rogers <irogers@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/ Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/lib/perf/cpumap.c | 6 +++--- tools/lib/perf/include/internal/cpumap.h | 4 ++++ tools/perf/tests/cpumap.c | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c index 6bbcbb83eb14cc45..27c3e73c6db29b57 100644 --- a/tools/lib/perf/cpumap.c +++ b/tools/lib/perf/cpumap.c @@ -40,7 +40,7 @@ struct perf_cpu_map *perf_cpu_map__dummy_new(void) static void cpu_map__delete(struct perf_cpu_map *map) { if (map) { - WARN_ONCE(refcount_read(&map->refcnt) != 0, + WARN_ONCE(refcount_read(perf_cpu_map__refcnt(map)) != 0, "cpu_map refcnt unbalanced\n"); free(map); } @@ -49,13 +49,13 @@ static void cpu_map__delete(struct perf_cpu_map *map) struct perf_cpu_map *perf_cpu_map__get(struct perf_cpu_map *map) { if (map) - refcount_inc(&map->refcnt); + refcount_inc(perf_cpu_map__refcnt(map)); return map; } void perf_cpu_map__put(struct perf_cpu_map *map) { - if (map && refcount_dec_and_test(&map->refcnt)) + if (map && refcount_dec_and_test(perf_cpu_map__refcnt(map))) cpu_map__delete(map); } diff --git a/tools/lib/perf/include/internal/cpumap.h b/tools/lib/perf/include/internal/cpumap.h index b82fd6607a00e3dc..1e840dd53a11adc6 100644 --- a/tools/lib/perf/include/internal/cpumap.h +++ b/tools/lib/perf/include/internal/cpumap.h @@ -30,4 +30,8 @@ bool perf_cpu_map__is_subset(const struct perf_cpu_map *a, const struct perf_cpu void perf_cpu_map__set_nr(struct perf_cpu_map *map, int nr_cpus); +static inline refcount_t *perf_cpu_map__refcnt(struct perf_cpu_map *map) +{ + return &map->refcnt; +} #endif /* __LIBPERF_INTERNAL_CPUMAP_H */ diff --git a/tools/perf/tests/cpumap.c b/tools/perf/tests/cpumap.c index 3150fc1fed6f503b..b1a924314e095b0d 100644 --- a/tools/perf/tests/cpumap.c +++ b/tools/perf/tests/cpumap.c @@ -68,7 +68,7 @@ static int process_event_cpus(struct perf_tool *tool __maybe_unused, TEST_ASSERT_VAL("wrong nr", perf_cpu_map__nr(map) == 2); TEST_ASSERT_VAL("wrong cpu", perf_cpu_map__cpu(map, 0).cpu == 1); TEST_ASSERT_VAL("wrong cpu", perf_cpu_map__cpu(map, 1).cpu == 256); - TEST_ASSERT_VAL("wrong refcnt", refcount_read(&map->refcnt) == 1); + TEST_ASSERT_VAL("wrong refcnt", refcount_read(perf_cpu_map__refcnt(map)) == 1); perf_cpu_map__put(map); return 0; } @@ -94,7 +94,7 @@ static int process_event_range_cpus(struct perf_tool *tool __maybe_unused, TEST_ASSERT_VAL("wrong nr", perf_cpu_map__nr(map) == 256); TEST_ASSERT_VAL("wrong cpu", perf_cpu_map__cpu(map, 0).cpu == 1); TEST_ASSERT_VAL("wrong cpu", perf_cpu_map__max(map).cpu == 256); - TEST_ASSERT_VAL("wrong refcnt", refcount_read(&map->refcnt) == 1); + TEST_ASSERT_VAL("wrong refcnt", refcount_read(perf_cpu_map__refcnt(map)) == 1); perf_cpu_map__put(map); return 0; } -- 2.39.2
| |