[src/trunk]: src/sys Revert posix_spawn() clean up for now, there are some bugs.

[rmind <rmind%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/7611f99987fd
branches:  trunk
changeset: 779106:7611f99987fd
user:      rmind <rmind%NetBSD.org@localhost>
date:      Wed May 02 23:33:11 2012 +0000

description:
Revert posix_spawn() clean up for now, there are some bugs.

diffstat:

 sys/compat/netbsd32/netbsd32_execve.c |   30 +-
 sys/kern/kern_exec.c                  |  312 ++++++++++++++++++---------------
 sys/sys/exec.h                        |    4 +-
 3 files changed, 192 insertions(+), 154 deletions(-)

diffs (truncated from 734 to 300 lines):

diff -r 0438a2678d1c -r 7611f99987fd sys/compat/netbsd32/netbsd32_execve.c
--- a/sys/compat/netbsd32/netbsd32_execve.c     Wed May 02 22:38:31 2012 +0000
+++ b/sys/compat/netbsd32/netbsd32_execve.c     Wed May 02 23:33:11 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: netbsd32_execve.c,v 1.35 2012/04/30 21:19:58 rmind Exp $       */
+/*     $NetBSD: netbsd32_execve.c,v 1.36 2012/05/02 23:33:11 rmind Exp $       */
 
 /*
  * Copyright (c) 1998, 2001 Matthew R. Green
@@ -28,7 +28,7 @@
 
 #include <sys/cdefs.h>
 
-__KERNEL_RCSID(0, "$NetBSD: netbsd32_execve.c,v 1.35 2012/04/30 21:19:58 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: netbsd32_execve.c,v 1.36 2012/05/02 23:33:11 rmind Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -178,6 +178,7 @@
        struct posix_spawn_file_actions *fa = NULL;
        struct posix_spawnattr *sa = NULL;
        pid_t pid;
+       bool child_ok = false;
 
        error = check_posix_spawn(l);
        if (error) {
@@ -190,7 +191,7 @@
                error = netbsd32_posix_spawn_fa_alloc(&fa,
                    SCARG_P32(uap, file_actions));
                if (error)
-                       goto fail;
+                       goto error_exit;
        }
 
        /* copyin posix_spawnattr struct */
@@ -198,17 +199,17 @@
                sa = kmem_alloc(sizeof(*sa), KM_SLEEP);
                error = copyin(SCARG_P32(uap, attrp), sa, sizeof(*sa));
                if (error)
-                       goto fail;
+                       goto error_exit;
        }
 
        /*
         * Do the spawn
         */
-       error = do_posix_spawn(l, &pid, SCARG_P32(uap, path), fa,
+       error = do_posix_spawn(l, &pid, &child_ok, SCARG_P32(uap, path), fa,
            sa, SCARG_P32(uap, argv), SCARG_P32(uap, envp),
            netbsd32_execve_fetch_element);
        if (error)
-               goto fail;
+               goto error_exit;
 
        if (error == 0 && SCARG_P32(uap, pid) != NULL)
                error = copyout(&pid, SCARG_P32(uap, pid), sizeof(pid));
@@ -216,14 +217,17 @@
        *retval = error;
        return 0;
 
-fail:
-       (void)chgproccnt(kauth_cred_getuid(l->l_cred), -1);
-       atomic_dec_uint(&nprocs);
+ error_exit:
+       if (!child_ok) {
+               (void)chgproccnt(kauth_cred_getuid(l->l_cred), -1);
+               atomic_dec_uint(&nprocs);
 
-       if (sa)
-               kmem_free(sa, sizeof(*sa));
-       if (fa)
-               posix_spawn_fa_free(fa, fa->len);
+               if (sa)
+                       kmem_free(sa, sizeof(*sa));
+               if (fa)
+                       posix_spawn_fa_free(fa, fa->len);
+       }
+
        *retval = error;
        return 0;
 }
