linux-kernel - Re: [PATCH v7 2/5] perf cpumap: Add reference count checking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZDbRIJknafLnDwtO@kernel.org>
Date:   Wed, 12 Apr 2023 12:41:20 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Ian Rogers <irogers@...gle.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Darren Hart <dvhart@...radead.org>,
        Davidlohr Bueso <dave@...olabs.net>,
        James Clark <james.clark@....com>,
        John Garry <john.g.garry@...cle.com>,
        Riccardo Mancini <rickyman7@...il.com>,
        Yury Norov <yury.norov@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Leo Yan <leo.yan@...aro.org>, Andi Kleen <ak@...ux.intel.com>,
        Thomas Richter <tmricht@...ux.ibm.com>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Madhavan Srinivasan <maddy@...ux.ibm.com>,
        Shunsuke Nakamura <nakamura.shun@...itsu.com>,
        Song Liu <song@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Miaoqian Lin <linmq006@...il.com>,
        Stephen Brennan <stephen.s.brennan@...cle.com>,
        Kajol Jain <kjain@...ux.ibm.com>,
        Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>,
        German Gomez <german.gomez@....com>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        Eric Dumazet <edumazet@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Hao Luo <haoluo@...gle.com>,
        Stephane Eranian <eranian@...gle.com>
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:

> 3. Shouldn't we have a map__refcnt(map) accessor to avoid using those
> verbose macros outside the object that is having its reference counts
> checked?

So I added this one, in the end its just to avoid 

>From 997a7909789d5b5570dc3cfcd1b9f03877f544bd Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@...hat.com>
Date: Wed, 12 Apr 2023 12:36:58 -0300
Subject: [PATCH 1/1] perf map: Add map__refcnt() accessor to use in the maps
 test

To remove one more direct access to 'struct map' so that we can intecept
accesses to its instantiations and refcount check it to catch use after
free, etc.

Signed-off-by: Ian Rogers <irogers@...gle.com>
Cc: Adrian Hunter <adrian.hunter@...el.com>
Cc: Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>
Cc: Dmitriy Vyukov <dvyukov@...gle.com>
Cc: Jiri Olsa <jolsa@...nel.org>
Cc: Namhyung Kim <namhyung@...nel.org>
Cc: Riccardo Mancini <rickyman7@...il.com>
Cc: Stephane Eranian <eranian@...gle.com>
Cc: Stephen Brennan <stephen.s.brennan@...cle.com>
Link: https://lore.kernel.org/lkml/
Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
---
 tools/perf/tests/maps.c | 4 ++--
 tools/perf/util/map.h   | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/maps.c b/tools/perf/tests/maps.c
index 1c7293476acad2e5..a6278f9c8b713220 100644
--- a/tools/perf/tests/maps.c
+++ b/tools/perf/tests/maps.c
@@ -30,7 +30,7 @@ static int check_maps(struct map_def *merged, unsigned int size, struct maps *ma
 			if (map__start(map) != merged[i].start ||
 			    map__end(map) != merged[i].end ||
 			    strcmp(map__dso(map)->name, merged[i].name) ||
-			    refcount_read(&map->refcnt) != 1) {
+			    refcount_read(map__refcnt(map)) != 1) {
 				failed = true;
 			}
 			i++;
@@ -50,7 +50,7 @@ static int check_maps(struct map_def *merged, unsigned int size, struct maps *ma
 				map__start(map),
 				map__end(map),
 				map__dso(map)->name,
-				refcount_read(&map->refcnt));
+				refcount_read(map__refcnt(map)));
 		}
 	}
 	return failed ? TEST_FAIL : TEST_OK;
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 102485699aa8d7c3..f89ab7c2d3277239 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -97,6 +97,11 @@ static inline bool map__priv(const struct map *map)
 	return map->priv;
 }
 
+static inline refcount_t *map__refcnt(struct map *map)
+{
+	return &map->refcnt;
+}
+
 static inline size_t map__size(const struct map *map)
 {
 	return map__end(map) - map__start(map);
-- 
2.39.2