Module Name: src
Committed By: rmind
Date: Mon Apr 30 21:19:58 UTC 2012
Modified Files:
src/sys/compat/netbsd32: netbsd32_execve.c
src/sys/kern: kern_exec.c
src/sys/sys: exec.h spawn.h
Log Message:
posix_spawn:
- Remove copy-pasting in error paths, use execve_free_{vmspace,data}().
- Move some code (both in the init and exit paths) out of the locks.
- Slightly simplify do_posix_spawn() callers.
- Add few asserts and comments.
To generate a diff of this commit:
cvs rdiff -u -r1.34 -r1.35 src/sys/compat/netbsd32/netbsd32_execve.c
cvs rdiff -u -r1.350 -r1.351 src/sys/kern/kern_exec.c
cvs rdiff -u -r1.135 -r1.136 src/sys/sys/exec.h
cvs rdiff -u -r1.2 -r1.3 src/sys/sys/spawn.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/compat/netbsd32/netbsd32_execve.c
diff -u src/sys/compat/netbsd32/netbsd32_execve.c:1.34 src/sys/compat/netbsd32/netbsd32_execve.c:1.35
--- src/sys/compat/netbsd32/netbsd32_execve.c:1.34 Sun Apr 8 11:27:44 2012
+++ src/sys/compat/netbsd32/netbsd32_execve.c Mon Apr 30 21:19:58 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: netbsd32_execve.c,v 1.34 2012/04/08 11:27:44 martin Exp $ */
+/* $NetBSD: netbsd32_execve.c,v 1.35 2012/04/30 21:19:58 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.34 2012/04/08 11:27:44 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: netbsd32_execve.c,v 1.35 2012/04/30 21:19:58 rmind Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -178,7 +178,6 @@ netbsd32_posix_spawn(struct lwp *l,
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) {
@@ -191,7 +190,7 @@ netbsd32_posix_spawn(struct lwp *l,
error = netbsd32_posix_spawn_fa_alloc(&fa,
SCARG_P32(uap, file_actions));
if (error)
- goto error_exit;
+ goto fail;
}
/* copyin posix_spawnattr struct */
@@ -199,17 +198,17 @@ netbsd32_posix_spawn(struct lwp *l,
sa = kmem_alloc(sizeof(*sa), KM_SLEEP);
error = copyin(SCARG_P32(uap, attrp), sa, sizeof(*sa));
if (error)
- goto error_exit;
+ goto fail;
}
/*
* Do the spawn
*/
- error = do_posix_spawn(l, &pid, &child_ok, SCARG_P32(uap, path), fa,
+ error = do_posix_spawn(l, &pid, SCARG_P32(uap, path), fa,
sa, SCARG_P32(uap, argv), SCARG_P32(uap, envp),
netbsd32_execve_fetch_element);
if (error)
- goto error_exit;
+ goto fail;
if (error == 0 && SCARG_P32(uap, pid) != NULL)
error = copyout(&pid, SCARG_P32(uap, pid), sizeof(pid));
@@ -217,17 +216,14 @@ netbsd32_posix_spawn(struct lwp *l,
*retval = error;
return 0;
- 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);
- }
-
+fail:
+ (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);
*retval = error;
return 0;
}
Index: src/sys/kern/kern_exec.c
diff -u src/sys/kern/kern_exec.c:1.350 src/sys/kern/kern_exec.c:1.351
--- src/sys/kern/kern_exec.c:1.350 Sun Apr 15 15:35:00 2012
+++ src/sys/kern/kern_exec.c Mon Apr 30 21:19:58 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_exec.c,v 1.350 2012/04/15 15:35:00 martin Exp $ */
+/* $NetBSD: kern_exec.c,v 1.351 2012/04/30 21:19:58 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.350 2012/04/15 15:35:00 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.351 2012/04/30 21:19:58 rmind Exp $");
#include "opt_exec.h"
#include "opt_ktrace.h"
@@ -211,9 +211,8 @@ struct emul emul_netbsd = {
* 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;
-
-static kmutex_t sigobject_lock;
+krwlock_t exec_lock __cacheline_aligned;
+static kmutex_t sigobject_lock __cacheline_aligned;
/*
* Data used between a loadvm and execve part of an "exec" operation
@@ -296,7 +295,6 @@ static struct pool_allocator exec_palloc
* exec header unmodified.
*/
int
-/*ARGSUSED*/
check_exec(struct lwp *l, struct exec_package *epp, struct pathbuf *pb)
{
int error, i;
@@ -502,7 +500,7 @@ sys_execve(struct lwp *l, const struct s
SCARG(uap, envp), execve_fetch_element);
}
-int
+int
sys_fexecve(struct lwp *l, const struct sys_fexecve_args *uap,
register_t *retval)
{
@@ -564,6 +562,43 @@ exec_autoload(void)
#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,
@@ -575,11 +610,12 @@ execve_loadvm(struct lwp *l, const char
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);
@@ -599,6 +635,7 @@ execve_loadvm(struct lwp *l, const char
* 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),
@@ -610,14 +647,6 @@ execve_loadvm(struct lwp *l, const char
}
/*
- * 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
@@ -626,9 +655,7 @@ execve_loadvm(struct lwp *l, const char
*/
error = pathbuf_copyin(path, &data->ed_pathbuf);
if (error) {
- DPRINTF(("%s: pathbuf_copyin path @%p %d\n", __func__,
- path, error));
- goto clrflg;
+ return error;
}
data->ed_pathstring = pathbuf_stringcopy_get(data->ed_pathbuf);
@@ -657,6 +684,13 @@ execve_loadvm(struct lwp *l, const char
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. */
@@ -665,8 +699,9 @@ execve_loadvm(struct lwp *l, const char
DPRINTF(("%s: check exec failed %d\n",
__func__, error));
}
- goto freehdr;
+ goto bad;
}
+ freevm = true;
/* XXX -- THE FOLLOWING SECTION NEEDS MAJOR CLEANUP */
@@ -802,36 +837,15 @@ execve_loadvm(struct lwp *l, const char
return 0;
- 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);
-
- 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);
-
+bad:
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 (freevm) {
+ execve_free_vmspace(data);
+ }
+ execve_free_data(data);
+
if (modgen != module_gen && error == ENOEXEC) {
modgen = module_gen;
exec_autoload();
@@ -842,41 +856,11 @@ execve_loadvm(struct lwp *l, const char
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) {
- 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, curlwp->l_cred);
- vput(data->ed_pack.ep_vp);
- pool_put(&exec_pool, data->ed_argp);
-
- 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);
-
- pathbuf_stringcopy_put(data->ed_pathbuf, data->ed_pathstring);
- pathbuf_destroy(data->ed_pathbuf);
- PNBUF_PUT(data->ed_resolvedpathbuf);
-}
-
static int
execve_runproc(struct lwp *l, struct execve_data * restrict data,
bool no_local_exec_lock, bool is_spawn)
{
- int error = 0;
- struct proc *p;
+ struct proc *p = l->l_proc;
size_t i;
char *stack, *dp;
const char *commandname;
@@ -886,6 +870,7 @@ execve_runproc(struct lwp *l, struct exe
struct vmspace *vm;
ksiginfo_t ksi;
ksiginfoq_t kq;
+ int error = 0;
/*
* In case of a posix_spawn operation, the child doing the exec
@@ -893,13 +878,9 @@ execve_runproc(struct lwp *l, struct exe
* will do this instead.
*/
KASSERT(no_local_exec_lock || rw_lock_held(&exec_lock));
+ KASSERT(!no_local_exec_lock || is_spawn);
+ KASSERT(rw_lock_held(&p->p_reflock));
KASSERT(data != NULL);
- if (data == NULL)
- return (EINVAL);
-
- p = l->l_proc;
- if (no_local_exec_lock)
- KASSERT(is_spawn);
base_vcp = NULL;
@@ -1404,25 +1385,17 @@ execve_runproc(struct lwp *l, struct exe
if (!no_local_exec_lock)
rw_exit(&exec_lock);
- pathbuf_stringcopy_put(data->ed_pathbuf, data->ed_pathstring);
- pathbuf_destroy(data->ed_pathbuf);
- PNBUF_PUT(data->ed_resolvedpathbuf);
-
/*
* the old process doesn't exist anymore. exit gracefully.
* get rid of the (new) address space we have created, if any, get rid
* of our namei data and vnode, and exit noting failure
*/
uvm_deallocate(&vm->vm_map, VM_MIN_ADDRESS,
- VM_MAXUSER_ADDRESS - VM_MIN_ADDRESS);
+ VM_MAXUSER_ADDRESS - VM_MIN_ADDRESS);
exec_free_emul_arg(&data->ed_pack);
pool_put(&exec_pool, data->ed_argp);
- 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);
+ execve_free_data(data);
/* Acquire the sched-state mutex (exit1() will release it). */
if (!is_spawn) {
@@ -1817,6 +1790,12 @@ spawn_return(void *arg)
bool have_reflock;
bool parent_is_waiting = true;
+ error = 0;
+
+ /* don't allow debugger access yet */
+ rw_enter(&l->l_proc->p_reflock, RW_WRITER);
+ have_reflock = true;
+
/*
* Check if we can release parent early.
* We either need to have no sed_attrs, or sed_attrs does not
@@ -1836,11 +1815,6 @@ spawn_return(void *arg)
mutex_exit(&spawn_data->sed_mtx_child);
}
- /* don't allow debugger access yet */
- rw_enter(&l->l_proc->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++) {
@@ -1953,14 +1927,13 @@ spawn_return(void *arg)
}
}
- /* now do the real exec */
+ /* Now perform the real exec (will drop the locks). */
error = execve_runproc(l, &spawn_data->sed_exec, parent_is_waiting,
true);
have_reflock = false;
- if (error == EJUSTRETURN)
- error = 0;
- else if (error)
+ if (error && error != EJUSTRETURN) {
goto report_error;
+ }
if (parent_is_waiting) {
mutex_enter(&spawn_data->sed_mtx_child);
@@ -1986,6 +1959,7 @@ spawn_return(void *arg)
* so release/free both here.
*/
rw_exit(&l->l_proc->p_reflock);
+ execve_free_vmspace(&spawn_data->sed_exec);
execve_free_data(&spawn_data->sed_exec);
}
@@ -2002,18 +1976,20 @@ spawn_return(void *arg)
/* release our refcount on the data */
spawn_exec_data_release(spawn_data);
- /* done, exit */
- mutex_enter(l->l_proc->p_lock);
/*
- * Posix explicitly asks for an exit code of 127 if we report
+ * POSIX explicitly requires an exit code of 127 if we report
* errors from the child process - so, unfortunately, there
* is no way to report a more exact error code.
* A NetBSD specific workaround is POSIX_SPAWN_RETURNERROR as
* flag bit in the attrp argument to posix_spawn(2), see above.
*/
+ mutex_enter(l->l_proc->p_lock);
exit1(l, W_EXITCODE(127, 0));
}
+/*
+ * posix_spawn_fa_free: free file action structures.
+ */
void
posix_spawn_fa_free(struct posix_spawn_file_actions *fa, size_t len)
{
@@ -2029,16 +2005,22 @@ posix_spawn_fa_free(struct posix_spawn_f
kmem_free(fa, sizeof(*fa));
}
+/*
+ * posix_spawn_fa_alloc: allocation file action structures and copy-in from
+ * the user space. Note: we re-use the same structure in the kernel; care
+ * is taken to reset userspace pointers.
+ */
static int
posix_spawn_fa_alloc(struct posix_spawn_file_actions **fap,
const struct posix_spawn_file_actions *ufa)
{
struct posix_spawn_file_actions *fa;
struct posix_spawn_file_actions_entry *fae;
+ size_t i = 0, totalsize;
char *pbuf = NULL;
int error;
- size_t i = 0;
+ /* Allocate structure containing file action entries. */
fa = kmem_alloc(sizeof(*fa), KM_SLEEP);
error = copyin(ufa, fa, sizeof(*fa));
if (error) {
@@ -2046,30 +2028,34 @@ posix_spawn_fa_alloc(struct posix_spawn_
fa->len = 0;
goto out;
}
-
if (fa->len == 0) {
kmem_free(fa, sizeof(*fa));
return 0;
}
fa->size = fa->len;
- size_t fal = fa->len * sizeof(*fae);
- fae = fa->fae;
- fa->fae = kmem_alloc(fal, KM_SLEEP);
- error = copyin(fae, fa->fae, fal);
+ totalsize = fa->len * sizeof(*fae);
+
+ /* Allocate memory block needed for file action entries. */
+ fae = kmem_alloc(totalsize, KM_SLEEP);
+ error = copyin(fa->fae, fae, totalsize);
+ fa->fae = fae;
if (error)
goto out;
+ /* Allocate a path and copy-in each entry. */
pbuf = PNBUF_GET();
for (; i < fa->len; i++) {
+ size_t len;
+
fae = &fa->fae[i];
if (fae->fae_action != FAE_OPEN)
continue;
- error = copyinstr(fae->fae_path, pbuf, MAXPATHLEN, &fal);
+ error = copyinstr(fae->fae_path, pbuf, MAXPATHLEN, &len);
if (error)
goto out;
- fae->fae_path = kmem_alloc(fal, KM_SLEEP);
- memcpy(fae->fae_path, pbuf, fal);
+ fae->fae_path = kmem_alloc(len, KM_SLEEP);
+ memcpy(fae->fae_path, pbuf, len);
}
PNBUF_PUT(pbuf);
@@ -2125,20 +2111,16 @@ check_posix_spawn(struct lwp *l1)
}
int
-do_posix_spawn(struct lwp *l1, pid_t *pid_res, bool *child_ok, const char *path,
- struct posix_spawn_file_actions *fa,
- struct posix_spawnattr *sa,
- char *const *argv, char *const *envp,
- execve_fetch_element_t fetch)
+do_posix_spawn(struct lwp *l1, pid_t *pid_res, const char *path,
+ struct posix_spawn_file_actions *fa, struct posix_spawnattr *sa,
+ char *const *argv, char *const *envp, execve_fetch_element_t fetch)
{
-
struct proc *p1, *p2;
struct lwp *l2;
int error;
struct spawn_exec_data *spawn_data;
vaddr_t uaddr;
pid_t pid;
- bool have_exec_lock = false;
p1 = l1->l_proc;
@@ -2147,20 +2129,6 @@ do_posix_spawn(struct lwp *l1, pid_t *pi
spawn_data->sed_refcnt = 1; /* only parent so far */
cv_init(&spawn_data->sed_cv_child_ready, "pspawn");
mutex_init(&spawn_data->sed_mtx_child, MUTEX_DEFAULT, IPL_NONE);
- mutex_enter(&spawn_data->sed_mtx_child);
-
- /*
- * Do the first part of the exec now, collect state
- * in spawn_data.
- */
- error = execve_loadvm(l1, path, argv,
- envp, fetch, &spawn_data->sed_exec);
- if (error == EJUSTRETURN)
- error = 0;
- else if (error)
- goto error_exit;
-
- have_exec_lock = true;
/*
* Allocate virtual address space for the U-area now, while it
@@ -2169,10 +2137,26 @@ do_posix_spawn(struct lwp *l1, pid_t *pi
*/
uaddr = uvm_uarea_alloc();
if (__predict_false(uaddr == 0)) {
- error = ENOMEM;
- goto error_exit;
+ spawn_exec_data_release(spawn_data);
+ return ENOMEM;
}
-
+
+ /*
+ * Do the first part of the exec now, collect state in spawn_data.
+ */
+ mutex_enter(&spawn_data->sed_mtx_child);
+ error = execve_loadvm(l1, path, argv,
+ envp, fetch, &spawn_data->sed_exec);
+ if (error && error != EJUSTRETURN) {
+ uvm_uarea_free(uaddr);
+ mutex_exit(&spawn_data->sed_mtx_child);
+ spawn_exec_data_release(spawn_data);
+ return error;
+ }
+ KASSERT(rw_lock_held(&p1->p_reflock));
+ KASSERT(rw_lock_held(&exec_lock));
+ error = 0;
+
/*
* Allocate new proc. Borrow proc0 vmspace for it, we will
* replace it with its own before returning to userland
@@ -2286,14 +2270,13 @@ do_posix_spawn(struct lwp *l1, pid_t *pi
cpu_proc_fork(p1, p2);
/*
- * Prepare remaining parts of spawn data
+ * Prepare remaining parts of spawn data. Child gets a reference.
*/
spawn_data->sed_actions = fa;
spawn_data->sed_attrs = sa;
-
spawn_data->sed_parent = p1;
+ spawn_data->sed_refcnt++;
- /* create LWP */
lwp_create(l1, p2, uaddr, 0, NULL, 0, spawn_return, spawn_data,
&l2, l1->l_class);
l2->l_ctxlink = NULL; /* reset ucontext link */
@@ -2322,8 +2305,6 @@ do_posix_spawn(struct lwp *l1, pid_t *pi
kauth_cred_free(ocred);
}
- *child_ok = true;
- spawn_data->sed_refcnt = 2; /* child gets it as well */
#if 0
l2->l_nopreempt = 1; /* start it non-preemptable */
#endif
@@ -2370,24 +2351,13 @@ do_posix_spawn(struct lwp *l1, pid_t *pi
cv_wait(&spawn_data->sed_cv_child_ready, &spawn_data->sed_mtx_child);
error = spawn_data->sed_error;
mutex_exit(&spawn_data->sed_mtx_child);
- spawn_exec_data_release(spawn_data);
rw_exit(&p1->p_reflock);
rw_exit(&exec_lock);
- have_exec_lock = false;
- *pid_res = pid;
- return error;
-
- error_exit:
- if (have_exec_lock) {
- execve_free_data(&spawn_data->sed_exec);
- rw_exit(&p1->p_reflock);
- rw_exit(&exec_lock);
- }
- mutex_exit(&spawn_data->sed_mtx_child);
spawn_exec_data_release(spawn_data);
-
+
+ *pid_res = pid;
return error;
}
@@ -2408,7 +2378,6 @@ sys_posix_spawn(struct lwp *l1, const st
struct posix_spawn_file_actions *fa = NULL;
struct posix_spawnattr *sa = NULL;
pid_t pid;
- bool child_ok = false;
error = check_posix_spawn(l1);
if (error) {
@@ -2420,7 +2389,7 @@ sys_posix_spawn(struct lwp *l1, const st
if (SCARG(uap, file_actions) != NULL) {
error = posix_spawn_fa_alloc(&fa, SCARG(uap, file_actions));
if (error)
- goto error_exit;
+ goto fail;
}
/* copyin posix_spawnattr struct */
@@ -2428,16 +2397,16 @@ sys_posix_spawn(struct lwp *l1, const st
sa = kmem_alloc(sizeof(*sa), KM_SLEEP);
error = copyin(SCARG(uap, attrp), sa, sizeof(*sa));
if (error)
- goto error_exit;
+ goto fail;
}
/*
* Do the spawn
*/
- error = do_posix_spawn(l1, &pid, &child_ok, SCARG(uap, path), fa, sa,
+ error = do_posix_spawn(l1, &pid, SCARG(uap, path), fa, sa,
SCARG(uap, argv), SCARG(uap, envp), execve_fetch_element);
if (error)
- goto error_exit;
+ goto fail;
if (error == 0 && SCARG(uap, pid) != NULL)
error = copyout(&pid, SCARG(uap, pid), sizeof(pid));
@@ -2445,17 +2414,14 @@ sys_posix_spawn(struct lwp *l1, const st
*retval = error;
return 0;
- error_exit:
- if (!child_ok) {
- (void)chgproccnt(kauth_cred_getuid(l1->l_cred), -1);
- atomic_dec_uint(&nprocs);
-
- if (sa)
- kmem_free(sa, sizeof(*sa));
- if (fa)
- posix_spawn_fa_free(fa, fa->len);
- }
-
+fail:
+ (void)chgproccnt(kauth_cred_getuid(l1->l_cred), -1);
+ atomic_dec_uint(&nprocs);
+
+ if (sa)
+ kmem_free(sa, sizeof(*sa));
+ if (fa)
+ posix_spawn_fa_free(fa, fa->len);
*retval = error;
return 0;
}
Index: src/sys/sys/exec.h
diff -u src/sys/sys/exec.h:1.135 src/sys/sys/exec.h:1.136
--- src/sys/sys/exec.h:1.135 Sun Apr 8 11:27:45 2012
+++ src/sys/sys/exec.h Mon Apr 30 21:19:58 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: exec.h,v 1.135 2012/04/08 11:27:45 martin Exp $ */
+/* $NetBSD: exec.h,v 1.136 2012/04/30 21:19:58 rmind Exp $ */
/*-
* Copyright (c) 1992, 1993
@@ -301,7 +301,7 @@ struct posix_spawn_file_actions;
struct posix_spawnattr;
int check_posix_spawn (struct lwp *);
void posix_spawn_fa_free(struct posix_spawn_file_actions *, size_t);
-int do_posix_spawn(struct lwp *, pid_t *, bool*, const char *,
+int do_posix_spawn(struct lwp *, pid_t *, const char *,
struct posix_spawn_file_actions *, struct posix_spawnattr *,
char *const *argv, char *const *, execve_fetch_element_t);
Index: src/sys/sys/spawn.h
diff -u src/sys/sys/spawn.h:1.2 src/sys/sys/spawn.h:1.3
--- src/sys/sys/spawn.h:1.2 Sun Apr 8 11:27:45 2012
+++ src/sys/sys/spawn.h Mon Apr 30 21:19:58 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: spawn.h,v 1.2 2012/04/08 11:27:45 martin Exp $ */
+/* $NetBSD: spawn.h,v 1.3 2012/04/30 21:19:58 rmind Exp $ */
/*-
* Copyright (c) 2008 Ed Schouten <e...@freebsd.org>
@@ -69,7 +69,7 @@ typedef struct posix_spawn_file_actions_
struct posix_spawn_file_actions {
unsigned int size; /* size of fae array */
unsigned int len; /* how many slots are used */
- posix_spawn_file_actions_entry_t *fae;
+ posix_spawn_file_actions_entry_t *fae;
};
typedef struct posix_spawnattr posix_spawnattr_t;
@@ -81,9 +81,9 @@ typedef struct posix_spawn_file_actions
#define POSIX_SPAWN_SETSCHEDULER 0x08
#define POSIX_SPAWN_SETSIGDEF 0x10
#define POSIX_SPAWN_SETSIGMASK 0x20
+
+#ifdef _NETBSD_SOURCE
/*
- * THIS IS A NON-PORTABLE NetBSD-ONLY EXTENSION, DO NOT USE OUTSIDE
- * OF UNIT TEST CODE!
* With this flag set, the kernel part of posix_spawn will not try to
* maximize parallelism, but instead the parent will wait for the child
* process to complete all file/scheduler actions and report back errors
@@ -98,6 +98,6 @@ typedef struct posix_spawn_file_actions
* case, but request the POSIX_SPAWN_RETURNERROR for some tests.
*/
#define POSIX_SPAWN_RETURNERROR 0x40
+#endif
#endif /* !_SYS_SPAWN_H_ */
-