diff -r 0438a2678d1c -r 7611f99987fd sys/kern/kern_exec.c
--- a/sys/kern/kern_exec.c      Wed May 02 22:38:31 2012 +0000
+++ b/sys/kern/kern_exec.c      Wed May 02 23:33:11 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_exec.c,v 1.351 2012/04/30 21:19:58 rmind Exp $    */
+/*     $NetBSD: kern_exec.c,v 1.352 2012/05/02 23:33:11 rmind 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.351 2012/04/30 21:19:58 rmind Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.352 2012/05/02 23:33:11 rmind Exp $");
 
 #include "opt_exec.h"
 #include "opt_ktrace.h"
@@ -211,8 +211,9 @@
  * Exec lock. Used to control access to execsw[] structures.
  * This must not be static so that netbsd32 can access it, too.
  */
-krwlock_t              exec_lock       __cacheline_aligned;
-static kmutex_t                sigobject_lock  __cacheline_aligned;
+krwlock_t exec_lock;
+
+static kmutex_t sigobject_lock;
 
 /*
  * Data used between a loadvm and execve part of an "exec" operation
@@ -295,6 +296,7 @@
  *                     exec header unmodified.
  */
 int
+/*ARGSUSED*/
 check_exec(struct lwp *l, struct exec_package *epp, struct pathbuf *pb)
 {
        int             error, i;
@@ -500,7 +502,7 @@
            SCARG(uap, envp), execve_fetch_element);
 }
 
-int
+int   
 sys_fexecve(struct lwp *l, const struct sys_fexecve_args *uap,
     register_t *retval)
 {
@@ -562,43 +564,6 @@
 #endif
 }
 
