perf test: Fix maps use after put
authorIan Rogers <irogers@google.com>
Thu, 20 Apr 2023 03:04:30 +0000 (20:04 -0700)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Thu, 20 Apr 2023 11:23:52 +0000 (08:23 -0300)
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: Ian 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.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
tools/perf/tests/thread-maps-share.c

index 75ce8ae..858e725 100644 (file)
@@ -76,16 +76,16 @@ static int test__thread_maps_share(struct test_suite *test __maybe_unused, int s
        TEST_ASSERT_VAL("maps don't match", RC_CHK_ACCESS(other_maps) == RC_CHK_ACCESS(other_leader->maps));
 
        /* release thread group */
-       thread__put(leader);
+       thread__put(t3);
        TEST_ASSERT_EQUAL("wrong refcnt", refcount_read(maps__refcnt(maps)), 3);
 
-       thread__put(t1);
+       thread__put(t2);
        TEST_ASSERT_EQUAL("wrong refcnt", refcount_read(maps__refcnt(maps)), 2);
 
-       thread__put(t2);
+       thread__put(t1);
        TEST_ASSERT_EQUAL("wrong refcnt", refcount_read(maps__refcnt(maps)), 1);
 
-       thread__put(t3);
+       thread__put(leader);
 
        /* release other group  */
        thread__put(other_leader);