Author: kevans Date: Wed Sep 25 19:22:03 2019 New Revision: 352712 URL: https://svnweb.freebsd.org/changeset/base/352712
Log: posix_spawn(3): handle potential signal issues with vfork Described in [1], signal handlers running in a vfork child have opportunities to corrupt the parent's state. Address this by adding a new rfork(2) flag, RFSPAWN, that has vfork(2) semantics but also resets signal handlers in the child during creation. x86 uses rfork_thread(3) instead of a direct rfork(2) because rfork with RFMEM/RFSPAWN cannot work when the return address is stored on the stack -- further information about this problem is described under RFMEM in the rfork(2) man page. Addressing this has been identified as a prerequisite to using posix_spawn in subprocess on FreeBSD [2]. [1] https://ewontfix.com/7/ [2] https://bugs.python.org/issue35823 Reviewed by: jilles, kib Differential Revision: https://reviews.freebsd.org/D19058 Modified: head/lib/libc/gen/posix_spawn.c head/lib/libc/sys/rfork.2 Modified: head/lib/libc/gen/posix_spawn.c ============================================================================== --- head/lib/libc/gen/posix_spawn.c Wed Sep 25 19:20:41 2019 (r352711) +++ head/lib/libc/gen/posix_spawn.c Wed Sep 25 19:22:03 2019 (r352712) @@ -194,43 +194,115 @@ process_file_actions(const posix_spawn_file_actions_t return (0); } +struct posix_spawn_args { + const char *path; + const posix_spawn_file_actions_t *fa; + const posix_spawnattr_t *sa; + char * const * argv; + char * const * envp; + int use_env_path; + int error; +}; + +#if defined(__i386__) || defined(__amd64__) +#define _RFORK_THREAD_STACK_SIZE 4096 +#endif + static int +_posix_spawn_thr(void *data) +{ + struct posix_spawn_args *psa; + char * const *envp; + + psa = data; + if (psa->sa != NULL) { + psa->error = process_spawnattr(*psa->sa); + if (psa->error) + _exit(127); + } + if (psa->fa != NULL) { + psa->error = process_file_actions(*psa->fa); + if (psa->error) + _exit(127); + } + envp = psa->envp != NULL ? psa->envp : environ; + if (psa->use_env_path) + _execvpe(psa->path, psa->argv, envp); + else + _execve(psa->path, psa->argv, envp); + psa->error = errno; + + /* This is called in such a way that it must not exit. */ + _exit(127); +} + +static int do_posix_spawn(pid_t *pid, const char *path, const posix_spawn_file_actions_t *fa, const posix_spawnattr_t *sa, char * const argv[], char * const envp[], int use_env_path) { + struct posix_spawn_args psa; pid_t p; - volatile int error = 0; +#ifdef _RFORK_THREAD_STACK_SIZE + char *stack; - p = vfork(); - switch (p) { - case -1: - return (errno); - case 0: - if (sa != NULL) { - error = process_spawnattr(*sa); - if (error) - _exit(127); - } - if (fa != NULL) { - error = process_file_actions(*fa); - if (error) - _exit(127); - } - if (use_env_path) - _execvpe(path, argv, envp != NULL ? envp : environ); - else - _execve(path, argv, envp != NULL ? envp : environ); - error = errno; - _exit(127); - default: - if (error != 0) - _waitpid(p, NULL, WNOHANG); - else if (pid != NULL) - *pid = p; - return (error); + stack = malloc(_RFORK_THREAD_STACK_SIZE); + if (stack == NULL) + return (ENOMEM); +#endif + psa.path = path; + psa.fa = fa; + psa.sa = sa; + psa.argv = argv; + psa.envp = envp; + psa.use_env_path = use_env_path; + psa.error = 0; + + /* + * Passing RFSPAWN to rfork(2) gives us effectively a vfork that drops + * non-ignored signal handlers. We'll fall back to the slightly less + * ideal vfork(2) if we get an EINVAL from rfork -- this should only + * happen with newer libc on older kernel that doesn't accept + * RFSPAWN. + */ +#ifdef _RFORK_THREAD_STACK_SIZE + /* + * x86 stores the return address on the stack, so rfork(2) cannot work + * as-is because the child would clobber the return address om the + * parent. Because of this, we must use rfork_thread instead while + * almost every other arch stores the return address in a register. + */ + p = rfork_thread(RFSPAWN, stack + _RFORK_THREAD_STACK_SIZE, + _posix_spawn_thr, &psa); + free(stack); +#else + p = rfork(RFSPAWN); + if (p == 0) + /* _posix_spawn_thr does not return */ + _posix_spawn_thr(&psa); +#endif + /* + * The above block should leave us in a state where we've either + * succeeded and we're ready to process the results, or we need to + * fallback to vfork() if the kernel didn't like RFSPAWN. + */ + + if (p == -1 && errno == EINVAL) { + p = vfork(); + if (p == 0) + /* _posix_spawn_thr does not return */ + _posix_spawn_thr(&psa); } + if (p == -1) + return (errno); + if (psa.error != 0) + /* Failed; ready to reap */ + _waitpid(p, NULL, WNOHANG); + else if (pid != NULL) + /* exec succeeded */ + *pid = p; + return (psa.error); } int Modified: head/lib/libc/sys/rfork.2 ============================================================================== --- head/lib/libc/sys/rfork.2 Wed Sep 25 19:20:41 2019 (r352711) +++ head/lib/libc/sys/rfork.2 Wed Sep 25 19:22:03 2019 (r352712) @@ -113,6 +113,9 @@ is passed, will use .Xr vfork 2 semantics but reset all signal actions in the child to default. +This flag is used by the +.Xr posix_spawn 3 +implementation in libc. .Pp If .Dv RFPROC _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
