Messages in this thread Patch in this message | | | Date | Wed, 12 Apr 2023 12:50:05 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH v7 2/5] perf cpumap: Add reference count checking |
| |
Em Tue, Apr 11, 2023 at 03:19:06PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Fri, Apr 07, 2023 at 04:04:02PM -0700, Ian Rogers escreveu: > > 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.
> I think this should be further split into self contained patches as it > does:
> These are missing convertions that should be in a separate patch, no? > > > @@ -239,7 +236,7 @@ struct cpu_aggr_map *cpu_aggr_map__new(const struct perf_cpu_map *cpus, > > { > > int idx; > > struct perf_cpu cpu; > > - struct cpu_aggr_map *c = cpu_aggr_map__empty_new(cpus->nr); > > + struct cpu_aggr_map *c = cpu_aggr_map__empty_new(perf_cpu_map__nr(cpus)); > > > > if (!c) > > return NULL;
Extracted this from your larger patch:
From f8a23fce48400168be0cc078a0b0bd0e7d4a889f Mon Sep 17 00:00:00 2001 From: Ian Rogers <irogers@google.com> Date: Wed, 12 Apr 2023 12:45:45 -0300 Subject: [PATCH 1/1] perf cpumap: Use perf_cpu_map__nr(cpus) to access cpus->nr
So that we can have a single point where to refcount check 'struct perf_cpu_map' instances for use after free, etc.
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/ Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/cpumap.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c index 5e564974fba4ffab..c8484b75413ef709 100644 --- a/tools/perf/util/cpumap.c +++ b/tools/perf/util/cpumap.c @@ -239,7 +239,7 @@ struct cpu_aggr_map *cpu_aggr_map__new(const struct perf_cpu_map *cpus, { int idx; struct perf_cpu cpu; - struct cpu_aggr_map *c = cpu_aggr_map__empty_new(cpus->nr); + struct cpu_aggr_map *c = cpu_aggr_map__empty_new(perf_cpu_map__nr(cpus)); if (!c) return NULL; @@ -263,7 +263,7 @@ struct cpu_aggr_map *cpu_aggr_map__new(const struct perf_cpu_map *cpus, } } /* Trim. */ - if (c->nr != cpus->nr) { + if (c->nr != perf_cpu_map__nr(cpus)) { struct cpu_aggr_map *trimmed_c = realloc(c, sizeof(struct cpu_aggr_map) + sizeof(struct aggr_cpu_id) * c->nr); @@ -582,9 +582,9 @@ size_t cpu_map__snprint(struct perf_cpu_map *map, char *buf, size_t size) #define COMMA first ? "" : "," - for (i = 0; i < map->nr + 1; i++) { + for (i = 0; i < perf_cpu_map__nr(map) + 1; i++) { struct perf_cpu cpu = { .cpu = INT_MAX }; - bool last = i == map->nr; + bool last = i == perf_cpu_map__nr(map); if (!last) cpu = map->map[i]; @@ -633,7 +633,7 @@ size_t cpu_map__snprint_mask(struct perf_cpu_map *map, char *buf, size_t size) int i, cpu; char *ptr = buf; unsigned char *bitmap; - struct perf_cpu last_cpu = perf_cpu_map__cpu(map, map->nr - 1); + struct perf_cpu last_cpu = perf_cpu_map__cpu(map, perf_cpu_map__nr(map) - 1); if (buf == NULL) return 0; @@ -644,7 +644,7 @@ size_t cpu_map__snprint_mask(struct perf_cpu_map *map, char *buf, size_t size) return 0; } - for (i = 0; i < map->nr; i++) { + for (i = 0; i < perf_cpu_map__nr(map); i++) { cpu = perf_cpu_map__cpu(map, i).cpu; bitmap[cpu / 8] |= 1 << (cpu % 8); } -- 2.39.2
| |