linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/22] Reference count checker and related fixes
@ 2022-02-11 10:33 Ian Rogers
  2022-02-11 10:33 ` [PATCH v3 01/22] perf cpumap: Migrate to libperf cpumap api Ian Rogers
                   ` (21 more replies)
  0 siblings, 22 replies; 58+ messages in thread
From: Ian Rogers @ 2022-02-11 10:33 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Thomas Gleixner, Darren Hart, Davidlohr Bueso,
	André Almeida, James Clark, John Garry, Riccardo Mancini,
	Yury Norov, Andy Shevchenko, Andrew Morton, Jin Yao,
	Adrian Hunter, Leo Yan, Andi Kleen, Thomas Richter, Kan Liang,
	Madhavan Srinivasan, Shunsuke Nakamura, Song Liu,
	Masami Hiramatsu, Steven Rostedt, Miaoqian Lin, Stephen Brennan,
	Kajol Jain, Alexey Bayduraev, German Gomez, linux-perf-users,
	linux-kernel, Eric Dumazet, Dmitry Vyukov, Hao Luo
  Cc: eranian, Ian Rogers

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.

The patches are arranged so that API refactors and bug fixes appear
first, then the reference count checker itself appears. This allows
for the refactor and fixes to be applied upstream first, as has
already happened with cpumap.

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/