-static void
-execve_free_vmspace(struct execve_data *ed)
-{
-
-       /*
-        * Free the vmspace-creation commands and release their references.
-        */
-       kill_vmcmds(&ed->ed_pack.ep_vmcmds);
-
-       /* Kill any opened file descriptor, if necessary. */
-       if (ed->ed_pack.ep_flags & EXEC_HASFD) {
-               ed->ed_pack.ep_flags &= ~EXEC_HASFD;
-               fd_close(ed->ed_pack.ep_fd);
-       }
-
-       /* Close and put the executed file. */
-       vn_lock(ed->ed_pack.ep_vp, LK_EXCLUSIVE | LK_RETRY);
-       VOP_CLOSE(ed->ed_pack.ep_vp, FREAD, curlwp->l_cred);
-       vput(ed->ed_pack.ep_vp);
-       pool_put(&exec_pool, ed->ed_argp);
-}
-
-static void
-execve_free_data(struct execve_data *ed)
-{
-
-       kmem_free(ed->ed_pack.ep_hdr, ed->ed_pack.ep_hdrlen);
-       if (ed->ed_pack.ep_emul_root != NULL)
-               vrele(ed->ed_pack.ep_emul_root);
-       if (ed->ed_pack.ep_interp != NULL)
-               vrele(ed->ed_pack.ep_interp);
-
-       pathbuf_stringcopy_put(ed->ed_pathbuf, ed->ed_pathstring);
-       pathbuf_destroy(ed->ed_pathbuf);
-       PNBUF_PUT(ed->ed_resolvedpathbuf);
-}
-
 static int
 execve_loadvm(struct lwp *l, const char *path, char * const *args,
        char * const *envs, execve_fetch_element_t fetch_element,
@@ -610,12 +575,11 @@
        size_t                  i, len;
        struct exec_fakearg     *tmpfap;
        u_int                   modgen;
-       bool                    freevm;
 
        KASSERT(data != NULL);
 
        p = l->l_proc;
-       modgen = 0;
+       modgen = 0;
 
        SDT_PROBE(proc,,,exec, path, 0, 0, 0, 0);
 
@@ -635,7 +599,6 @@
         * to call exec in order to do something useful.
         */
  retry:
-       freevm = false;
        if (p->p_flag & PK_SUGID) {
                if (kauth_authorize_process(l->l_cred, KAUTH_PROCESS_RLIMIT,
                     p, KAUTH_ARG(KAUTH_REQ_PROCESS_RLIMIT_BYPASS),
@@ -647,6 +610,14 @@
        }
 
        /*
+        * Drain existing references and forbid new ones.  The process
+        * should be left alone until we're done here.  This is necessary
+        * to avoid race conditions - e.g. in ptrace() - that might allow
+        * a local user to illicitly obtain elevated privileges.
+        */
+       rw_enter(&p->p_reflock, RW_WRITER);
+
+       /*
         * Init the namei data to point the file user's program name.
         * This is done here rather than in check_exec(), so that it's
         * possible to override this settings if any of makecmd/probe
@@ -655,7 +626,9 @@
         */
        error = pathbuf_copyin(path, &data->ed_pathbuf);
        if (error) {
-               return error;
+               DPRINTF(("%s: pathbuf_copyin path @%p %d\n", __func__,
+                   path, error));
+               goto clrflg;
        }
        data->ed_pathstring = pathbuf_stringcopy_get(data->ed_pathbuf);
 
@@ -684,13 +657,6 @@
        data->ed_pack.ep_esch = NULL;
        data->ed_pack.ep_pax_flags = 0;
 
-       /*
-        * Drain existing references and forbid new ones.  The process
-        * should be left alone until we're done here.  This is necessary
-        * to avoid race conditions - e.g. in ptrace() - that might allow
-        * a local user to illicitly obtain elevated privileges.
-        */
-       rw_enter(&p->p_reflock, RW_WRITER);
        rw_enter(&exec_lock, RW_READER);
 
        /* see if we can run it. */
@@ -699,9 +665,8 @@
                        DPRINTF(("%s: check exec failed %d\n",
                            __func__, error));
                }
-               goto bad;
+               goto freehdr;
        }
-       freevm = true;
 
        /* XXX -- THE FOLLOWING SECTION NEEDS MAJOR CLEANUP */
 
@@ -837,14 +802,35 @@
 
        return 0;
 
-bad:
-       rw_exit(&exec_lock);
-       rw_exit(&p->p_reflock);
+ bad:
+       /* free the vmspace-creation commands, and release their references */
+       kill_vmcmds(&data->ed_pack.ep_vmcmds);
+       /* kill any opened file descriptor, if necessary */
+       if (data->ed_pack.ep_flags & EXEC_HASFD) {
+               data->ed_pack.ep_flags &= ~EXEC_HASFD;
+               fd_close(data->ed_pack.ep_fd);
+       }
+       /* close and put the exec'd file */
+       vn_lock(data->ed_pack.ep_vp, LK_EXCLUSIVE | LK_RETRY);
+       VOP_CLOSE(data->ed_pack.ep_vp, FREAD, l->l_cred);
+       vput(data->ed_pack.ep_vp);
+       pool_put(&exec_pool, data->ed_argp);
 
-       if (freevm) {
-               execve_free_vmspace(data);
-       }
-       execve_free_data(data);
+ freehdr:
+       kmem_free(data->ed_pack.ep_hdr, data->ed_pack.ep_hdrlen);
+       if (data->ed_pack.ep_emul_root != NULL)
+               vrele(data->ed_pack.ep_emul_root);
+       if (data->ed_pack.ep_interp != NULL)
+               vrele(data->ed_pack.ep_interp);
+
+       rw_exit(&exec_lock);
+
+       pathbuf_stringcopy_put(data->ed_pathbuf, data->ed_pathstring);
+       pathbuf_destroy(data->ed_pathbuf);
+       PNBUF_PUT(data->ed_resolvedpathbuf);
+
+ clrflg:
+       rw_exit(&p->p_reflock);
 
        if (modgen != module_gen && error == ENOEXEC) {
                modgen = module_gen;
@@ -856,11 +842,41 @@
        return error;
 }
 
+static void
+execve_free_data(struct execve_data *data)
+{
+
+       /* free the vmspace-creation commands, and release their references */
+       kill_vmcmds(&data->ed_pack.ep_vmcmds);
+       /* kill any opened file descriptor, if necessary */
+       if (data->ed_pack.ep_flags & EXEC_HASFD) {