svn commit: r352712 - in head/lib/libc: gen sys
Kyle Evans
kevans at FreeBSD.org
Wed Sep 25 19:22:04 UTC 2019
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
More information about the svn-src-all
mailing list