Module Name: src Committed By: martin Date: Sun Apr 8 11:27:46 UTC 2012
Modified Files: src/lib/libc/gen: posix_spawn_fileactions.c src/sys/compat/netbsd32: netbsd32.h netbsd32_execve.c syscalls.master src/sys/kern: exec_elf.c kern_exec.c kern_exit.c src/sys/sys: exec.h spawn.h src/sys/uvm: uvm_extern.h uvm_glue.c uvm_map.c src/tests/lib/libc/gen/posix_spawn: t_fileactions.c Log Message: Rework posix_spawn locking and memory management: - always provide a vmspace for the new proc, initially borrowing from proc0 (this part fixes PR 46286) - increase parallelism between parent and child if arguments allow this, avoiding a potential deadlock on exec_lock - add a new flag for userland to request old (lockstepped) behaviour for better error reporting - adapt test cases to the previous two and add a new variant to test the diagnostics flag - fix a few memory (and lock) leaks - provide netbsd32 compat To generate a diff of this commit: cvs rdiff -u -r1.1 -r1.2 src/lib/libc/gen/posix_spawn_fileactions.c cvs rdiff -u -r1.94 -r1.95 src/sys/compat/netbsd32/netbsd32.h \ src/sys/compat/netbsd32/syscalls.master cvs rdiff -u -r1.33 -r1.34 src/sys/compat/netbsd32/netbsd32_execve.c cvs rdiff -u -r1.37 -r1.38 src/sys/kern/exec_elf.c cvs rdiff -u -r1.347 -r1.348 src/sys/kern/kern_exec.c cvs rdiff -u -r1.237 -r1.238 src/sys/kern/kern_exit.c cvs rdiff -u -r1.134 -r1.135 src/sys/sys/exec.h cvs rdiff -u -r1.1 -r1.2 src/sys/sys/spawn.h cvs rdiff -u -r1.182 -r1.183 src/sys/uvm/uvm_extern.h cvs rdiff -u -r1.158 -r1.159 src/sys/uvm/uvm_glue.c cvs rdiff -u -r1.316 -r1.317 src/sys/uvm/uvm_map.c cvs rdiff -u -r1.3 -r1.4 src/tests/lib/libc/gen/posix_spawn/t_fileactions.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/lib/libc/gen/posix_spawn_fileactions.c diff -u src/lib/libc/gen/posix_spawn_fileactions.c:1.1 src/lib/libc/gen/posix_spawn_fileactions.c:1.2 --- src/lib/libc/gen/posix_spawn_fileactions.c:1.1 Sat Feb 11 23:31:24 2012 +++ src/lib/libc/gen/posix_spawn_fileactions.c Sun Apr 8 11:27:44 2012 @@ -25,7 +25,7 @@ */ #include <sys/cdefs.h> -__RCSID("$NetBSD: posix_spawn_fileactions.c,v 1.1 2012/02/11 23:31:24 martin Exp $"); +__RCSID("$NetBSD: posix_spawn_fileactions.c,v 1.2 2012/04/08 11:27:44 martin Exp $"); #include "namespace.h" @@ -62,7 +62,7 @@ posix_spawn_file_actions_init(posix_spaw int posix_spawn_file_actions_destroy(posix_spawn_file_actions_t *fa) { - int i; + unsigned int i; if (fa == NULL) return (-1); @@ -80,7 +80,7 @@ static int posix_spawn_file_actions_getentry(posix_spawn_file_actions_t *fa) { if (fa == NULL) - return (-1); + return -1; if (fa->len < fa->size) return fa->len; @@ -89,7 +89,7 @@ posix_spawn_file_actions_getentry(posix_ sizeof(struct posix_spawn_file_actions_entry)); if (fa->fae == NULL) - return (-1); + return -1; fa->size += MIN_SIZE; Index: src/sys/compat/netbsd32/netbsd32.h diff -u src/sys/compat/netbsd32/netbsd32.h:1.94 src/sys/compat/netbsd32/netbsd32.h:1.95 --- src/sys/compat/netbsd32/netbsd32.h:1.94 Tue Mar 6 07:37:05 2012 +++ src/sys/compat/netbsd32/netbsd32.h Sun Apr 8 11:27:44 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: netbsd32.h,v 1.94 2012/03/06 07:37:05 macallan Exp $ */ +/* $NetBSD: netbsd32.h,v 1.95 2012/04/08 11:27:44 martin Exp $ */ /* * Copyright (c) 1998, 2001, 2008 Matthew R. Green @@ -151,6 +151,10 @@ typedef netbsd32_pointer_t netbsd32_lwpi typedef netbsd32_pointer_t netbsd32_ucontextp; typedef netbsd32_pointer_t netbsd32_caddr_t; typedef netbsd32_pointer_t netbsd32_lwpctlp; +typedef netbsd32_pointer_t netbsd32_posix_spawn_file_actionsp; +typedef netbsd32_pointer_t netbsd32_posix_spawnattrp; +typedef netbsd32_pointer_t netbsd32_posix_spawn_file_actions_entryp; +typedef netbsd32_pointer_t netbsd32_pid_tp; /* * now, the compatibility structures and their fake pointer types. @@ -941,6 +945,28 @@ struct netbsd32_msdosfs_args { int gmtoff; /* v3: offset from UTC in seconds */ }; +struct netbsd32_posix_spawn_file_actions_entry { + enum { FAE32_OPEN, FAE32_DUP2, FAE32_CLOSE } fae_action; + + int fae_fildes; + union { + struct { + netbsd32_charp path; + int oflag; + mode_t mode; + } open; + struct { + int newfildes; + } dup2; + } fae_data; +}; + +struct netbsd32_posix_spawn_file_actions { + unsigned int size; /* size of fae array */ + unsigned int len; /* how many slots are used */ + netbsd32_posix_spawn_file_actions_entryp fae; +}; + #if 0 int netbsd32_kevent(struct lwp *, void *, register_t *); #endif Index: src/sys/compat/netbsd32/syscalls.master diff -u src/sys/compat/netbsd32/syscalls.master:1.94 src/sys/compat/netbsd32/syscalls.master:1.95 --- src/sys/compat/netbsd32/syscalls.master:1.94 Sat Mar 10 21:51:59 2012 +++ src/sys/compat/netbsd32/syscalls.master Sun Apr 8 11:27:44 2012 @@ -1,4 +1,4 @@ - $NetBSD: syscalls.master,v 1.94 2012/03/10 21:51:59 joerg Exp $ + $NetBSD: syscalls.master,v 1.95 2012/04/08 11:27:44 martin Exp $ ; from: NetBSD: syscalls.master,v 1.81 1998/07/05 08:49:50 jonathan Exp ; @(#)syscalls.master 8.2 (Berkeley) 1/13/94 @@ -1023,3 +1023,10 @@ const netbsd32_timespecp_t tptr); } 473 STD { int|netbsd32||__quotactl(const netbsd32_charp path, \ netbsd32_voidp args); } +474 NOERR { int|netbsd32||posix_spawn(netbsd32_pid_tp pid, \ + const netbsd32_charp path, \ + const netbsd32_posix_spawn_file_actionsp \ + file_actions, \ + const netbsd32_posix_spawnattrp attrp, \ + netbsd32_charpp argv, netbsd32_charpp envp); } + Index: src/sys/compat/netbsd32/netbsd32_execve.c diff -u src/sys/compat/netbsd32/netbsd32_execve.c:1.33 src/sys/compat/netbsd32/netbsd32_execve.c:1.34 --- src/sys/compat/netbsd32/netbsd32_execve.c:1.33 Tue Jan 31 22:53:56 2012 +++ src/sys/compat/netbsd32/netbsd32_execve.c Sun Apr 8 11:27:44 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: netbsd32_execve.c,v 1.33 2012/01/31 22:53:56 matt Exp $ */ +/* $NetBSD: netbsd32_execve.c,v 1.34 2012/04/08 11:27:44 martin Exp $ */ /* * Copyright (c) 1998, 2001 Matthew R. Green @@ -28,12 +28,16 @@ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: netbsd32_execve.c,v 1.33 2012/01/31 22:53:56 matt Exp $"); +__KERNEL_RCSID(0, "$NetBSD: netbsd32_execve.c,v 1.34 2012/04/08 11:27:44 martin Exp $"); #include <sys/param.h> #include <sys/systm.h> +#include <sys/atomic.h> #include <sys/mount.h> +#include <sys/namei.h> #include <sys/stat.h> +#include <sys/spawn.h> +#include <sys/uidinfo.h> #include <sys/vnode.h> #include <sys/file.h> #include <sys/filedesc.h> @@ -90,3 +94,140 @@ netbsd32_fexecve(struct lwp *l, const st return sys_fexecve(l, &ua, retval); } + +static int +netbsd32_posix_spawn_fa_alloc(struct posix_spawn_file_actions **fap, + const struct netbsd32_posix_spawn_file_actions *ufa) +{ + struct posix_spawn_file_actions *fa; + struct netbsd32_posix_spawn_file_actions fa32; + struct netbsd32_posix_spawn_file_actions_entry *fae32 = NULL, *f32 = NULL; + struct posix_spawn_file_actions_entry *fae; + char *pbuf = NULL; + int error; + size_t fal, fal32, slen, i = 0; + + error = copyin(ufa, &fa32, sizeof(fa32)); + if (error) + return error; + + if (fa32.len == 0) + return 0; + + fa = kmem_alloc(sizeof(*fa), KM_SLEEP); + fa->len = fa->size = fa32.len; + + fal = fa->len * sizeof(*fae); + fal32 = fa->len * sizeof(*fae32); + + fa->fae = kmem_alloc(fal, KM_SLEEP); + fae32 = kmem_alloc(fal32, KM_SLEEP); + error = copyin(NETBSD32PTR64(fa32.fae), fae32, fal32); + if (error) + goto out; + + pbuf = PNBUF_GET(); + for (; i < fa->len; i++) { + fae = &fa->fae[i]; + f32 = &fae32[i]; + fae->fae_action = f32->fae_action; + fae->fae_fildes = f32->fae_fildes; + if (fae->fae_action == FAE_DUP2) + fae->fae_data.dup2.newfildes = + f32->fae_data.dup2.newfildes; + if (fae->fae_action != FAE_OPEN) + continue; + error = copyinstr(NETBSD32PTR64(f32->fae_path), pbuf, + MAXPATHLEN, &slen); + if (error) + goto out; + fae->fae_path = kmem_alloc(fal, KM_SLEEP); + memcpy(fae->fae_path, pbuf, slen); + fae->fae_oflag = f32->fae_oflag; + fae->fae_mode = f32->fae_mode; + } + PNBUF_PUT(pbuf); + if (fae32) + kmem_free(fae32, fal32); + *fap = fa; + return 0; + +out: + if (fae32) + kmem_free(fae32, fal32); + if (pbuf) + PNBUF_PUT(pbuf); + posix_spawn_fa_free(fa, i); + return error; +} + +int +netbsd32_posix_spawn(struct lwp *l, + const struct netbsd32_posix_spawn_args *uap, register_t *retval) +{ + /* { + syscallarg(netbsd32_pid_tp) pid; + syscallarg(const netbsd32_charp) path; + syscallarg(const netbsd32_posix_spawn_file_actionsp) file_actions; + syscallarg(const netbsd32_posix_spawnattrp) attrp; + syscallarg(netbsd32_charpp) argv; + syscallarg(netbsd32_charpp) envp; + } */ + + int error; + 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) { + *retval = error; + return 0; + } + + /* copy in file_actions struct */ + if (SCARG_P32(uap, file_actions) != NULL) { + error = netbsd32_posix_spawn_fa_alloc(&fa, + SCARG_P32(uap, file_actions)); + if (error) + goto error_exit; + } + + /* copyin posix_spawnattr struct */ + if (SCARG_P32(uap, attrp) != NULL) { + sa = kmem_alloc(sizeof(*sa), KM_SLEEP); + error = copyin(SCARG_P32(uap, attrp), sa, sizeof(*sa)); + if (error) + goto error_exit; + } + + /* + * Do the spawn + */ + 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 error_exit; + + if (error == 0 && SCARG_P32(uap, pid) != NULL) + error = copyout(&pid, SCARG_P32(uap, pid), sizeof(pid)); + + *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); + } + + *retval = error; + return 0; +} Index: src/sys/kern/exec_elf.c diff -u src/sys/kern/exec_elf.c:1.37 src/sys/kern/exec_elf.c:1.38 --- src/sys/kern/exec_elf.c:1.37 Sat Feb 11 23:16:16 2012 +++ src/sys/kern/exec_elf.c Sun Apr 8 11:27:44 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: exec_elf.c,v 1.37 2012/02/11 23:16:16 martin Exp $ */ +/* $NetBSD: exec_elf.c,v 1.38 2012/04/08 11:27:44 martin Exp $ */ /*- * Copyright (c) 1994, 2000, 2005 The NetBSD Foundation, Inc. @@ -57,7 +57,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(1, "$NetBSD: exec_elf.c,v 1.37 2012/02/11 23:16:16 martin Exp $"); +__KERNEL_RCSID(1, "$NetBSD: exec_elf.c,v 1.38 2012/04/08 11:27:44 martin Exp $"); #ifdef _KERNEL_OPT #include "opt_pax.h" @@ -421,7 +421,8 @@ elf_load_file(struct lwp *l, struct exec p = l->l_proc; - if (p->p_vmspace) + KASSERT(p->p_vmspace); + if (__predict_true(p->p_vmspace != proc0.p_vmspace)) use_topdown = p->p_vmspace->vm_map.flags & VM_MAP_TOPDOWN; else #ifdef __USING_TOPDOWN_VM Index: src/sys/kern/kern_exec.c diff -u src/sys/kern/kern_exec.c:1.347 src/sys/kern/kern_exec.c:1.348 --- src/sys/kern/kern_exec.c:1.347 Tue Mar 13 18:40:52 2012 +++ src/sys/kern/kern_exec.c Sun Apr 8 11:27:44 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: kern_exec.c,v 1.347 2012/03/13 18:40:52 elad Exp $ */ +/* $NetBSD: kern_exec.c,v 1.348 2012/04/08 11:27:44 martin 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.347 2012/03/13 18:40:52 elad Exp $"); +__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.348 2012/04/08 11:27:44 martin Exp $"); #include "opt_exec.h" #include "opt_ktrace.h" @@ -237,14 +237,14 @@ struct execve_data { */ struct spawn_exec_data { struct execve_data sed_exec; - size_t sed_actions_len; - struct posix_spawn_file_actions_entry + struct posix_spawn_file_actions *sed_actions; struct posix_spawnattr *sed_attrs; struct proc *sed_parent; kcondvar_t sed_cv_child_ready; kmutex_t sed_mtx_child; int sed_error; + volatile uint32_t sed_refcnt; }; static void * @@ -842,8 +842,38 @@ 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) +execve_runproc(struct lwp *l, struct execve_data * restrict data, + bool no_local_exec_lock, bool is_spawn) { int error = 0; struct proc *p; @@ -856,15 +886,20 @@ execve_runproc(struct lwp *l, struct exe struct vmspace *vm; ksiginfo_t ksi; ksiginfoq_t kq; - bool proc_is_new; - KASSERT(rw_lock_held(&exec_lock)); + /* + * In case of a posix_spawn operation, the child doing the exec + * might not hold the reader lock on exec_lock, but the parent + * will do this instead. + */ + KASSERT(no_local_exec_lock || rw_lock_held(&exec_lock)); KASSERT(data != NULL); if (data == NULL) return (EINVAL); p = l->l_proc; - proc_is_new = p->p_vmspace == NULL; + if (no_local_exec_lock) + KASSERT(is_spawn); base_vcp = NULL; @@ -893,7 +928,12 @@ execve_runproc(struct lwp *l, struct exe * for remapping. Note that this might replace the current * vmspace with another! */ - uvmspace_exec(l, data->ed_pack.ep_vm_minaddr, data->ed_pack.ep_vm_maxaddr); + if (is_spawn) + uvmspace_spawn(l, data->ed_pack.ep_vm_minaddr, + data->ed_pack.ep_vm_maxaddr); + else + uvmspace_exec(l, data->ed_pack.ep_vm_minaddr, + data->ed_pack.ep_vm_maxaddr); /* record proc's vnode, for use by procfs and others */ if (p->p_textvp) @@ -1318,7 +1358,8 @@ execve_runproc(struct lwp *l, struct exe /* Allow new references from the debugger/procfs. */ rw_exit(&p->p_reflock); - rw_exit(&exec_lock); + if (!no_local_exec_lock) + rw_exit(&exec_lock); mutex_enter(proc_lock); @@ -1360,7 +1401,8 @@ execve_runproc(struct lwp *l, struct exe exec_abort: SDT_PROBE(proc,,,exec_failure, error, 0, 0, 0, 0); rw_exit(&p->p_reflock); - rw_exit(&exec_lock); + if (!no_local_exec_lock) + rw_exit(&exec_lock); pathbuf_stringcopy_put(data->ed_pathbuf, data->ed_pathstring); pathbuf_destroy(data->ed_pathbuf); @@ -1373,6 +1415,7 @@ execve_runproc(struct lwp *l, struct exe */ uvm_deallocate(&vm->vm_map, 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); @@ -1382,13 +1425,12 @@ execve_runproc(struct lwp *l, struct exe vrele(data->ed_pack.ep_interp); /* Acquire the sched-state mutex (exit1() will release it). */ - if (!proc_is_new) { + if (!is_spawn) { mutex_enter(p->p_lock); exit1(l, W_EXITCODE(error, SIGABRT)); } - /* NOTREACHED */ - return 0; + return error; } int @@ -1401,7 +1443,7 @@ execve1(struct lwp *l, const char *path, error = execve_loadvm(l, path, args, envs, fetch_element, &data); if (error) return error; - error = execve_runproc(l, &data); + error = execve_runproc(l, &data, false, false); return error; } @@ -1729,9 +1771,38 @@ exec_sigcode_map(struct proc *p, const s } /* + * Release a refcount on spawn_exec_data and destroy memory, if this + * was the last one. + */ +static void +spawn_exec_data_release(struct spawn_exec_data *data) +{ + if (atomic_dec_32_nv(&data->sed_refcnt) != 0) + return; + + cv_destroy(&data->sed_cv_child_ready); + mutex_destroy(&data->sed_mtx_child); + + if (data->sed_actions) + posix_spawn_fa_free(data->sed_actions, + data->sed_actions->len); + if (data->sed_attrs) + kmem_free(data->sed_attrs, + sizeof(*data->sed_attrs)); + kmem_free(data, sizeof(*data)); +} + +/* * A child lwp of a posix_spawn operation starts here and ends up in * cpu_spawn_return, dealing with all filedescriptor and scheduler * manipulations in between. + * The parent waits for the child, as it is not clear wether the child + * will be able to aquire its own exec_lock. If it can, the parent can + * be released early and continue running in parallel. If not (or if the + * magic debug flag is passed in the scheduler attribute struct), the + * child rides on the parent's exec lock untill it is ready to return to + * to userland - and only then releases the parent. This method loses + * concurrency, but improves error reporting. */ static void spawn_return(void *arg) @@ -1741,19 +1812,29 @@ spawn_return(void *arg) int error, newfd; size_t i; const struct posix_spawn_file_actions_entry *fae; + pid_t ppid; register_t retval; bool have_reflock; - - /* we have been created non-preemptable */ - KASSERT(l->l_nopreempt == 1); + bool parent_is_waiting = true; /* - * The following actions may block, so we need a temporary - * vmspace - borrow the kernel one - */ - l->l_proc->p_vmspace = proc0.p_vmspace; - pmap_activate(l); - KPREEMPT_ENABLE(l); + * Check if we can release parent early. + * We either need to have no sed_attrs, or sed_attrs does not + * have POSIX_SPAWN_RETURNERROR or one of the flags, that require + * safe access to the parent proc (passed in sed_parent). + * 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) + && rw_tryenter(&exec_lock, RW_READER)) { + parent_is_waiting = false; + mutex_enter(&spawn_data->sed_mtx_child); + cv_signal(&spawn_data->sed_cv_child_ready); + mutex_exit(&spawn_data->sed_mtx_child); + } /* don't allow debugger access yet */ rw_enter(&l->l_proc->p_reflock, RW_WRITER); @@ -1762,8 +1843,8 @@ spawn_return(void *arg) 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[i]; + 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) { @@ -1832,7 +1913,7 @@ spawn_return(void *arg) &spawn_data->sed_attrs->sa_schedparam); else if (spawn_data->sed_attrs->sa_flags & POSIX_SPAWN_SETSCHEDPARAM) { - error = do_sched_setparam(spawn_data->sed_parent->p_pid, 0, + error = do_sched_setparam(ppid, 0, SCHED_NONE, &spawn_data->sed_attrs->sa_schedparam); } if (error) @@ -1872,28 +1953,23 @@ spawn_return(void *arg) } } - /* stop using kernel vmspace */ - KPREEMPT_DISABLE(l); - pmap_deactivate(l); - l->l_proc->p_vmspace = NULL; - /* now do the real exec */ - rw_enter(&exec_lock, RW_READER); - error = execve_runproc(l, &spawn_data->sed_exec); + error = execve_runproc(l, &spawn_data->sed_exec, parent_is_waiting, + true); have_reflock = false; if (error == EJUSTRETURN) error = 0; else if (error) goto report_error; - /* we now have our own vmspace */ - KPREEMPT_ENABLE(l); - KASSERT(l->l_nopreempt == 0); + if (parent_is_waiting) { + mutex_enter(&spawn_data->sed_mtx_child); + cv_signal(&spawn_data->sed_cv_child_ready); + mutex_exit(&spawn_data->sed_mtx_child); + } - /* done, signal parent */ - mutex_enter(&spawn_data->sed_mtx_child); - cv_signal(&spawn_data->sed_cv_child_ready); - mutex_exit(&spawn_data->sed_mtx_child); + /* release our refcount on the data */ + spawn_exec_data_release(spawn_data); /* and finaly: leave to userland for the first time */ cpu_spawn_return(l); @@ -1902,30 +1978,35 @@ spawn_return(void *arg) return; report_error: - if (have_reflock) + if (have_reflock) rw_exit(&l->l_proc->p_reflock); - /* stop using kernel vmspace (if we haven't already) */ - if (l->l_proc->p_vmspace) { - KPREEMPT_DISABLE(l); - pmap_deactivate(l); - l->l_proc->p_vmspace = NULL; - /* do not enable preemption without vmspace */ + if (parent_is_waiting) { + /* pass error to parent */ + mutex_enter(&spawn_data->sed_mtx_child); + spawn_data->sed_error = error; + cv_signal(&spawn_data->sed_cv_child_ready); + mutex_exit(&spawn_data->sed_mtx_child); + } else { + rw_exit(&exec_lock); } - /* - * Set error value for parent to pick up (and take over ownership - * of spawn_data again), signal parent and exit this process. - */ - mutex_enter(&spawn_data->sed_mtx_child); - spawn_data->sed_error = error; - cv_signal(&spawn_data->sed_cv_child_ready); - mutex_exit(&spawn_data->sed_mtx_child); + /* release our refcount on the data */ + spawn_exec_data_release(spawn_data); + + /* done, exit */ mutex_enter(l->l_proc->p_lock); - exit1(l, W_EXITCODE(error, SIGABRT)); + /* + * Posix explicitly asks for 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. + */ + exit1(l, W_EXITCODE(127, SIGABRT)); } -static void +void posix_spawn_fa_free(struct posix_spawn_file_actions *fa, size_t len) { @@ -1935,7 +2016,7 @@ posix_spawn_fa_free(struct posix_spawn_f continue; kmem_free(fae->fae_path, strlen(fae->fae_path) + 1); } - if (fa->len) + if (fa->len > 0) kmem_free(fa->fae, sizeof(*fa->fae) * fa->len); kmem_free(fa, sizeof(*fa)); } @@ -1958,9 +2039,12 @@ posix_spawn_fa_alloc(struct posix_spawn_ goto out; } - if (fa->len == 0) + 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); @@ -1980,6 +2064,7 @@ posix_spawn_fa_alloc(struct posix_spawn_ memcpy(fae->fae_path, pbuf, fal); } PNBUF_PUT(pbuf); + *fap = fa; return 0; out: @@ -1990,29 +2075,11 @@ out: } int -sys_posix_spawn(struct lwp *l1, const struct sys_posix_spawn_args *uap, - register_t *retval) +check_posix_spawn(struct lwp *l1) { - /* { - syscallarg(pid_t *) pid; - syscallarg(const char *) path; - syscallarg(const struct posix_spawn_file_actions *) file_actions; - syscallarg(const struct posix_spawnattr *) attrp; - syscallarg(char *const *) argv; - syscallarg(char *const *) envp; - } */ - - struct proc *p1, *p2; - struct plimit *p1_lim; - struct lwp *l2; - int error = 0, tnprocs, count; - struct posix_spawn_file_actions *fa = NULL; - struct posix_spawnattr *sa = NULL; - struct spawn_exec_data *spawn_data; + int error, tnprocs, count; uid_t uid; - vaddr_t uaddr; - pid_t pid; - bool have_exec_lock = false; + struct proc *p1; p1 = l1->l_proc; uid = kauth_cred_getuid(l1->l_cred); @@ -2030,8 +2097,7 @@ sys_posix_spawn(struct lwp *l1, const st if (error) { atomic_dec_uint(&nprocs); - *retval = EAGAIN; - return 0; + return EAGAIN; } /* @@ -2042,32 +2108,45 @@ sys_posix_spawn(struct lwp *l1, const st p1, KAUTH_ARG(KAUTH_REQ_PROCESS_RLIMIT_BYPASS), &p1->p_rlimit[RLIMIT_NPROC], KAUTH_ARG(RLIMIT_NPROC)) != 0 && __predict_false(count > p1->p_rlimit[RLIMIT_NPROC].rlim_cur)) { - error = EAGAIN; - goto error_exit; + (void)chgproccnt(uid, -1); + atomic_dec_uint(&nprocs); + return EAGAIN; } - /* copy in file_actions struct */ - if (SCARG(uap, file_actions) != NULL) { - error = posix_spawn_fa_alloc(&fa, SCARG(uap, file_actions)); - if (error) - goto error_exit; - } + return 0; +} - /* copyin posix_spawnattr struct */ - if (SCARG(uap, attrp) != NULL) { - sa = kmem_alloc(sizeof(*sa), KM_SLEEP); - error = copyin(SCARG(uap, attrp), sa, sizeof(*sa)); - if (error) - goto error_exit; - } +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) +{ + + 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; + + /* Allocate and init spawn_data */ + spawn_data = kmem_zalloc(sizeof(*spawn_data), KM_SLEEP); + 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. */ - spawn_data = kmem_zalloc(sizeof(*spawn_data), KM_SLEEP); - error = execve_loadvm(l1, SCARG(uap, path), SCARG(uap, argv), - SCARG(uap, envp), execve_fetch_element, &spawn_data->sed_exec); + error = execve_loadvm(l1, path, argv, + envp, fetch, &spawn_data->sed_exec); if (error == EJUSTRETURN) error = 0; else if (error) @@ -2087,7 +2166,9 @@ sys_posix_spawn(struct lwp *l1, const st } /* - * Allocate new proc. Leave it's p_vmspace NULL for now. + * Allocate new proc. Borrow proc0 vmspace for it, we will + * replace it with its own before returning to userland + * in the child. * This is a point of no return, we will have to go through * the child proc to properly clean it up past this point. */ @@ -2103,7 +2184,7 @@ sys_posix_spawn(struct lwp *l1, const st (unsigned) ((char *)&p2->p_endzero - (char *)&p2->p_startzero)); memcpy(&p2->p_startcopy, &p1->p_startcopy, (unsigned) ((char *)&p2->p_endcopy - (char *)&p2->p_startcopy)); - p2->p_vmspace = NULL; + p2->p_vmspace = proc0.p_vmspace; CIRCLEQ_INIT(&p2->p_sigpend.sp_info); @@ -2145,10 +2226,9 @@ sys_posix_spawn(struct lwp *l1, const st * Note: p_limit (rlimit stuff) is copy-on-write, so normally * we just need increase pl_refcnt. */ - p1_lim = p1->p_limit; - if (!p1_lim->pl_writeable) { - lim_addref(p1_lim); - p2->p_limit = p1_lim; + if (!p1->p_limit->pl_writeable) { + lim_addref(p1->p_limit); + p2->p_limit = p1->p_limit; } else { p2->p_limit = lim_copy(p1->p_limit); } @@ -2200,17 +2280,10 @@ sys_posix_spawn(struct lwp *l1, const st /* * Prepare remaining parts of spawn data */ - if (fa && fa->len) { - spawn_data->sed_actions_len = fa->len; - spawn_data->sed_actions = fa->fae; - } - if (sa) - spawn_data->sed_attrs = sa; + spawn_data->sed_actions = fa; + spawn_data->sed_attrs = sa; spawn_data->sed_parent = p1; - 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); /* create LWP */ lwp_create(l1, p2, uaddr, 0, NULL, 0, spawn_return, spawn_data, @@ -2241,7 +2314,11 @@ sys_posix_spawn(struct lwp *l1, const st 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 /* * It's now safe for the scheduler and other processes to see the @@ -2283,23 +2360,76 @@ sys_posix_spawn(struct lwp *l1, const st mutex_exit(proc_lock); cv_wait(&spawn_data->sed_cv_child_ready, &spawn_data->sed_mtx_child); - mutex_exit(&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; - if (fa) - posix_spawn_fa_free(fa, fa->len); + *pid_res = pid; + return error; - if (sa) - kmem_free(sa, sizeof(*sa)); + 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); + + return error; +} - cv_destroy(&spawn_data->sed_cv_child_ready); - mutex_destroy(&spawn_data->sed_mtx_child); +int +sys_posix_spawn(struct lwp *l1, const struct sys_posix_spawn_args *uap, + register_t *retval) +{ + /* { + syscallarg(pid_t *) pid; + syscallarg(const char *) path; + syscallarg(const struct posix_spawn_file_actions *) file_actions; + syscallarg(const struct posix_spawnattr *) attrp; + syscallarg(char *const *) argv; + syscallarg(char *const *) envp; + } */ + + int error; + 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) { + *retval = error; + return 0; + } + + /* copy in file_actions struct */ + if (SCARG(uap, file_actions) != NULL) { + error = posix_spawn_fa_alloc(&fa, SCARG(uap, file_actions)); + if (error) + goto error_exit; + } + + /* copyin posix_spawnattr struct */ + if (SCARG(uap, attrp) != NULL) { + sa = kmem_alloc(sizeof(*sa), KM_SLEEP); + error = copyin(SCARG(uap, attrp), sa, sizeof(*sa)); + if (error) + goto error_exit; + } - kmem_free(spawn_data, sizeof(*spawn_data)); + /* + * Do the spawn + */ + error = do_posix_spawn(l1, &pid, &child_ok, SCARG(uap, path), fa, sa, + SCARG(uap, argv), SCARG(uap, envp), execve_fetch_element); + if (error) + goto error_exit; if (error == 0 && SCARG(uap, pid) != NULL) error = copyout(&pid, SCARG(uap, pid), sizeof(pid)); @@ -2308,17 +2438,15 @@ sys_posix_spawn(struct lwp *l1, const st return 0; error_exit: - if (have_exec_lock) - rw_exit(&exec_lock); - - if (fa) - posix_spawn_fa_free(fa, fa->len); - - if (sa) - kmem_free(sa, sizeof(*sa)); + if (!child_ok) { + (void)chgproccnt(kauth_cred_getuid(l1->l_cred), -1); + atomic_dec_uint(&nprocs); - (void)chgproccnt(uid, -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_exit.c diff -u src/sys/kern/kern_exit.c:1.237 src/sys/kern/kern_exit.c:1.238 --- src/sys/kern/kern_exit.c:1.237 Sun Feb 19 21:06:50 2012 +++ src/sys/kern/kern_exit.c Sun Apr 8 11:27:45 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: kern_exit.c,v 1.237 2012/02/19 21:06:50 rmind Exp $ */ +/* $NetBSD: kern_exit.c,v 1.238 2012/04/08 11:27:45 martin Exp $ */ /*- * Copyright (c) 1998, 1999, 2006, 2007, 2008 The NetBSD Foundation, Inc. @@ -67,7 +67,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: kern_exit.c,v 1.237 2012/02/19 21:06:50 rmind Exp $"); +__KERNEL_RCSID(0, "$NetBSD: kern_exit.c,v 1.238 2012/04/08 11:27:45 martin Exp $"); #include "opt_ktrace.h" #include "opt_perfctrs.h" @@ -202,6 +202,7 @@ exit1(struct lwp *l, int rv) p = l->l_proc; KASSERT(mutex_owned(p->p_lock)); + KASSERT(p->p_vmspace != NULL); if (__predict_false(p == initproc)) { panic("init died (signal %d, exit %d)", @@ -567,12 +568,8 @@ exit1(struct lwp *l, int rv) * case these resources are in the PCB. */ cpu_lwp_free(l, 1); - /* - * A new child of a posix_spawn operation might never have been - * to userland - no pmap deactivation is needed in this case - */ - if (__predict_true(l->l_proc->p_vmspace != NULL)) - pmap_deactivate(l); + + pmap_deactivate(l); /* This process no longer needs to hold the kernel lock. */ #ifdef notyet Index: src/sys/sys/exec.h diff -u src/sys/sys/exec.h:1.134 src/sys/sys/exec.h:1.135 --- src/sys/sys/exec.h:1.134 Fri Feb 3 20:11:54 2012 +++ src/sys/sys/exec.h Sun Apr 8 11:27:45 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: exec.h,v 1.134 2012/02/03 20:11:54 matt Exp $ */ +/* $NetBSD: exec.h,v 1.135 2012/04/08 11:27:45 martin Exp $ */ /*- * Copyright (c) 1992, 1993 @@ -273,6 +273,7 @@ int coredump_write (void *, enum uio_se void exec_free_emul_arg (struct exec_package *); + /* * Machine dependent functions */ @@ -296,6 +297,14 @@ typedef int (*execve_fetch_element_t)(ch int execve1(struct lwp *, const char *, char * const *, char * const *, execve_fetch_element_t); +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 *, + struct posix_spawn_file_actions *, struct posix_spawnattr *, + char *const *argv, char *const *, execve_fetch_element_t); + extern int maxexec; #endif /* _KERNEL */ Index: src/sys/sys/spawn.h diff -u src/sys/sys/spawn.h:1.1 src/sys/sys/spawn.h:1.2 --- src/sys/sys/spawn.h:1.1 Sat Feb 11 23:16:18 2012 +++ src/sys/sys/spawn.h Sun Apr 8 11:27:45 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: spawn.h,v 1.1 2012/02/11 23:16:18 martin Exp $ */ +/* $NetBSD: spawn.h,v 1.2 2012/04/08 11:27:45 martin Exp $ */ /*- * Copyright (c) 2008 Ed Schouten <e...@freebsd.org> @@ -67,8 +67,8 @@ typedef struct posix_spawn_file_actions_ } posix_spawn_file_actions_entry_t; struct posix_spawn_file_actions { - int size; /* size of fae array */ - int len; /* how many slots are used */ + unsigned int size; /* size of fae array */ + unsigned int len; /* how many slots are used */ posix_spawn_file_actions_entry_t *fae; }; @@ -81,6 +81,23 @@ typedef struct posix_spawn_file_actions #define POSIX_SPAWN_SETSCHEDULER 0x08 #define POSIX_SPAWN_SETSIGDEF 0x10 #define POSIX_SPAWN_SETSIGMASK 0x20 +/* + * 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 + * from that via the return value of the posix_spawn syscall. This is + * usefull for testing, as it can verify the generated error codes and + * match to the supposedly triggered failures. + * In general, the kernel will return from the posix_spawn syscall as + * early as possible, as soon as creating the new process is known to + * work. Errors might either be reported back via the return value in + * the parent, or (less explicit) by an error exit of the child + * process. Our test cases deal with both behaviours in the general + * case, but request the POSIX_SPAWN_RETURNERROR for some tests. + */ +#define POSIX_SPAWN_RETURNERROR 0x40 #endif /* !_SYS_SPAWN_H_ */ Index: src/sys/uvm/uvm_extern.h diff -u src/sys/uvm/uvm_extern.h:1.182 src/sys/uvm/uvm_extern.h:1.183 --- src/sys/uvm/uvm_extern.h:1.182 Sun Mar 18 13:31:14 2012 +++ src/sys/uvm/uvm_extern.h Sun Apr 8 11:27:45 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: uvm_extern.h,v 1.182 2012/03/18 13:31:14 uebayasi Exp $ */ +/* $NetBSD: uvm_extern.h,v 1.183 2012/04/08 11:27:45 martin Exp $ */ /* * Copyright (c) 1997 Charles D. Cranor and Washington University. @@ -647,6 +647,7 @@ struct vmspace *uvmspace_alloc(vaddr_t, void uvmspace_init(struct vmspace *, struct pmap *, vaddr_t, vaddr_t); void uvmspace_exec(struct lwp *, vaddr_t, vaddr_t); +void uvmspace_spawn(struct lwp *, vaddr_t, vaddr_t); struct vmspace *uvmspace_fork(struct vmspace *); void uvmspace_addref(struct vmspace *); void uvmspace_free(struct vmspace *); Index: src/sys/uvm/uvm_glue.c diff -u src/sys/uvm/uvm_glue.c:1.158 src/sys/uvm/uvm_glue.c:1.159 --- src/sys/uvm/uvm_glue.c:1.158 Fri Apr 6 17:16:30 2012 +++ src/sys/uvm/uvm_glue.c Sun Apr 8 11:27:45 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: uvm_glue.c,v 1.158 2012/04/06 17:16:30 chs Exp $ */ +/* $NetBSD: uvm_glue.c,v 1.159 2012/04/08 11:27:45 martin Exp $ */ /* * Copyright (c) 1997 Charles D. Cranor and Washington University. @@ -62,7 +62,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: uvm_glue.c,v 1.158 2012/04/06 17:16:30 chs Exp $"); +__KERNEL_RCSID(0, "$NetBSD: uvm_glue.c,v 1.159 2012/04/08 11:27:45 martin Exp $"); #include "opt_kgdb.h" #include "opt_kstack.h" @@ -419,19 +419,21 @@ uvm_proc_exit(struct proc *p) KASSERT(p == l->l_proc); ovm = p->p_vmspace; + KASSERT(ovm != NULL); + + if (__predict_false(ovm == proc0.p_vmspace)) + return; /* * borrow proc0's address space. */ KPREEMPT_DISABLE(l); - if (__predict_true(ovm != NULL)) - pmap_deactivate(l); + pmap_deactivate(l); p->p_vmspace = proc0.p_vmspace; pmap_activate(l); KPREEMPT_ENABLE(l); - if (__predict_true(ovm!=NULL)) - uvmspace_free(ovm); + uvmspace_free(ovm); } void Index: src/sys/uvm/uvm_map.c diff -u src/sys/uvm/uvm_map.c:1.316 src/sys/uvm/uvm_map.c:1.317 --- src/sys/uvm/uvm_map.c:1.316 Tue Mar 13 18:41:15 2012 +++ src/sys/uvm/uvm_map.c Sun Apr 8 11:27:45 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: uvm_map.c,v 1.316 2012/03/13 18:41:15 elad Exp $ */ +/* $NetBSD: uvm_map.c,v 1.317 2012/04/08 11:27:45 martin Exp $ */ /* * Copyright (c) 1997 Charles D. Cranor and Washington University. @@ -66,7 +66,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: uvm_map.c,v 1.316 2012/03/13 18:41:15 elad Exp $"); +__KERNEL_RCSID(0, "$NetBSD: uvm_map.c,v 1.317 2012/04/08 11:27:45 martin Exp $"); #include "opt_ddb.h" #include "opt_uvmhist.h" @@ -4010,6 +4010,28 @@ uvmspace_unshare(struct lwp *l) #endif + +/* + * uvmspace_spawn: a new process has been spawned and needs a vmspace + */ + +void +uvmspace_spawn(struct lwp *l, vaddr_t start, vaddr_t end) +{ + struct proc *p = l->l_proc; + struct vmspace *nvm; + +#ifdef __HAVE_CPU_VMSPACE_EXEC + cpu_vmspace_exec(l, start, end); +#endif + + nvm = uvmspace_alloc(start, end); + kpreempt_disable(); + p->p_vmspace = nvm; + pmap_activate(l); + kpreempt_enable(); +} + /* * uvmspace_exec: the process wants to exec a new program */ @@ -4021,23 +4043,11 @@ uvmspace_exec(struct lwp *l, vaddr_t sta struct vmspace *nvm, *ovm = p->p_vmspace; struct vm_map *map; + KASSERT(ovm != NULL); #ifdef __HAVE_CPU_VMSPACE_EXEC cpu_vmspace_exec(l, start, end); #endif - /* - * Special case: no vmspace yet (see posix_spawn) - - * no races possible in this case. - */ - if (ovm == NULL) { - ovm = uvmspace_alloc(start, end); - kpreempt_disable(); - p->p_vmspace = ovm; - pmap_activate(l); - kpreempt_enable(); - return; - } - map = &ovm->vm_map; /* * see if more than one process is using this vmspace... Index: src/tests/lib/libc/gen/posix_spawn/t_fileactions.c diff -u src/tests/lib/libc/gen/posix_spawn/t_fileactions.c:1.3 src/tests/lib/libc/gen/posix_spawn/t_fileactions.c:1.4 --- src/tests/lib/libc/gen/posix_spawn/t_fileactions.c:1.3 Mon Feb 20 12:22:40 2012 +++ src/tests/lib/libc/gen/posix_spawn/t_fileactions.c Sun Apr 8 11:27:46 2012 @@ -1,4 +1,4 @@ -/* $NetBSD: t_fileactions.c,v 1.3 2012/02/20 12:22:40 martin Exp $ */ +/* $NetBSD: t_fileactions.c,v 1.4 2012/04/08 11:27:46 martin Exp $ */ /*- * Copyright (c) 2012 The NetBSD Foundation, Inc. @@ -222,7 +222,7 @@ ATF_TC_HEAD(t_spawn_open_nonexistent, tc ATF_TC_BODY(t_spawn_open_nonexistent, tc) { - int err; + int err, status; pid_t pid; char * const args[2] = { __UNCONST("cat"), NULL }; posix_spawn_file_actions_t fa; @@ -231,8 +231,58 @@ ATF_TC_BODY(t_spawn_open_nonexistent, tc posix_spawn_file_actions_addopen(&fa, STDIN_FILENO, "./non/ex/ist/ent", O_RDONLY, 0); err = posix_spawn(&pid, "/bin/cat", &fa, NULL, args, NULL); + if (err == 0) { + /* + * The child has been created - it should fail and + * return exit code 127 + */ + waitpid(pid, &status, 0); + ATF_REQUIRE(WEXITSTATUS(status) == 127); + + } else { + /* + * The error has been noticed early enough, no child has + * been run + */ + ATF_REQUIRE(err == ENOENT); + } + posix_spawn_file_actions_destroy(&fa); +} + +ATF_TC(t_spawn_open_nonexistent_diag); + +ATF_TC_HEAD(t_spawn_open_nonexistent_diag, tc) +{ + atf_tc_set_md_var(tc, "descr", + "posix_spawn fails when a file to open does not exist " + "and delivers proper diagnostic"); + atf_tc_set_md_var(tc, "require.progs", "/bin/cat"); +} + +ATF_TC_BODY(t_spawn_open_nonexistent_diag, tc) +{ + int err; + pid_t pid; + char * const args[2] = { __UNCONST("cat"), NULL }; + posix_spawnattr_t attr; + posix_spawn_file_actions_t fa; + + posix_spawnattr_init(&attr); + /* + * POSIX_SPAWN_RETURNERROR is a NetBSD specific flag that + * will cause a "proper" return value from posix_spawn(2) + * instead of a (potential) success there and a 127 exit + * status from the child process (c.f. the non-diag variant + * of this test). + */ + posix_spawnattr_setflags(&attr, POSIX_SPAWN_RETURNERROR); + posix_spawn_file_actions_init(&fa); + posix_spawn_file_actions_addopen(&fa, STDIN_FILENO, + "./non/ex/ist/ent", O_RDONLY, 0); + err = posix_spawn(&pid, "/bin/cat", &fa, &attr, args, NULL); ATF_REQUIRE(err == ENOENT); posix_spawn_file_actions_destroy(&fa); + posix_spawnattr_destroy(&attr); } ATF_TC(t_spawn_fileactions); @@ -327,6 +377,7 @@ ATF_TP_ADD_TCS(tp) { ATF_TP_ADD_TC(tp, t_spawn_fileactions); ATF_TP_ADD_TC(tp, t_spawn_open_nonexistent); + ATF_TP_ADD_TC(tp, t_spawn_open_nonexistent_diag); ATF_TP_ADD_TC(tp, t_spawn_reopen); ATF_TP_ADD_TC(tp, t_spawn_openmode); ATF_TP_ADD_TC(tp, t_spawn_empty_fileactions);