Messages in this thread | | | From | Ian Rogers <> | Date | Wed, 12 Apr 2023 09:07:54 -0700 | Subject | Re: [PATCH v7 2/5] perf cpumap: Add reference count checking |
| |
On Wed, Apr 12, 2023 at 8:50 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > 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.
Thanks! In one of your previous messages you mentioned adding some setters for cpu_map to bury the macro usage. As cpu maps are immutable, can we make these internal APIs? Other than that, thanks for helping to get this landed!
Ian
> 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 >
| |