tsan: don't start background thread after clone
Start the background thread only after fork, but not after clone.
For fork we did this always and it's known to work (or user code has adopted).
But if we do this for the new clone interceptor some code (sandbox2) fails.
So model we used to do for years and don't start the background thread after clone.
Reviewed By: melver
Differential Revision: https://reviews.llvm.org/D113744
GitOrigin-RevId: e91595bf948a23bb44a47854321aa80307a48fd1
diff --git a/lib/tsan/rtl/tsan_interceptors_posix.cpp b/lib/tsan/rtl/tsan_interceptors_posix.cpp
index c7dcd07..9a85ee0 100644
--- a/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -2189,7 +2189,7 @@
return;
ThreadState *thr = cur_thread();
const uptr pc = StackTrace::GetCurrentPc();
- ForkChildAfter(thr, pc);
+ ForkChildAfter(thr, pc, true);
FdOnFork(thr, pc);
}
@@ -2222,7 +2222,12 @@
auto wrapper = +[](void *p) -> int {
auto *thr = cur_thread();
uptr pc = GET_CURRENT_PC();
- ForkChildAfter(thr, pc);
+ // Start the background thread for fork, but not for clone.
+ // For fork we did this always and it's known to work (or user code has
+ // adopted). But if we do this for the new clone interceptor some code
+ // (sandbox2) fails. So model we used to do for years and don't start the
+ // background thread after clone.
+ ForkChildAfter(thr, pc, false);
FdOnFork(thr, pc);
auto *arg = static_cast<Arg *>(p);
return arg->fn(arg->arg);
@@ -2570,7 +2575,7 @@
ThreadState *thr = cur_thread();
if (pid == 0) {
// child
- ForkChildAfter(thr, pc);
+ ForkChildAfter(thr, pc, true);
FdOnFork(thr, pc);
} else if (pid > 0) {
// parent
diff --git a/lib/tsan/rtl/tsan_rtl.cpp b/lib/tsan/rtl/tsan_rtl.cpp
index 6e57d4a..46dec04 100644
--- a/lib/tsan/rtl/tsan_rtl.cpp
+++ b/lib/tsan/rtl/tsan_rtl.cpp
@@ -506,7 +506,8 @@
ctx->thread_registry.Unlock();
}
-void ForkChildAfter(ThreadState *thr, uptr pc) NO_THREAD_SAFETY_ANALYSIS {
+void ForkChildAfter(ThreadState *thr, uptr pc,
+ bool start_thread) NO_THREAD_SAFETY_ANALYSIS {
thr->suppress_reports--; // Enabled in ForkBefore.
thr->ignore_interceptors--;
ScopedErrorReportLock::Unlock();
@@ -518,7 +519,8 @@
VPrintf(1, "ThreadSanitizer: forked new process with pid %d,"
" parent had %d threads\n", (int)internal_getpid(), (int)nthread);
if (nthread == 1) {
- StartBackgroundThread();
+ if (start_thread)
+ StartBackgroundThread();
} else {
// We've just forked a multi-threaded process. We cannot reasonably function
// after that (some mutexes may be locked before fork). So just enable
diff --git a/lib/tsan/rtl/tsan_rtl.h b/lib/tsan/rtl/tsan_rtl.h
index 089144c..eab8370 100644
--- a/lib/tsan/rtl/tsan_rtl.h
+++ b/lib/tsan/rtl/tsan_rtl.h
@@ -440,7 +440,7 @@
void ForkBefore(ThreadState *thr, uptr pc);
void ForkParentAfter(ThreadState *thr, uptr pc);
-void ForkChildAfter(ThreadState *thr, uptr pc);
+void ForkChildAfter(ThreadState *thr, uptr pc, bool start_thread);
void ReportRace(ThreadState *thr);
bool OutputReport(ThreadState *thr, const ScopedReport &srep);
diff --git a/test/tsan/Linux/clone_setns.cpp b/test/tsan/Linux/clone_setns.cpp
new file mode 100644
index 0000000..25e3034
--- /dev/null
+++ b/test/tsan/Linux/clone_setns.cpp
@@ -0,0 +1,39 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+
+// The test models how sandbox2 unshares user namespace after clone:
+// https://github.com/google/sandboxed-api/blob/c95837a6c131fbdf820db352a97d54fcbcbde6c0/sandboxed_api/sandbox2/forkserver.cc#L249
+// which works only in sigle-threaded processes.
+
+#include "../test.h"
+#include <errno.h>
+#include <sched.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+static int cloned(void *arg) {
+ if (unshare(CLONE_NEWUSER)) {
+ fprintf(stderr, "unshare failed: %d\n", errno);
+ exit(1);
+ }
+ exit(0);
+ return 0;
+}
+
+int main() {
+ char stack[64 << 10] __attribute__((aligned(64)));
+ int pid = clone(cloned, stack + sizeof(stack), SIGCHLD, 0);
+ if (pid == -1) {
+ fprintf(stderr, "failed to clone: %d\n", errno);
+ exit(1);
+ }
+ int status = 0;
+ while (wait(&status) != pid) {
+ }
+ if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+ fprintf(stderr, "child failed: %d\n", status);
+ exit(1);
+ }
+ fprintf(stderr, "DONE\n");
+}
+
+// CHECK: DONE
