Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys - Untangle spawn_return by splitting it up to sub-functi...
details: https://anonhg.NetBSD.org/src/rev/66f96900b094
branches: trunk
changeset: 850505:66f96900b094
user: christos <christos%NetBSD.org@localhost>
date: Sun Apr 05 20:53:17 2020 +0000
description:
- Untangle spawn_return by splitting it up to sub-functions.
- Merge the eventswitch parent notification code which was copied in two
places (eventswitchchild)
- Fix bugs in the eventswitch parent notification code:
1. p_slflags should be accessed holding both proc_lock and p->p_lock
2. p->p_opptr can be NULL if the parent was PSL_CHTRACED and exited.
Fixes random crashes the posix_spawn_kill_spawner unit test which tried
to dereference a NULL pptr.
diffstat:
sys/kern/kern_exec.c | 330 +++++++++++++++++++++++++-------------------------
sys/kern/kern_fork.c | 14 +-
sys/kern/kern_sig.c | 17 ++-
sys/sys/signalvar.h | 4 +-
4 files changed, 186 insertions(+), 179 deletions(-)
diffs (truncated from 479 to 300 lines):
diff -r a14897cef29d -r 66f96900b094 sys/kern/kern_exec.c
--- a/sys/kern/kern_exec.c Sun Apr 05 19:48:27 2020 +0000
+++ b/sys/kern/kern_exec.c Sun Apr 05 20:53:17 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_exec.c,v 1.493 2020/02/23 22:14:03 ad Exp $ */
+/* $NetBSD: kern_exec.c,v 1.494 2020/04/05 20:53:17 christos Exp $ */
/*-
* Copyright (c) 2008, 2019, 2020 The NetBSD Foundation, Inc.
@@ -62,7 +62,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.493 2020/02/23 22:14:03 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.494 2020/04/05 20:53:17 christos Exp $");
#include "opt_exec.h"
#include "opt_execfmt.h"
@@ -2062,6 +2062,156 @@
kmem_free(data, sizeof(*data));
}
+static int
+handle_posix_spawn_file_actions(struct posix_spawn_file_actions *actions)
+{
+ struct lwp *l = curlwp;
+ register_t retval;
+ int error, newfd;
+
+ if (actions == NULL)
+ return 0;
+
+ for (size_t i = 0; i < actions->len; i++) {
+ const struct posix_spawn_file_actions_entry *fae =
+ &actions->fae[i];
+ switch (fae->fae_action) {
+ case FAE_OPEN:
+ if (fd_getfile(fae->fae_fildes) != NULL) {
+ error = fd_close(fae->fae_fildes);
+ if (error)
+ return error;
+ }
+ error = fd_open(fae->fae_path, fae->fae_oflag,
+ fae->fae_mode, &newfd);
+ if (error)
+ return error;
+ if (newfd != fae->fae_fildes) {
+ error = dodup(l, newfd,
+ fae->fae_fildes, 0, &retval);
+ if (fd_getfile(newfd) != NULL)
+ fd_close(newfd);
+ }
+ break;
+ case FAE_DUP2:
+ error = dodup(l, fae->fae_fildes,
+ fae->fae_newfildes, 0, &retval);
+ break;
+ case FAE_CLOSE:
+ if (fd_getfile(fae->fae_fildes) == NULL) {
+ return EBADF;
+ }
+ error = fd_close(fae->fae_fildes);
+ break;
+ }
+ if (error)
+ return error;
+ }
+ return 0;
+}
+
+static int
+handle_posix_spawn_attrs(struct posix_spawnattr *attrs, struct proc *parent)
+{
+ struct sigaction sigact;
+ int error;
+ struct proc *p = curproc;
+ struct lwp *l = curlwp;
+
+ if (attrs == NULL)
+ return 0;
+
+ memset(&sigact, 0, sizeof(sigact));
+ sigact._sa_u._sa_handler = SIG_DFL;
+ sigact.sa_flags = 0;
+
+ /*
+ * set state to SSTOP so that this proc can be found by pid.
+ * see proc_enterprp, do_sched_setparam below
+ */
+ mutex_enter(proc_lock);
+ /*
+ * p_stat should be SACTIVE, so we need to adjust the
+ * parent's p_nstopchild here. For safety, just make
+ * we're on the good side of SDEAD before we adjust.
+ */
+ int ostat = p->p_stat;
+ KASSERT(ostat < SSTOP);
+ p->p_stat = SSTOP;
+ p->p_waited = 0;
+ p->p_pptr->p_nstopchild++;
+ mutex_exit(proc_lock);
+
+ /* Set process group */
+ if (attrs->sa_flags & POSIX_SPAWN_SETPGROUP) {
+ pid_t mypid = p->p_pid;
+ pid_t pgrp = attrs->sa_pgroup;
+
+ if (pgrp == 0)
+ pgrp = mypid;
+
+ error = proc_enterpgrp(parent, mypid, pgrp, false);
+ if (error)
+ goto out;
+ }
+
+ /* Set scheduler policy */
+ if (attrs->sa_flags & POSIX_SPAWN_SETSCHEDULER)
+ error = do_sched_setparam(p->p_pid, 0, attrs->sa_schedpolicy,
+ &attrs->sa_schedparam);
+ else if (attrs->sa_flags & POSIX_SPAWN_SETSCHEDPARAM) {
+ error = do_sched_setparam(parent->p_pid, 0,
+ SCHED_NONE, &attrs->sa_schedparam);
+ }
+ if (error)
+ goto out;
+
+ /* Reset user ID's */
+ if (attrs->sa_flags & POSIX_SPAWN_RESETIDS) {
+ error = do_setresuid(l, -1, kauth_cred_getgid(l->l_cred), -1,
+ ID_E_EQ_R | ID_E_EQ_S);
+ if (error)
+ return error;
+ error = do_setresuid(l, -1, kauth_cred_getuid(l->l_cred), -1,
+ ID_E_EQ_R | ID_E_EQ_S);
+ if (error)
+ goto out;
+ }
+
+ /* Set signal masks/defaults */
+ if (attrs->sa_flags & POSIX_SPAWN_SETSIGMASK) {
+ mutex_enter(p->p_lock);
+ error = sigprocmask1(l, SIG_SETMASK, &attrs->sa_sigmask, NULL);
+ mutex_exit(p->p_lock);
+ if (error)
+ goto out;
+ }
+
+ if (attrs->sa_flags & POSIX_SPAWN_SETSIGDEF) {
+ /*
+ * The following sigaction call is using a sigaction
+ * version 0 trampoline which is in the compatibility
+ * code only. This is not a problem because for SIG_DFL
+ * and SIG_IGN, the trampolines are now ignored. If they
+ * were not, this would be a problem because we are
+ * holding the exec_lock, and the compat code needs
+ * to do the same in order to replace the trampoline
+ * code of the process.
+ */
+ for (int i = 1; i <= NSIG; i++) {
+ if (sigismember(&attrs->sa_sigdefault, i))
+ sigaction1(l, i, &sigact, NULL, NULL, 0);
+ }
+ }
+ error = 0;
+out:
+ mutex_enter(proc_lock);
+ p->p_stat = ostat;
+ p->p_pptr->p_nstopchild--;
+ mutex_exit(proc_lock);
+ return error;
+}
+
/*
* A child lwp of a posix_spawn operation starts here and ends up in
* cpu_spawn_return, dealing with all filedescriptor and scheduler
@@ -2080,12 +2230,7 @@
struct spawn_exec_data *spawn_data = arg;
struct lwp *l = curlwp;
struct proc *p = l->l_proc;
- int error, newfd;
- int ostat;
- size_t i;
- const struct posix_spawn_file_actions_entry *fae;
- pid_t ppid;
- register_t retval;
+ int error;
bool have_reflock;
bool parent_is_waiting = true;
@@ -2097,10 +2242,9 @@
* We then try to get the exec_lock, and only if that works, we can
* release the parent here already.
*/
- ppid = spawn_data->sed_parent->p_pid;
- if ((!spawn_data->sed_attrs
- || (spawn_data->sed_attrs->sa_flags
- & (POSIX_SPAWN_RETURNERROR|POSIX_SPAWN_SETPGROUP)) == 0)
+ struct posix_spawnattr *attrs = spawn_data->sed_attrs;
+ if ((!attrs || (attrs->sa_flags
+ & (POSIX_SPAWN_RETURNERROR|POSIX_SPAWN_SETPGROUP)) == 0)
&& rw_tryenter(&exec_lock, RW_READER)) {
parent_is_waiting = false;
mutex_enter(&spawn_data->sed_mtx_child);
@@ -2112,144 +2256,15 @@
rw_enter(&p->p_reflock, RW_WRITER);
have_reflock = true;
- error = 0;
/* handle posix_spawn_file_actions */
- if (spawn_data->sed_actions != NULL) {
- for (i = 0; i < spawn_data->sed_actions->len; i++) {
- fae = &spawn_data->sed_actions->fae[i];
- switch (fae->fae_action) {
- case FAE_OPEN:
- if (fd_getfile(fae->fae_fildes) != NULL) {
- error = fd_close(fae->fae_fildes);
- if (error)
- break;
- }
- error = fd_open(fae->fae_path, fae->fae_oflag,
- fae->fae_mode, &newfd);
- if (error)
- break;
- if (newfd != fae->fae_fildes) {
- error = dodup(l, newfd,
- fae->fae_fildes, 0, &retval);
- if (fd_getfile(newfd) != NULL)
- fd_close(newfd);
- }
- break;
- case FAE_DUP2:
- error = dodup(l, fae->fae_fildes,
- fae->fae_newfildes, 0, &retval);
- break;
- case FAE_CLOSE:
- if (fd_getfile(fae->fae_fildes) == NULL) {
- error = EBADF;
- break;
- }
- error = fd_close(fae->fae_fildes);
- break;
- }
- if (error)
- goto report_error;
- }
- }
+ error = handle_posix_spawn_file_actions(spawn_data->sed_actions);
+ if (error)
+ goto report_error;
/* handle posix_spawnattr */
- if (spawn_data->sed_attrs != NULL) {
- struct sigaction sigact;
- memset(&sigact, 0, sizeof(sigact));
- sigact._sa_u._sa_handler = SIG_DFL;
- sigact.sa_flags = 0;
-
- /*
- * set state to SSTOP so that this proc can be found by pid.
- * see proc_enterprp, do_sched_setparam below
- */
- mutex_enter(proc_lock);
- /*
- * p_stat should be SACTIVE, so we need to adjust the
- * parent's p_nstopchild here. For safety, just make
- * we're on the good side of SDEAD before we adjust.
- */
- ostat = p->p_stat;
- KASSERT(ostat < SSTOP);
- p->p_stat = SSTOP;
- p->p_waited = 0;
- p->p_pptr->p_nstopchild++;
- mutex_exit(proc_lock);
-
- /* Set process group */
- if (spawn_data->sed_attrs->sa_flags & POSIX_SPAWN_SETPGROUP) {
- pid_t mypid = p->p_pid,
- pgrp = spawn_data->sed_attrs->sa_pgroup;
-
- if (pgrp == 0)
- pgrp = mypid;
-
- error = proc_enterpgrp(spawn_data->sed_parent,
- mypid, pgrp, false);
- if (error)
- goto report_error_stopped;
- }
-
- /* Set scheduler policy */
- if (spawn_data->sed_attrs->sa_flags & POSIX_SPAWN_SETSCHEDULER)
- error = do_sched_setparam(p->p_pid, 0,
- spawn_data->sed_attrs->sa_schedpolicy,
- &spawn_data->sed_attrs->sa_schedparam);
- else if (spawn_data->sed_attrs->sa_flags
- & POSIX_SPAWN_SETSCHEDPARAM) {
- error = do_sched_setparam(ppid, 0,
- SCHED_NONE, &spawn_data->sed_attrs->sa_schedparam);
- }
- if (error)
- goto report_error_stopped;
Home |
Main Index |
Thread Index |
Old Index