| 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
| ||
|
Date: Fri, 7 Apr 2023 16:04:00 -0700
From: Ian Rogers <irogers@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
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>
Cc: Stephane Eranian <eranian@...gle.com>,
Ian Rogers <irogers@...gle.com>
Subject: [PATCH v7 0/5] Reference count checker and related fixes
The perf tool has a class of memory problems where reference counts
are used incorrectly. Memory/address sanitizers and valgrind don't
provide useful ways to debug these problems, you see a memory leak
where the only pertinent information is the original allocation
site. What would be more useful is knowing where a get fails to have a
corresponding put, where there are double puts, etc.
This work was motivated by the roll-back of:
https://lore.kernel.org/linux-perf-users/20211118193714.2293728-1-irogers@google.com/
where fixing a missed put resulted in a use-after-free in a different
context. There was a sense in fixing the issue that a game of
wac-a-mole had been embarked upon in adding missed gets and puts.
The basic approach of the change is to add a level of indirection at
the get and put calls. Get allocates a level of indirection that, if
no corresponding put is called, becomes a memory leak (and associated
stack trace) that leak sanitizer can report. Similarly if two puts are
called for the same get, then a double free can be detected by address
sanitizer. This can also detect the use after put, which should also
yield a segv without a sanitizer.
Adding reference count checking to cpu map was done as a proof of
concept, it yielded little other than a location where the use of get
could be cleaner by using its result. Reference count checking on
nsinfo identified a double free of the indirection layer and the
related threads, thereby identifying a data race as discussed here:
https://lore.kernel.org/linux-perf-users/CAP-5=fWZH20L4kv-BwVtGLwR=Em3AOOT+Q4QGivvQuYn5AsPRg@mail.gmail.com/
Accordingly the dso->lock was extended and use to cover the race.
The v3 version addresses problems in v2, in particular using macros to
avoid #ifdefs. The v3 version applies the reference count checking
approach to two more data structures, maps and map. While maps was
straightforward, struct map showed a problem where reference counted
thing can be on lists and rb-trees that are oblivious to the
reference count. To sanitize this, struct map is changed so that it is
referenced by either a list or rb-tree node and not part of it. This
simplifies the reference count and the patches have caught and fixed a
number of missed or mismatched reference counts relating to struct
map.
A wider discussion of the approach is on the mailing list:
https://lore.kernel.org/linux-perf-users/YffqnynWcc5oFkI5@kernel.org/T/#mf25ccd7a2e03de92cec29d36e2999a8ab5ec7f88
Comparing it to a past approach:
https://lore.kernel.org/all/20151209021047.10245.8918.stgit@localhost.localdomain/
and to ref_tracker:
https://lwn.net/Articles/877603/
v7. rebase on top of merged and Arnaldo fixed changes. The remaining 5
patches no longer refactor APIs but just add the necessary
reference count checking macros and usage.
v6. rebase removing 5 merged changes. Fix missed issues with libunwind.
v5. rebase removing 5 merged changes. Add map_list_node__new to the
1st patch (perf map: Move map list node into symbol) as suggested
by Arnaldo. Remove unnecessary map__puts from patch 12 (perf map:
Changes to reference counting) as suggested by Adrian.
v4. rebases on to acme's perf-tools-next, fixes more issues with
map/maps and breaks apart the accessor functions to reduce
individual patch sizes. The accessor functions are mechanical
changes where the single biggest one is refactoring use of
map->dso to be map__dso(map).
Ian Rogers (5):
libperf: Add reference count checking macros.
perf cpumap: Add reference count checking
perf namespaces: Add reference count checking
perf maps: Add reference count checking.
perf map: Add reference count checking
tools/lib/perf/Makefile | 2 +-
tools/lib/perf/cpumap.c | 94 ++++++++-------
tools/lib/perf/include/internal/cpumap.h | 4 +-
tools/lib/perf/include/internal/rc_check.h | 94 +++++++++++++++
tools/perf/builtin-inject.c | 2 +-
tools/perf/builtin-top.c | 4 +-
tools/perf/tests/cpumap.c | 4 +-
tools/perf/tests/hists_link.c | 2 +-
tools/perf/tests/maps.c | 20 ++--
tools/perf/tests/thread-maps-share.c | 29 ++---
tools/perf/tests/vmlinux-kallsyms.c | 4 +-
tools/perf/util/annotate.c | 2 +-
tools/perf/util/cpumap.c | 40 +++----
tools/perf/util/dso.c | 2 +-
tools/perf/util/dsos.c | 2 +-
tools/perf/util/machine.c | 27 +++--
tools/perf/util/map.c | 69 ++++++-----
tools/perf/util/map.h | 32 ++---
tools/perf/util/maps.c | 64 +++++-----
tools/perf/util/maps.h | 17 +--
tools/perf/util/namespaces.c | 132 ++++++++++++---------
tools/perf/util/namespaces.h | 3 +-
tools/perf/util/pmu.c | 8 +-
tools/perf/util/symbol-elf.c | 26 ++--
tools/perf/util/symbol.c | 55 +++++----
tools/perf/util/unwind-libdw.c | 2 +-
tools/perf/util/unwind-libunwind-local.c | 2 +-
tools/perf/util/unwind-libunwind.c | 2 +-
28 files changed, 448 insertions(+), 296 deletions(-)
create mode 100644 tools/lib/perf/include/internal/rc_check.h
--
2.40.0.577.gac1e443424-goog
Powered by blists - more mailing lists