svn commit: r353789 - in stable: 11/lib/libc/gen 11/lib/libc/sys 11/sys/kern 11/sys/sys 12/lib/libc/gen 12/lib/libc/sys 12/sys/kern 12/sys/sys
Kyle Evans
kevans at FreeBSD.org
Mon Oct 21 01:24:23 UTC 2019
Author: kevans
Date: Mon Oct 21 01:24:21 2019
New Revision: 353789
URL: https://svnweb.freebsd.org/changeset/base/353789
Log:
MFC r352711-r352712: Address posix_spawn(3) signal issues
r352711:
rfork(2): add RFSPAWN flag
When RFSPAWN is passed, rfork exhibits vfork(2) semantics but also resets
signal handlers in the child during creation to avoid a point of corruption
of parent state from the child.
This flag will be used by posix_spawn(3) to handle potential signal issues.
Reviewed by: jilles, kib
Differential Revision: https://reviews.freebsd.org/D19058
r352712:
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
Modified:
stable/11/lib/libc/gen/posix_spawn.c
stable/11/lib/libc/sys/rfork.2
stable/11/sys/kern/kern_fork.c
stable/11/sys/kern/kern_sig.c
stable/11/sys/sys/proc.h
stable/11/sys/sys/signalvar.h
stable/11/sys/sys/unistd.h
Directory Properties:
stable/11/ (props changed)
Changes in other areas also in this revision:
Modified:
stable/12/lib/libc/gen/posix_spawn.c
stable/12/lib/libc/sys/rfork.2
stable/12/sys/kern/kern_fork.c
stable/12/sys/kern/kern_sig.c
stable/12/sys/sys/proc.h
stable/12/sys/sys/signalvar.h
stable/12/sys/sys/unistd.h
Directory Properties:
stable/12/ (props changed)
Modified: stable/11/lib/libc/gen/posix_spawn.c
==============================================================================
--- stable/11/lib/libc/gen/posix_spawn.c Mon Oct 21 00:52:21 2019 (r353788)
+++ stable/11/lib/libc/gen/posix_spawn.c Mon Oct 21 01:24:21 2019 (r353789)
@@ -192,43 +192,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: stable/11/lib/libc/sys/rfork.2
==============================================================================
--- stable/11/lib/libc/sys/rfork.2 Mon Oct 21 00:52:21 2019 (r353788)
+++ stable/11/lib/libc/sys/rfork.2 Mon Oct 21 01:24:21 2019 (r353789)
@@ -5,7 +5,7 @@
.\"
.\" $FreeBSD$
.\"
-.Dd July 12, 2011
+.Dd September 25, 2019
.Dt RFORK 2
.Os
.Sh NAME
@@ -34,7 +34,9 @@ and open files.
The
.Fa flags
argument
-is the logical OR of some subset of:
+is either
+.Dv RFSPAWN
+or the logical OR of some subset of:
.Bl -tag -width ".Dv RFLINUXTHPN"
.It Dv RFPROC
If set a new process is created; otherwise changes affect the
@@ -103,6 +105,17 @@ This is intended to mimic certain Linux clone behaviou
File descriptors in a shared file descriptor table are kept
open until either they are explicitly closed
or all processes sharing the table exit.
+.Pp
+If
+.Dv RFSPAWN
+is passed,
+.Nm
+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
Modified: stable/11/sys/kern/kern_fork.c
==============================================================================
--- stable/11/sys/kern/kern_fork.c Mon Oct 21 00:52:21 2019 (r353788)
+++ stable/11/sys/kern/kern_fork.c Mon Oct 21 01:24:21 2019 (r353789)
@@ -170,10 +170,18 @@ sys_rfork(struct thread *td, struct rfork_args *uap)
/* Don't allow kernel-only flags. */
if ((uap->flags & RFKERNELONLY) != 0)
return (EINVAL);
+ /* RFSPAWN must not appear with others */
+ if ((uap->flags & RFSPAWN) != 0 && uap->flags != RFSPAWN)
+ return (EINVAL);
AUDIT_ARG_FFLAGS(uap->flags);
bzero(&fr, sizeof(fr));
- fr.fr_flags = uap->flags;
+ if ((uap->flags & RFSPAWN) != 0) {
+ fr.fr_flags = RFFDG | RFPROC | RFPPWAIT | RFMEM;
+ fr.fr_flags2 = FR2_DROPSIG_CAUGHT;
+ } else {
+ fr.fr_flags = uap->flags;
+ }
fr.fr_pidp = &pid;
error = fork1(td, &fr);
if (error == 0) {
@@ -532,6 +540,11 @@ do_fork(struct thread *td, struct fork_req *fr, struct
} else {
sigacts_copy(newsigacts, p1->p_sigacts);
p2->p_sigacts = newsigacts;
+ if ((fr->fr_flags2 & FR2_DROPSIG_CAUGHT) != 0) {
+ mtx_lock(&p2->p_sigacts->ps_mtx);
+ sig_drop_caught(p2);
+ mtx_unlock(&p2->p_sigacts->ps_mtx);
+ }
}
if (fr->fr_flags & RFTSIGZMB)
Modified: stable/11/sys/kern/kern_sig.c
==============================================================================
--- stable/11/sys/kern/kern_sig.c Mon Oct 21 00:52:21 2019 (r353788)
+++ stable/11/sys/kern/kern_sig.c Mon Oct 21 01:24:21 2019 (r353789)
@@ -987,12 +987,7 @@ execsigs(struct proc *p)
PROC_LOCK_ASSERT(p, MA_OWNED);
ps = p->p_sigacts;
mtx_lock(&ps->ps_mtx);
- while (SIGNOTEMPTY(ps->ps_sigcatch)) {
- sig = sig_ffs(&ps->ps_sigcatch);
- sigdflt(ps, sig);
- if ((sigprop(sig) & SA_IGNORE) != 0)
- sigqueue_delete_proc(p, sig);
- }
+ sig_drop_caught(p);
/*
* As CloudABI processes cannot modify signal handlers, fully
@@ -3843,4 +3838,21 @@ sigacts_shared(struct sigacts *ps)
{
return (ps->ps_refcnt > 1);
+}
+
+void
+sig_drop_caught(struct proc *p)
+{
+ int sig;
+ struct sigacts *ps;
+
+ ps = p->p_sigacts;
+ PROC_LOCK_ASSERT(p, MA_OWNED);
+ mtx_assert(&ps->ps_mtx, MA_OWNED);
+ while (SIGNOTEMPTY(ps->ps_sigcatch)) {
+ sig = sig_ffs(&ps->ps_sigcatch);
+ sigdflt(ps, sig);
+ if ((sigprop(sig) & SA_IGNORE) != 0)
+ sigqueue_delete_proc(p, sig);
+ }
}
Modified: stable/11/sys/sys/proc.h
==============================================================================
--- stable/11/sys/sys/proc.h Mon Oct 21 00:52:21 2019 (r353788)
+++ stable/11/sys/sys/proc.h Mon Oct 21 01:24:21 2019 (r353789)
@@ -983,6 +983,8 @@ struct fork_req {
int *fr_pd_fd;
int fr_pd_flags;
struct filecaps *fr_pd_fcaps;
+ int fr_flags2;
+#define FR2_DROPSIG_CAUGHT 0x00001 /* Drop caught non-DFL signals */
};
/*
Modified: stable/11/sys/sys/signalvar.h
==============================================================================
--- stable/11/sys/sys/signalvar.h Mon Oct 21 00:52:21 2019 (r353788)
+++ stable/11/sys/sys/signalvar.h Mon Oct 21 01:24:21 2019 (r353789)
@@ -379,6 +379,7 @@ void sigacts_copy(struct sigacts *dest, struct sigacts
void sigacts_free(struct sigacts *ps);
struct sigacts *sigacts_hold(struct sigacts *ps);
int sigacts_shared(struct sigacts *ps);
+void sig_drop_caught(struct proc *p);
void sigexit(struct thread *td, int sig) __dead2;
int sigev_findtd(struct proc *p, struct sigevent *sigev, struct thread **);
int sig_ffs(sigset_t *set);
Modified: stable/11/sys/sys/unistd.h
==============================================================================
--- stable/11/sys/sys/unistd.h Mon Oct 21 00:52:21 2019 (r353788)
+++ stable/11/sys/sys/unistd.h Mon Oct 21 01:24:21 2019 (r353789)
@@ -186,11 +186,14 @@
#define RFTSIGNUM(flags) (((flags) >> RFTSIGSHIFT) & RFTSIGMASK)
#define RFTSIGFLAGS(signum) ((signum) << RFTSIGSHIFT)
#define RFPROCDESC (1<<28) /* return a process descriptor */
-#define RFPPWAIT (1<<31) /* parent sleeps until child exits (vfork) */
+/* kernel: parent sleeps until child exits (vfork) */
+#define RFPPWAIT (1<<31)
+/* user: vfork(2) semantics, clear signals */
+#define RFSPAWN (1U<<31)
#define RFFLAGS (RFFDG | RFPROC | RFMEM | RFNOWAIT | RFCFDG | \
RFTHREAD | RFSIGSHARE | RFLINUXTHPN | RFSTOPPED | RFHIGHPID | RFTSIGZMB | \
- RFPROCDESC | RFPPWAIT)
-#define RFKERNELONLY (RFSTOPPED | RFHIGHPID | RFPPWAIT | RFPROCDESC)
+ RFPROCDESC | RFSPAWN | RFPPWAIT)
+#define RFKERNELONLY (RFSTOPPED | RFHIGHPID | RFPROCDESC)
#endif /* __BSD_VISIBLE */
More information about the svn-src-stable-11
mailing list