Ian Rogers (22):
  perf cpumap: Migrate to libperf cpumap api
  perf cpumap: Use for each loop
  perf dso: Make lock error check and add BUG_ONs
  perf dso: Hold lock when accessing nsinfo
  perf maps: Use a pointer for kmaps
  perf test: Use pointer for maps
  perf maps: Reduce scope of init and exit
  perf maps: Move maps code to own C file
  perf map: Add const to map_ip and unmap_ip
  perf map: Make map__contains_symbol args const
  perf map: Move map list node into symbol
  perf maps: Remove rb_node from struct map
  perf namespaces: Add functions to access nsinfo
  perf maps: Add functions to access maps
  perf map: Use functions to access the variables in map
  perf test: Add extra diagnostics to maps test
  perf map: Changes to reference counting
  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/cpumap.c                       |  93 +--
 tools/lib/perf/include/internal/cpumap.h      |   4 +-
 tools/lib/perf/include/internal/rc_check.h    |  94 +++
 tools/perf/arch/s390/annotate/instructions.c  |   4 +-
 tools/perf/arch/x86/tests/dwarf-unwind.c      |   2 +-
 tools/perf/arch/x86/util/event.c              |  15 +-
 tools/perf/builtin-annotate.c                 |   8 +-
 tools/perf/builtin-inject.c                   |  14 +-
 tools/perf/builtin-kallsyms.c                 |   6 +-
 tools/perf/builtin-kmem.c                     |   4 +-
 tools/perf/builtin-mem.c                      |   4 +-
 tools/perf/builtin-probe.c                    |   2 +-
 tools/perf/builtin-report.c                   |  26 +-
 tools/perf/builtin-script.c                   |  26 +-
 tools/perf/builtin-top.c                      |  16 +-
 tools/perf/builtin-trace.c                    |   2 +-
 .../scripts/python/Perf-Trace-Util/Context.c  |  14 +-
 tools/perf/tests/code-reading.c               |  32 +-
 tools/perf/tests/cpumap.c                     |  14 +-
 tools/perf/tests/hists_common.c               |   4 +-
 tools/perf/tests/hists_cumulate.c             |  14 +-
 tools/perf/tests/hists_filter.c               |  14 +-
 tools/perf/tests/hists_link.c                 |  18 +-
 tools/perf/tests/hists_output.c               |  12 +-
 tools/perf/tests/maps.c                       |  87 ++-
 tools/perf/tests/mmap-thread-lookup.c         |   3 +-
 tools/perf/tests/thread-maps-share.c          |  29 +-
 tools/perf/tests/vmlinux-kallsyms.c           |  56 +-
 tools/perf/ui/browsers/annotate.c             |   7 +-
 tools/perf/ui/browsers/hists.c                |  21 +-
 tools/perf/ui/browsers/map.c                  |   4 +-
 tools/perf/util/Build                         |   1 +
 tools/perf/util/annotate.c                    |  38 +-
 tools/perf/util/auxtrace.c                    |   2 +-
 tools/perf/util/block-info.c                  |   4 +-
 tools/perf/util/bpf-event.c                   |  10 +-
 tools/perf/util/build-id.c                    |   6 +-
 tools/perf/util/callchain.c                   |  28 +-
 tools/perf/util/cpumap.c                      |  36 +-
 tools/perf/util/data-convert-json.c           |   4 +-
 tools/perf/util/db-export.c                   |  16 +-
 tools/perf/util/dlfilter.c                    |  29 +-
 tools/perf/util/dso.c                         |  21 +-
 tools/perf/util/event.c                       |  30 +-
 tools/perf/util/evsel_fprintf.c               |   4 +-
 tools/perf/util/hist.c                        |  22 +-
 tools/perf/util/intel-pt.c                    |  48 +-
 tools/perf/util/jitdump.c                     |  10 +-
 tools/perf/util/machine.c                     | 252 ++++---
 tools/perf/util/machine.h                     |   8 +-
 tools/perf/util/map.c                         | 629 ++++--------------
 tools/perf/util/map.h                         |  80 ++-
 tools/perf/util/maps.c                        | 475 +++++++++++++
 tools/perf/util/maps.h                        |  69 +-
 tools/perf/util/namespaces.c                  | 158 +++--
 tools/perf/util/namespaces.h                  |  13 +-
 tools/perf/util/pmu.c                         |  18 +-
 tools/perf/util/probe-event.c                 |  58 +-
 .../util/scripting-engines/trace-event-perl.c |   9 +-
 .../scripting-engines/trace-event-python.c    |  14 +-
 tools/perf/util/sort.c                        |  48 +-
 tools/perf/util/symbol-elf.c                  |  59 +-
 tools/perf/util/symbol.c                      | 280 +++++---
 tools/perf/util/symbol_fprintf.c              |   2 +-
 tools/perf/util/synthetic-events.c            |  34 +-
 tools/perf/util/thread-stack.c                |   4 +-
 tools/perf/util/thread.c                      |  40 +-
 tools/perf/util/unwind-libunwind-local.c      |  50 +-
 tools/perf/util/unwind-libunwind.c            |  34 +-
 tools/perf/util/vdso.c                        |   7 +-
 70 files changed, 1941 insertions(+), 1358 deletions(-)
 create mode 100644 tools/lib/perf/include/internal/rc_check.h
 create mode 100644 tools/perf/util/maps.c

-- 
2.35.1.265.g69c8d7142f-goog


^ permalink raw reply	[flat|nested] 58+ messages in thread

end of thread, other threads:[~2022-02-16 22:08 UTC | newest]

