diff options
author | jeff <jeff@FreeBSD.org> | 2002-08-13 06:55:28 +0000 |
---|---|---|
committer | jeff <jeff@FreeBSD.org> | 2002-08-13 06:55:28 +0000 |
commit | a996673e123dab0805b05084774227cae72d75e5 (patch) | |
tree | 9e630263213e8fdda8c938a92f150220b0eb745d /sys/kern | |
parent | 216ea61cb629b077f8cf1b29f90f2c08ff1b1eea (diff) | |
download | FreeBSD-src-a996673e123dab0805b05084774227cae72d75e5.zip FreeBSD-src-a996673e123dab0805b05084774227cae72d75e5.tar.gz |
- Hold the vnode lock throughout execve.
- Set VV_TEXT in the top level execve code.
- Fixup the image activators to deal with the newly locked vnode.
Diffstat (limited to 'sys/kern')
-rw-r--r-- | sys/kern/imgact_aout.c | 4 | ||||
-rw-r--r-- | sys/kern/imgact_elf.c | 13 | ||||
-rw-r--r-- | sys/kern/kern_exec.c | 47 |
3 files changed, 39 insertions, 25 deletions
diff --git a/sys/kern/imgact_aout.c b/sys/kern/imgact_aout.c index ba2f76b..48eed90 100644 --- a/sys/kern/imgact_aout.c +++ b/sys/kern/imgact_aout.c @@ -239,10 +239,6 @@ exec_aout_imgact(imgp) imgp->proc->p_sysent = &aout_sysvec; - /* Indicate that this file should not be modified */ - mp_fixme("Unlocked vflag access."); - imgp->vp->v_vflag |= VV_TEXT; - return (0); } diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c index 92e86af..7d79c1d 100644 --- a/sys/kern/imgact_elf.c +++ b/sys/kern/imgact_elf.c @@ -599,6 +599,7 @@ __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp) const char *interp = NULL; Elf_Brandinfo *brand_info; char *path; + struct thread *td = curthread; GIANT_REQUIRED; @@ -624,16 +625,7 @@ __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp) * From this point on, we may have resources that need to be freed. */ - /* - * Yeah, I'm paranoid. There is every reason in the world to get - * VTEXT now since from here on out, there are places we can have - * a context switch. Better safe than sorry; I really don't want - * the file to change while it's being loaded. - * - * XXX We can't really set this flag safely without the vnode lock. - */ - mp_fixme("This needs the vnode lock to be safe."); - imgp->vp->v_vflag |= VV_TEXT; + VOP_UNLOCK(imgp->vp, 0, td); if ((error = exec_extract_strings(imgp)) != 0) goto fail; @@ -846,6 +838,7 @@ __CONCAT(exec_, __elfN(imgact))(struct image_params *imgp) imgp->interpreted = 0; fail: + vn_lock(imgp->vp, LK_EXCLUSIVE | LK_RETRY, td); return error; } diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index 0d6689f..c01f4fa 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -148,6 +148,7 @@ execve(td, uap) #endif struct vnode *textvp = NULL; int credential_changing; + int textset; imgp = &image_params; @@ -227,15 +228,23 @@ interpret: * Check file permissions (also 'opens' file) */ error = exec_check_permissions(imgp); - if (error) { - VOP_UNLOCK(imgp->vp, 0, td); + if (error) goto exec_fail_dealloc; - } - VOP_GETVOBJECT(imgp->vp, &imgp->object); - vm_object_reference(imgp->object); + + if (VOP_GETVOBJECT(imgp->vp, &imgp->object) == 0) + vm_object_reference(imgp->object); + + /* + * Set VV_TEXT now so no one can write to the executable while we're + * activating it. + * + * Remember if this was set before and unset it in case this is not + * actually an executable image. + */ + textset = imgp->vp->v_vflag & VV_TEXT; + imgp->vp->v_vflag |= VV_TEXT; error = exec_map_first_page(imgp); - VOP_UNLOCK(imgp->vp, 0, td); if (error) goto exec_fail_dealloc; @@ -262,8 +271,11 @@ interpret: } if (error) { - if (error == -1) + if (error == -1) { + if (textset == 0) + imgp->vp->v_vflag &= ~VV_TEXT; error = ENOEXEC; + } goto exec_fail_dealloc; } @@ -273,9 +285,16 @@ interpret: */ if (imgp->interpreted) { exec_unmap_first_page(imgp); + /* + * VV_TEXT needs to be unset for scripts. There is a short + * period before we determine that something is a script where + * VV_TEXT will be set. The vnode lock is held over this + * entire period so nothing should illegitimately be blocked. + */ + imgp->vp->v_vflag &= ~VV_TEXT; /* free name buffer and old vnode */ NDFREE(ndp, NDF_ONLY_PNBUF); - vrele(ndp->ni_vp); + vput(ndp->ni_vp); vm_object_deallocate(imgp->object); imgp->object = NULL; /* set new name to that of the interpreter */ @@ -329,6 +348,9 @@ interpret: /* close files on exec */ fdcloseexec(td); + /* Get a reference to the vnode prior to locking the proc */ + VREF(ndp->ni_vp); + /* * For security and other reasons, signal handlers cannot * be shared after an exec. The new process gets a copy of the old @@ -453,10 +475,10 @@ interpret: } /* - * Store the vp for use in procfs + * Store the vp for use in procfs. This vnode was referenced prior + * to locking the proc lock. */ textvp = p->p_textvp; - VREF(ndp->ni_vp); p->p_textvp = ndp->ni_vp; /* @@ -499,6 +521,7 @@ interpret: done1: PROC_UNLOCK(p); + /* * Free any resources malloc'd earlier that we didn't use. */ @@ -512,6 +535,8 @@ done1: */ if (textvp != NULL) vrele(textvp); + if (ndp->ni_vp && error != 0) + vrele(ndp->ni_vp); #ifdef KTRACE if (tracevp != NULL) vrele(tracevp); @@ -535,7 +560,7 @@ exec_fail_dealloc: if (imgp->vp) { NDFREE(ndp, NDF_ONLY_PNBUF); - vrele(imgp->vp); + vput(imgp->vp); } if (imgp->object) |