[patch] 0001-libsandbox-drop-synchronization-from-vfork-and-fork.patch

0001-libsandbox-drop-synchronization-from-vfork-and-fork.patch (text/plain), 3.14 KB, created by Sergei Trofimovich (RETIRED) on 2021-07-22 07:29:21 UTC

From c7611333396661579314761223e2bbb3aad1e19a Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <[e-mail]>
Date: Thu, 22 Jul 2021 08:13:24 +0100
Subject: [PATCH] libsandbox: drop synchronization from vfork() and fork()

Change f43378e143 "libsandbox: implement vfork() via fork()"
chaged vfork() implementation to fork() without removing
sandbox's global state synchronization.

As a result early _atfork() handlers in new process and new
address space run sandbox before sb_unlock() is executed.

This causes deadlocks in complicated libraries like clang's libomp:

- [parent] sb_lock()
- [parent] fork()
- [child]  atfork() / .init
  - shm_open() [-> sandbox_interceptor]
  - open_DEFAULT [sandbox]
  - sb_init() [sandbox]
  - sb_lock() [deadlock]

Locking was only needed for real vfork() handling. As we don't
use it anymore we can rely on fork() unsharing of any shared state.

Fixes: f43378e143 ("libsandbox: implement vfork() via fork()")
Reported-by: Xi
Bug: https://bugs.gentoo.org/801259
Signed-off-by: Sergei Trofimovich <[e-mail]>
---
 libsandbox/wrapper-funcs/fork.c  |  8 --------
 libsandbox/wrapper-funcs/vfork.c | 14 +++++---------
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/libsandbox/wrapper-funcs/fork.c b/libsandbox/wrapper-funcs/fork.c
index c250018..f0d4b59 100644
--- a/libsandbox/wrapper-funcs/fork.c
+++ b/libsandbox/wrapper-funcs/fork.c
@@ -5,20 +5,12 @@
  * Licensed under the GPL-2
  */
 
-/* We're only wrapping fork() as a poor man's pthread_atfork().  That would
- * require dedicated linkage against libpthread.  So here we force the locks
- * to a consistent state before forking. #263657
- */
-
 #define WRAPPER_ARGS_PROTO
 #define WRAPPER_ARGS
 #define WRAPPER_SAFE() 0
 #define WRAPPER_PRE_CHECKS() \
 ({ \
-	/* pthread_atfork(sb_lock, sb_unlock, sb_unlock); */ \
-	sb_lock(); \
 	result = SB_HIDDEN_FUNC(WRAPPER_NAME)(WRAPPER_ARGS_FULL); \
-	sb_unlock(); \
 	false; \
 })
 #include "__wrapper_simple.c"
diff --git a/libsandbox/wrapper-funcs/vfork.c b/libsandbox/wrapper-funcs/vfork.c
index b28e74c..190e953 100644
--- a/libsandbox/wrapper-funcs/vfork.c
+++ b/libsandbox/wrapper-funcs/vfork.c
@@ -5,13 +5,12 @@
  * Licensed under the GPL-2
  */
 
-/* We're only wrapping vfork() as a poor man's pthread_atfork().  That would
- * require dedicated linkage against libpthread.  So here we force the locks
- * to a consistent state before forking.
- *
- * We also implement vfork() as fork() because sandbox does not meet vfork()
- * requirements bet ween vfork()/exec("some-static-bianary") because we launch
+/* We implement vfork() as fork() because sandbox does not meet vfork()
+ * requirements between vfork()/exec("some-static-bianary") because we launch
  * ptrace in the middle.
+ *
+ * vfork() can also cause races in multithreaded apps in sandbox code.
+ * fork() does not as it unstares address space.
  */
 
 #define WRAPPER_ARGS_PROTO
@@ -19,10 +18,7 @@
 #define WRAPPER_SAFE() 0
 #define WRAPPER_PRE_CHECKS() \
 ({ \
-	/* pthread_atfork(sb_lock, sb_unlock, sb_unlock); */ \
-	sb_lock(); \
 	result = sb_unwrapped_fork_DEFAULT(WRAPPER_ARGS_FULL); \
-	sb_unlock(); \
 	false; \
 })
 #include "__wrapper_simple.c"
-- 
2.32.0