Thread overview: 58+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-11 10:33 [PATCH v3 00/22] Reference count checker and related fixes Ian Rogers
2022-02-11 10:33 ` [PATCH v3 01/22] perf cpumap: Migrate to libperf cpumap api Ian Rogers
2022-02-11 17:02   ` Arnaldo Carvalho de Melo
2022-02-11 10:33 ` [PATCH v3 02/22] perf cpumap: Use for each loop Ian Rogers
2022-02-11 17:04   ` Arnaldo Carvalho de Melo
2022-02-11 10:33 ` [PATCH v3 03/22] perf dso: Make lock error check and add BUG_ONs Ian Rogers
2022-02-11 17:13   ` Arnaldo Carvalho de Melo
2022-02-11 17:43     ` Ian Rogers
2022-02-11 19:21       ` Arnaldo Carvalho de Melo
2022-02-11 19:35         ` Ian Rogers
2022-02-12 15:48           ` Arnaldo Carvalho de Melo
2022-02-12 15:49             ` Arnaldo Carvalho de Melo
2022-02-12 20:59               ` Ian Rogers
2022-02-11 10:33 ` [PATCH v3 04/22] perf dso: Hold lock when accessing nsinfo Ian Rogers
2022-02-11 17:14   ` Arnaldo Carvalho de Melo
2022-02-12 11:30   ` Jiri Olsa
2022-02-11 10:33 ` [PATCH v3 05/22] perf maps: Use a pointer for kmaps Ian Rogers
2022-02-11 17:23   ` Arnaldo Carvalho de Melo
2022-02-14 19:45     ` Arnaldo Carvalho de Melo
2022-02-11 10:33 ` [PATCH v3 06/22] perf test: Use pointer for maps Ian Rogers
2022-02-11 17:24   ` Arnaldo Carvalho de Melo
2022-02-14 19:48   ` Arnaldo Carvalho de Melo
2022-02-14 19:50     ` Arnaldo Carvalho de Melo
2022-02-11 10:34 ` [PATCH v3 07/22] perf maps: Reduce scope of init and exit Ian Rogers
2022-02-11 17:26   ` Arnaldo Carvalho de Melo
2022-02-11 10:34 ` [PATCH v3 08/22] perf maps: Move maps code to own C file Ian Rogers
2022-02-11 17:27   ` Arnaldo Carvalho de Melo
2022-02-14 19:58   ` Arnaldo Carvalho de Melo
2022-02-11 10:34 ` [PATCH v3 09/22] perf map: Add const to map_ip and unmap_ip Ian Rogers
2022-02-11 17:28   ` Arnaldo Carvalho de Melo
2022-02-11 10:34 ` [PATCH v3 10/22] perf map: Make map__contains_symbol args const Ian Rogers
2022-02-11 17:28   ` Arnaldo Carvalho de Melo
2022-02-11 10:34 ` [PATCH v3 11/22] perf map: Move map list node into symbol Ian Rogers
2022-02-11 10:34 ` [PATCH v3 12/22] perf maps: Remove rb_node from struct map Ian Rogers
2022-02-16 14:08   ` Arnaldo Carvalho de Melo
2022-02-16 17:36     ` Ian Rogers
2022-02-16 20:12       ` Arnaldo Carvalho de Melo
2022-02-16 22:07         ` Ian Rogers
2022-02-11 10:34 ` [PATCH v3 13/22] perf namespaces: Add functions to access nsinfo Ian Rogers
2022-02-11 17:31   ` Arnaldo Carvalho de Melo
2022-02-11 10:34 ` [PATCH v3 14/22] perf maps: Add functions to access maps Ian Rogers
2022-02-11 17:33   ` Arnaldo Carvalho de Melo
2022-02-11 10:34 ` [PATCH v3 15/22] perf map: Use functions to access the variables in map Ian Rogers
2022-02-11 17:35   ` Arnaldo Carvalho de Melo
2022-02-11 17:36   ` Arnaldo Carvalho de Melo
2022-02-11 17:54     ` Ian Rogers
2022-02-11 19:22       ` Arnaldo Carvalho de Melo
2022-02-11 10:34 ` [PATCH v3 16/22] perf test: Add extra diagnostics to maps test Ian Rogers
2022-02-11 10:34 ` [PATCH v3 17/22] perf map: Changes to reference counting Ian Rogers
2022-02-12  8:45   ` Masami Hiramatsu
2022-02-12 20:48     ` Ian Rogers
2022-02-14  2:00       ` Masami Hiramatsu
2022-02-14 18:56       ` Arnaldo Carvalho de Melo
2022-02-11 10:34 ` [PATCH v3 18/22] libperf: Add reference count checking macros Ian Rogers
2022-02-11 10:34 ` [PATCH v3 19/22] perf cpumap: Add reference count checking Ian Rogers
2022-02-11 10:34 ` [PATCH v3 20/22] perf namespaces: " Ian Rogers
2022-02-11 10:34 ` [PATCH v3 21/22] perf maps: " Ian Rogers
2022-02-11 10:34 ` [PATCH v3 22/22] perf map: " Ian Rogers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).