1. 20 Apr, 2023 2 commits
    • Ian Rogers's avatar
      libperf rc_check: Enable implicitly with sanitizers · 9be6ab18
      Ian Rogers authored
      If using leak sanitizer then implicitly enable reference count checking.
      Signed-off-by: default avatarIan Rogers <irogers@google.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Link: https://lore.kernel.org/r/20230420171812.561603-1-irogers@google.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      9be6ab18
    • Ian Rogers's avatar
      perf test: Fix maps use after put · edd4cab2
      Ian Rogers authored
      Fix a use after put reference count issue. maps is copied from leader,
      but the leader is put on line 79 and then maps is used to read the
      reference count below - so a use after put, with the put of maps
      happening within thread__put. Fix by reversing the order of puts so
      that the leader is put last.
      
      To explain the reference count checker, I wrote this up as a little
      example here:
      https://perf.wiki.kernel.org/index.php/Reference_Count_Checking
      
      Note, the bug was introduced by the committer and wasn't present in
      the original reference count patch set.
      
      Committer notes:
      
      Yes, the bug predated your patch and is detected by the reference count
      checking you contributed.
      
      This was just part of splitting up your series into smaller chunks, in
      this case either we fix the problem detected while developing this
      reference counting infrastructure before the patch introducing REFCNT_CHECKING
      or fix it later after the merged infrastructure, when built with
      EXTRA_CFLAGS="-DREFCNT_CHECKING=1" detects it when running 'perf test', which
      is what this patch does.
      Signed-off-by: default avatarIan Rogers <irogers@google.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Link: http://lore.kernel.org/lkml/20230420030430.489243-1-irogers@google.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      edd4cab2
  2. 19 Apr, 2023 9 commits
  3. 18 Apr, 2023 5 commits
  4. 17 Apr, 2023 7 commits
    • Ian Rogers's avatar
      perf namespaces: Add reference count checking · c35ce1d9
      Ian Rogers authored
      Add reference count checking controlled by REFCNT_CHECKING ifdef. The
      reference count checking interposes an allocated pointer between the
      reference counted struct on a get and frees the pointer on a put.
      Accesses after a put cause faults and use after free, missed puts are
      caughts as leaks and double puts are double frees.
      
      This checking helped resolve a memory leak and use after free:
      https://lore.kernel.org/linux-perf-users/CAP-5=fWZH20L4kv-BwVtGLwR=Em3AOOT+Q4QGivvQuYn5AsPRg@mail.gmail.com/Signed-off-by: default avatarIan 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/20230407230405.2931830-4-irogers@google.com
      [ Extracted from a larger patch ]
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      c35ce1d9
    • Arnaldo Carvalho de Melo's avatar
      perf dso: Add dso__filename_with_chroot() to reduce number of accesses to dso->nsinfo members · 7031edac
      Arnaldo Carvalho de Melo authored
      We'll need to reference count dso->nsinfo, so reduce the number of
      direct accesses by having a shorter form of obtaining a filename with
      a chroot (namespace one).
      
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Ian Rogers <irogers@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/ZD26ZlqSbgSyH5lX@kernel.org
      [ Used nsinfo__pid(dso->nsinfo), as it was already present ]
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      7031edac
    • Ian Rogers's avatar
      perf cpumap: Add reference count checking · da885a0e
      Ian Rogers authored
      Enabled when REFCNT_CHECKING is defined. The change adds a memory
      allocated pointer that is interposed between the reference counted cpu
      map at a get and freed by a put. The pointer replaces the original
      perf_cpu_map struct, so use of the perf_cpu_map via APIs remains
      unchanged. Any use of the cpu map without the API requires two versions,
      handled via the RC_CHK_ACCESS macro.
      
      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.
      
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Andi Kleen <ak@linux.intel.com>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
      Cc: Darren Hart <dvhart@infradead.org>
      Cc: Davidlohr Bueso <dave@stgolabs.net>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Eric Dumazet <edumazet@google.com>
      Cc: German Gomez <german.gomez@arm.com>
      Cc: Hao Luo <haoluo@google.com>
      Cc: Ingo Molnar <mingo@redhat.com>
      Cc: James Clark <james.clark@arm.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: John Garry <john.g.garry@oracle.com>
      Cc: Kajol Jain <kjain@linux.ibm.com>
      Cc: Kan Liang <kan.liang@linux.intel.com>
      Cc: Leo Yan <leo.yan@linaro.org>
      Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Masami Hiramatsu <mhiramat@kernel.org>
      Cc: Miaoqian Lin <linmq006@gmail.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Riccardo Mancini <rickyman7@gmail.com>
      Cc: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
      Cc: Song Liu <song@kernel.org>
      Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
      Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
      Cc: Thomas Gleixner <tglx@linutronix.de>
      Cc: Thomas Richter <tmricht@linux.ibm.com>,
      Cc: Yury Norov <yury.norov@gmail.com>
      Link: https://lore.kernel.org/lkml/20230407230405.2931830-3-irogers@google.com
      [ Extracted from a larger patch ]
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      da885a0e
    • Ian Rogers's avatar
      perf cpumap: Use perf_cpu_map__cpu(map, cpu) instead of accessing map->map[cpu] directly · 491b13c4
      Ian Rogers authored
      So that we can validate the 'map' instance wrt refcount checking.
      
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Ian Rogers <irogers@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/20230407230405.2931830-3-irogers@google.com
      [ Extracted from a larger patch ]
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      491b13c4
    • Arnaldo Carvalho de Melo's avatar
      perf cpumap: Remove initializations done in perf_cpu_map__alloc() · d57fd492
      Arnaldo Carvalho de Melo authored
      When extracting this patch from Ian's original patch I forgot to remove
      the setting of ->nr and ->refcnt, no need to do those initializations
      again as those are done in perf_cpu_map__alloc() already, duh.
      
      Cc: Ian Rogers <irogers@google.com>
      Fixes: 1f94479e ("libperf: Make perf_cpu_map__alloc() available as an internal function for tools/perf to use")
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      d57fd492
    • Ian Rogers's avatar
      libperf: Add reference count checking macros · a9b867f6
      Ian Rogers authored
      The macros serve as a way to debug use of a reference counted struct.
      
      The macros add a memory allocated pointer that is interposed between
      the reference counted original struct at a get and freed by a put.
      
      The pointer replaces the original struct, so use of the struct name
      via APIs remains unchanged.
      Signed-off-by: default avatarIan Rogers <irogers@google.com>
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Andi Kleen <ak@linux.intel.com>
      Cc: Andrew Morton <akpm@linux-foundation.org>
      Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
      Cc: Darren Hart <dvhart@infradead.org>
      Cc: Davidlohr Bueso <dave@stgolabs.net>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Eric Dumazet <edumazet@google.com>
      Cc: German Gomez <german.gomez@arm.com>
      Cc: Hao Luo <haoluo@google.com>
      Cc: James Clark <james.clark@arm.com>
      Cc: Jiri Olsa <jolsa@kernel.org>
      Cc: John Garry <john.g.garry@oracle.com>
      Cc: Kajol Jain <kjain@linux.ibm.com>
      Cc: Kan Liang <kan.liang@linux.intel.com>
      Cc: Leo Yan <leo.yan@linaro.org>
      Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
      Cc: Mark Rutland <mark.rutland@arm.com>
      Cc: Masami Hiramatsu <mhiramat@kernel.org>
      Cc: Miaoqian Lin <linmq006@gmail.com>
      Cc: Namhyung Kim <namhyung@kernel.org>
      Cc: Peter Zijlstra <peterz@infradead.org>
      Cc: Riccardo Mancini <rickyman7@gmail.com>
      Cc: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
      Cc: Song Liu <song@kernel.org>
      Cc: Stephane Eranian <eranian@google.com>
      Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
      Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
      Cc: Thomas Gleixner <tglx@linutronix.de>
      Cc: Thomas Richter <tmricht@linux.ibm.com>
      Cc: Yury Norov <yury.norov@gmail.com>
      Link: http://lore.kernel.org/lkml/20230407230405.2931830-2-irogers@google.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      a9b867f6
    • Arnaldo Carvalho de Melo's avatar
      libperf: Add perf_cpu_map__refcnt() interanl accessor to use in the maps test · 4121234a
      Arnaldo Carvalho de Melo authored
      To remove one more direct access to 'struct perf_cpu_map' so that we can
      intercept accesses to its instantiations and refcount check it to catch
      use after free, etc.
      
      Cc: Adrian Hunter <adrian.hunter@intel.com>
      Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
      Cc: Dmitriy Vyukov <dvyukov@google.com>
      Cc: Ian Rogers <irogers@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/ZD1qdYjG+DL6KOfP@kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      4121234a
  5. 15 Apr, 2023 1 commit
    • Arnaldo Carvalho de Melo's avatar
      perf test: Simplify for_each_test() to avoid tripping on -Werror=array-bounds · 17354d15
      Arnaldo Carvalho de Melo authored
      When cross building on debian to the mips 32-bit arch we get these
      warnings:
      
        In function '__cmd_test',
            inlined from 'cmd_test' at tests/builtin-test.c:561:9:
        tests/builtin-test.c:260:66: error: array subscript 1 is outside array bounds of 'struct test_suite *[1]' [-Werror=array-bounds]
          260 |                 for (k = 0, t = tests[j][k]; tests[j][k]; k++, t = tests[j][k])
              |                                                                  ^
        tests/builtin-test.c:369:9: note: in expansion of macro 'for_each_test'
          369 |         for_each_test(j, k, t) {
              |         ^~~~~~~~~~~~~
        tests/builtin-test.c: In function 'cmd_test':
        tests/builtin-test.c:36:27: note: at offset 4 into object 'arch_tests' of size 4
           36 | struct test_suite *__weak arch_tests[] = {
              |                           ^~~~~~~~~~
        cc1: all warnings being treated as errors
      
      Switch to using a while(!sentinel) for the second level of the 'tests'
      array to avoid that compiler complaint.
      Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
      17354d15
  6. 13 Apr, 2023 16 commits