linux-kernel - [PATCH] Fix perf test data race for tsan build

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-next>] [day] [month] [year] [list]

Message-Id: <20190710221620.211059-1-nums@google.com>
Date:   Wed, 10 Jul 2019 15:16:20 -0700
From:   Numfor Mbiziwo-Tiapo <nums@...gle.com>
To:     peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
        alexander.shishkin@...ux.intel.com, jolsa@...hat.com,
        namhyung@...nel.org, songliubraving@...com, mbd@...com
Cc:     linux-kernel@...r.kernel.org, irogers@...gle.com,
        eranian@...gle.com, Numfor Mbiziwo-Tiapo <nums@...gle.com>
Subject: [PATCH] Fix perf test data race for tsan build

The tsan build of perf gives a data race warning in
mmap-thread-lookup.c when perf test is run on our local machines.
This is because there is no lock around the "go_away" variable in
"mmap-thread-lookup.c".

The warning is difficult to reproduce from the tip branch and we
suspect it has something to do with tsan working differently
depending on the clang version.

The warning can be silenced by adding locks around the variable
but it is better practice to make the variable atomic and use
the atomic_set and atomic_read functions. This does not actually
silence the warning because tsan doesn't recognize the atomic
functions as thread safe, but in actuality it should prevent a
data race.

Signed-off-by: Numfor Mbiziwo-Tiapo <nums@...gle.com>
---
 tools/perf/tests/mmap-thread-lookup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/mmap-thread-lookup.c b/tools/perf/tests/mmap-thread-lookup.c
index 5ede9b561d32..f4cbb3d92a46 100644
--- a/tools/perf/tests/mmap-thread-lookup.c
+++ b/tools/perf/tests/mmap-thread-lookup.c
@@ -17,7 +17,7 @@
 
 #define THREADS 4
 
-static int go_away;
+static atomic_t go_away;
 
 struct thread_data {
 	pthread_t	pt;
@@ -64,7 +64,7 @@ static void *thread_fn(void *arg)
 		return NULL;
 	}
 
-	while (!go_away) {
+	while (!atomic_read(&go_away)) {
 		/* Waiting for main thread to kill us. */
 		usleep(100);
 	}
@@ -98,7 +98,7 @@ static int threads_create(void)
 	struct thread_data *td0 = &threads[0];
 	int i, err = 0;
 
-	go_away = 0;
+	atomic_set(&go_away, 0);
 
 	/* 0 is main thread */
 	if (thread_init(td0))
@@ -118,7 +118,7 @@ static int threads_destroy(void)
 	/* cleanup the main thread */
 	munmap(td0->map, page_size);
 
-	go_away = 1;
+	atomic_set(&go_away, 1);
 
 	for (i = 1; !err && i < THREADS; i++)
 		err = pthread_join(threads[i].pt, NULL);
-- 
2.22.0.410.gd8fdbe21b5-goog