fix public clone function to be safe and usable by applications
authorRich Felker <dalias@aerifal.cx>
Wed, 31 May 2023 16:04:06 +0000 (12:04 -0400)
committerRich Felker <dalias@aerifal.cx>
Thu, 1 Jun 2023 20:15:38 +0000 (16:15 -0400)
the clone() function has been effectively unusable since it was added,
due to producing a child process with inconsistent state. in
particular, the child process's thread structure still contains the
tid, thread list pointers, thread count, and robust list for the
parent. this will cause malfunction in interfaces that attempt to use
the tid or thread list, some of which are specified to be
async-signal-safe.

this patch attempts to make clone() consistent in a _Fork-like sense.
as in _Fork, when the parent process is multi-threaded, the child
process inherits an async-signal context where it cannot call
AS-unsafe functions, but its context is now intended to be safe for
calling AS-safe functions. making clone fork-like would also be a
future option, if it turns out that this is what makes sense to
applications, but it's not done at this time because the changes would
be more invasive.

in the case where the CLONE_VM flag is used, clone is only vfork-like,
not _Fork-like. in particular, the child will see itself as having the
parent's tid, and cannot safely call any libc functions but one of the
exec family or _exit.

handling of flags and variadic arguments is also changed so that
arguments are only consumed with flags that indicate their presence,
and so that flags which produce an inconsistent state are disallowed
(reported as EINVAL). in particular, all libc functions carry a
contract that they are only callable with ABI requirements met, which
includes having a valid thread pointer to a thread structure that's
unique within the process, and whose contents are opaque and only able
to be setup internally by the implementation. the only way for an
application to use flags that violate these requirements without
executing any libc code is to perform the syscall from
application-provided asm.

src/internal/fork_impl.h
src/linux/clone.c
src/process/_Fork.c

index 354e733..f995fce 100644 (file)
@@ -17,3 +17,5 @@ extern hidden volatile int *const __vmlock_lockptr;
 hidden void __malloc_atfork(int);
 hidden void __ldso_atfork(int);
 hidden void __pthread_key_atfork(int);
+
+hidden void __post_Fork(int);
index 8c1af7d..257c1ce 100644 (file)
@@ -4,18 +4,62 @@
 #include <sched.h>
 #include "pthread_impl.h"
 #include "syscall.h"
+#include "lock.h"
+#include "fork_impl.h"
+
+struct clone_start_args {
+       int (*func)(void *);
+       void *arg;
+       sigset_t sigmask;
+};
+
+static int clone_start(void *arg)
+{
+       struct clone_start_args *csa = arg;
+       __post_Fork(0);
+       __restore_sigs(&csa->sigmask);
+       return csa->func(csa->arg);
+}
 
 int clone(int (*func)(void *), void *stack, int flags, void *arg, ...)
 {
+       struct clone_start_args csa;
        va_list ap;
-       pid_t *ptid, *ctid;
-       void  *tls;
+       pid_t *ptid = 0, *ctid = 0;
+       void  *tls = 0;
+
+       /* Flags that produce an invalid thread/TLS state are disallowed. */
+       int badflags = CLONE_THREAD | CLONE_SETTLS | CLONE_CHILD_CLEARTID;
+
+       if ((flags & badflags) || !stack)
+               return __syscall_ret(-EINVAL);
 
        va_start(ap, arg);
-       ptid = va_arg(ap, pid_t *);
-       tls  = va_arg(ap, void *);
-       ctid = va_arg(ap, pid_t *);
+       if (flags & (CLONE_PIDFD | CLONE_PARENT_SETTID | CLONE_CHILD_SETTID))
+               ptid = va_arg(ap, pid_t *);
+       if (flags & CLONE_CHILD_SETTID) {
+               tls = va_arg(ap, void *);
+               ctid = va_arg(ap, pid_t *);
+       }
        va_end(ap);
 
-       return __syscall_ret(__clone(func, stack, flags, arg, ptid, tls, ctid));
+       /* If CLONE_VM is used, it's impossible to give the child a consistent
+        * thread structure. In this case, the best we can do is assume the
+        * caller is content with an extremely restrictive execution context
+        * like the one vfork() would provide. */
+       if (flags & CLONE_VM) return __syscall_ret(
+               __clone(func, stack, flags, arg, ptid, tls, ctid));
+
+       __block_all_sigs(&csa.sigmask);
+       LOCK(__abort_lock);
+
+       /* Setup the a wrapper start function for the child process to do
+        * mimic _Fork in producing a consistent execution state. */
+       csa.func = func;
+       csa.arg = arg;
+       int ret = __clone(clone_start, stack, flags, &csa, ptid, tls, ctid);
+
+       __post_Fork(ret);
+       __restore_sigs(&csa.sigmask);
+       return __syscall_ret(ret);
 }
index e765086..9c07792 100644 (file)
@@ -5,21 +5,13 @@
 #include "lock.h"
 #include "pthread_impl.h"
 #include "aio_impl.h"
+#include "fork_impl.h"
 
 static void dummy(int x) { }
 weak_alias(dummy, __aio_atfork);
 
-pid_t _Fork(void)
+void __post_Fork(int ret)
 {
-       pid_t ret;
-       sigset_t set;
-       __block_all_sigs(&set);
-       LOCK(__abort_lock);
-#ifdef SYS_fork
-       ret = __syscall(SYS_fork);
-#else
-       ret = __syscall(SYS_clone, SIGCHLD, 0);
-#endif
        if (!ret) {
                pthread_t self = __pthread_self();
                self->tid = __syscall(SYS_set_tid_address, &__thread_list_lock);
@@ -32,6 +24,20 @@ pid_t _Fork(void)
        }
        UNLOCK(__abort_lock);
        if (!ret) __aio_atfork(1);
+}
+
+pid_t _Fork(void)
+{
+       pid_t ret;
+       sigset_t set;
+       __block_all_sigs(&set);
+       LOCK(__abort_lock);
+#ifdef SYS_fork
+       ret = __syscall(SYS_fork);
+#else
+       ret = __syscall(SYS_clone, SIGCHLD, 0);
+#endif
+       __post_Fork(ret);
        __restore_sigs(&set);
        return __syscall_ret(ret);
 }