subprocess-posix: fewer ifdefs and fix ASan

Author: ruihe774

osdep/compiler.h

@@ -16,11 +16,13 @@
 #define MP_FALLTHROUGH __attribute__((fallthrough))
 #define MP_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
 #define MP_UNUSED __attribute__((unused))
+#define MP_NO_ASAN __attribute__((no_sanitize("address")))
 #else
 #define MP_NORETURN
 #define MP_FALLTHROUGH do {} while (0)
 #define MP_WARN_UNUSED_RESULT
 #define MP_UNUSED
+#define MP_NO_ASAN
 #endif
 
 #if defined(__STDC_VERSION__) && (__STDC_VERSION__ < 202311L) && !defined(thread_local)

osdep/subprocess-posix.c

@@ -74,11 +74,12 @@ static int as_execvpe(const char *path, const char *file, char *const argv[],
     return -1;
 }
 
-#if !HAVE_RFORK
 // In the child process, resets the signal mask to defaults. Also clears any
 // signal handlers first so nothing funny happens.
 static void reset_signals_child(void)
 {
+#if !HAVE_RFORK
+    // RFSPAWN has reset all signal actions in the child to default already
     struct sigaction sa = { 0 };
     sigset_t sigmask;
     sa.sa_handler = SIG_DFL;
@@ -87,29 +88,33 @@ static void reset_signals_child(void)
     for (int nr = 1; nr <= SIGNAL_MAX; nr++)
         sigaction(nr, &sa, NULL);
     sigprocmask(SIG_SETMASK, &sigmask, NULL);
-}
 #endif
+}
 
 struct child_args {
     const char *path;
     struct mp_subprocess_opts *opts;
     int *src_fds;
     void *child_stack;
     int pipe_end;
+    bool failed;
     bool detach;
 };
 
 static pid_t spawn_process_inner(const char *path, struct mp_subprocess_opts *opts,
                                  int src_fds[], bool detach, void *stacks[]);
 
-static int child_main(void* args)
+// This function is called from a clone(CLONE_VM)/rfork_thread context where
+// the child shares the parent's address space. Use MP_NO_ASAN to avoid false
+// positives from ASan when the child writes to shared memory.
+MP_NO_ASAN static int child_main(void* args)
 {
     struct child_args *child_args = args;
     const char *path = child_args->path;
     struct mp_subprocess_opts *opts = child_args->opts;
     int *src_fds = child_args->src_fds;
     void *child_stack = child_args->child_stack;
-    int *pipe_end = &child_args->pipe_end;
+    int pipe_end = child_args->pipe_end;
     bool detach = child_args->detach;
 
     if (detach) {
@@ -119,10 +124,7 @@ static int child_main(void* args)
         return 0;
     }
 
-    // RFSPAWN has reset all signal actions in the child to default
-#if !HAVE_RFORK
     reset_signals_child();
-#endif
 
     for (int n = 0; n < opts->num_fds; n++) {
         if (src_fds[n] == opts->fds[n].fd) {
@@ -140,35 +142,35 @@ static int child_main(void* args)
     as_execvpe(path, opts->exe, opts->args, opts->env ? opts->env : environ);
 
 child_failed:
-#if HAVE_CLONE || HAVE_RFORK
-    *pipe_end = 1;
-#else
-    (void)write(*pipe_end, &(char){1}, 1); // shouldn't be able to fail
-#endif
+    child_args->failed = true;
+    if (pipe_end >= 0)
+        (void)write(pipe_end, &(char){1}, 1); // shouldn't be able to fail
     return 1;
 }
 
 static pid_t spawn_process_inner(const char *path, struct mp_subprocess_opts *opts,
                                  int src_fds[], bool detach, void *stacks[])
 {
     pid_t fres = 0;
-    int r;
+    int r = 0;
 
     struct child_args child_args = {
         .path = path,
         .opts = opts,
         .src_fds = src_fds,
         .child_stack = stacks[1],
-        .pipe_end = 0,
+        .pipe_end = -1,
+        .failed = false,
         .detach = detach,
     };
 
+    int p[2] = {-1, -1};
+
 #if HAVE_RFORK
     fres = rfork_thread(RFSPAWN, stacks[0], child_main, &child_args);
 #elif HAVE_CLONE
     fres = clone(child_main, stacks[0], CLONE_VM | CLONE_VFORK | SIGCHLD, &child_args);
 #else
-    int p[2] = {-1, -1};
     // We setup a communication pipe to signal failure. Since the child calls
     // exec() and becomes the calling process, we don't know if or when the
     // child process successfully ran exec() just from the PID.
@@ -191,9 +193,7 @@ static pid_t spawn_process_inner(const char *path, struct mp_subprocess_opts *op
         goto done;
     }
 
-#if HAVE_CLONE || HAVE_RFORK
-    r = child_args.pipe_end;
-#else
+#if !HAVE_CLONE && !HAVE_RFORK
     if (fres == 0) {
         _exit(child_main(&child_args));
     }
@@ -206,34 +206,32 @@ static pid_t spawn_process_inner(const char *path, struct mp_subprocess_opts *op
 #endif
 
     // If exec()ing child failed, collect it immediately.
-    if (detach || r != 0) {
+    if (detach || child_args.failed || r != 0) {
         int child_status = 0;
         while (waitpid(fres, &child_status, 0) < 0 && errno == EINTR) {}
         if (r != 0 || !WIFEXITED(child_status) || WEXITSTATUS(child_status) != 0)
             fres = 0;
     }
 
 done:
-#if !HAVE_CLONE && !HAVE_RFORK
     SAFE_CLOSE(p[0]);
     SAFE_CLOSE(p[1]);
-#endif
 
     return fres;
 }
 
 // Returns 0 on any error, valid PID on success.
-// This function must be async-signal-safe, as it may be called from a fork().
 static pid_t spawn_process(const char *path, struct mp_subprocess_opts *opts,
                            int src_fds[])
 {
     bool detach = opts->detach;
     void *stacks[2];
+    void *ctx = NULL;
 
 #if HAVE_CLONE || HAVE_RFORK
     // pre-allocate stacks so we can be async-signal-safe in spawn_process_inner()
     const size_t stack_size = 0x8000;
-    void *ctx = talloc_new(NULL);
+    ctx = talloc_new(NULL);
     // stack should be aligned to 16 bytes, which is guaranteed by malloc
     stacks[0] = (int8_t*)talloc_size(ctx, stack_size) + stack_size;
     if (detach) stacks[1] = (int8_t*)talloc_size(ctx, stack_size) + stack_size;
@@ -252,9 +250,7 @@ static pid_t spawn_process(const char *path, struct mp_subprocess_opts *opts,
     pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
 #endif
 
-#if HAVE_CLONE || HAVE_RFORK
     talloc_free(ctx);
-#endif
 
     return fres;
 }