Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/kern In spawn_return() we temporarily move the process s...



details:   https://anonhg.NetBSD.org/src/rev/287811f7e9af
branches:  trunk
changeset: 340951:287811f7e9af
user:      pgoyette <pgoyette%NetBSD.org@localhost>
date:      Tue Oct 13 00:29:34 2015 +0000

description:
In spawn_return() we temporarily move the process state to SSTOP, but
without updating its p_waited value or its parent's p_nstopchild
counter.  Later, we restore the original state, again without any
adjustment of the related values.  This leaves a relatively short
window when the values are inconsistent and could interfere with the
proper operation of sys_wait() for the parent (if it manages to be
scheduled;  it's not totally clear what, if anything, prevents
scheduling/execution of the parent).

If during this window, any of the checks being made result in an
error, we call exit1() which will eventually migrate the process's
state to SDEAD (with an intermediate transition to SDYING).  At
this point the other variables get updated, and we finally restore
a consistent state.

This change updates the p_waited and parent's p_nstopchild at each
step to eliminate any windows during which the values could lead to
incorrect decisions.

Fixes PR kern/50330

Pullups will be requested for NetBSD-7, -6, -6-0, and -6-1

diffstat:

 sys/kern/kern_exec.c |  34 ++++++++++++++++++++++++++--------
 1 files changed, 26 insertions(+), 8 deletions(-)

diffs (118 lines):

diff -r 8d804c27b732 -r 287811f7e9af sys/kern/kern_exec.c
--- a/sys/kern/kern_exec.c      Tue Oct 13 00:28:22 2015 +0000
+++ b/sys/kern/kern_exec.c      Tue Oct 13 00:29:34 2015 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_exec.c,v 1.419 2015/10/13 00:24:35 pgoyette Exp $ */
+/*     $NetBSD: kern_exec.c,v 1.420 2015/10/13 00:29:34 pgoyette Exp $ */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -59,7 +59,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.419 2015/10/13 00:24:35 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.420 2015/10/13 00:29:34 pgoyette Exp $");
 
 #include "opt_exec.h"
 #include "opt_execfmt.h"
@@ -1983,6 +1983,7 @@
        struct spawn_exec_data *spawn_data = arg;
        struct lwp *l = curlwp;
        int error, newfd;
+       int ostat;
        size_t i;
        const struct posix_spawn_file_actions_entry *fae;
        pid_t ppid;
@@ -2055,7 +2056,6 @@
 
        /* handle posix_spawnattr */
        if (spawn_data->sed_attrs != NULL) {
-               int ostat;
                struct sigaction sigact;
                sigact._sa_u._sa_handler = SIG_DFL;
                sigact.sa_flags = 0;
@@ -2064,8 +2064,18 @@
                 * 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 = l->l_proc->p_stat;
+               KASSERT(ostat < SSTOP);
                l->l_proc->p_stat = SSTOP;
+               l->l_proc->p_waited = 0;
+               l->l_proc->p_pptr->p_nstopchild++;
+               mutex_exit(proc_lock);
 
                /* Set process group */
                if (spawn_data->sed_attrs->sa_flags & POSIX_SPAWN_SETPGROUP) {
@@ -2078,7 +2088,7 @@
                        error = proc_enterpgrp(spawn_data->sed_parent,
                            mypid, pgrp, false);
                        if (error)
-                               goto report_error;
+                               goto report_error_stopped;
                }
 
                /* Set scheduler policy */
@@ -2092,7 +2102,7 @@
                            SCHED_NONE, &spawn_data->sed_attrs->sa_schedparam);
                }
                if (error)
-                       goto report_error;
+                       goto report_error_stopped;
 
                /* Reset user ID's */
                if (spawn_data->sed_attrs->sa_flags & POSIX_SPAWN_RESETIDS) {
@@ -2100,12 +2110,12 @@
                             kauth_cred_getgid(l->l_cred), -1,
                             ID_E_EQ_R | ID_E_EQ_S);
                        if (error)
-                               goto report_error;
+                               goto report_error_stopped;
                        error = do_setresuid(l, -1,
                            kauth_cred_getuid(l->l_cred), -1,
                            ID_E_EQ_R | ID_E_EQ_S);
                        if (error)
-                               goto report_error;
+                               goto report_error_stopped;
                }
 
                /* Set signal masks/defaults */
@@ -2115,7 +2125,7 @@
                            &spawn_data->sed_attrs->sa_sigmask, NULL);
                        mutex_exit(l->l_proc->p_lock);
                        if (error)
-                               goto report_error;
+                               goto report_error_stopped;
                }
 
                if (spawn_data->sed_attrs->sa_flags & POSIX_SPAWN_SETSIGDEF) {
@@ -2136,7 +2146,10 @@
                                            0);
                        }
                }
+               mutex_enter(proc_lock);
                l->l_proc->p_stat = ostat;
+               l->l_proc->p_pptr->p_nstopchild--;
+               mutex_exit(proc_lock);
        }
 
        /* now do the real exec */
@@ -2163,6 +2176,11 @@
        /* NOTREACHED */
        return;
 
+ report_error_stopped:
+       mutex_enter(proc_lock);
+       l->l_proc->p_stat = ostat;
+       l->l_proc->p_pptr->p_nstopchild--;
+       mutex_exit(proc_lock);
  report_error:
        if (have_reflock) {
                /*



Home | Main Index | Thread Index | Old Index