- Reorder calls to vrele() after calls to vput() when the vrele is a

directory.  vrele() may lock the passed vnode, which in these cases would
   give an invalid lock order of child -> parent.  These situations are
   deadlock prone although do not typically deadlock because the vrele
   is typically not releasing the last reference to the vnode.  Users of
   vrele must consider it as a call to vn_lock() and order it appropriately.

MFC After: 	1 week
Sponsored by:	Isilon Systems, Inc.
Tested by:	kkenn
This commit is contained in:
Jeff Roberson 2006-02-01 00:25:26 +00:00
parent f2ca64ca71
commit 89b0e10910
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=155160
8 changed files with 81 additions and 88 deletions

View File

@ -1289,21 +1289,18 @@ coda_rename(struct vop_rename_args *ap)
/* XXX - do we need to call cache pureg on the moved vnode? */
cache_purge(ap->a_fvp);
/* It seems to be incumbent on us to drop locks on all four vnodes */
/* From-vnodes are not locked, only ref'd. To-vnodes are locked. */
vrele(ap->a_fvp);
/* Release parents first, then children. */
vrele(odvp);
if (ap->a_tvp) {
if (ap->a_tvp == ndvp) {
vrele(ap->a_tvp);
} else {
vput(ap->a_tvp);
}
}
if (ap->a_tvp == ndvp)
vrele(ndvp);
else
vput(ndvp);
vput(ap->a_tvp);
} else
vput(ndvp);
vrele(ap->a_fvp);
vput(ndvp);
return(error);
}

View File

@ -1289,21 +1289,18 @@ coda_rename(struct vop_rename_args *ap)
/* XXX - do we need to call cache pureg on the moved vnode? */
cache_purge(ap->a_fvp);
/* It seems to be incumbent on us to drop locks on all four vnodes */
/* From-vnodes are not locked, only ref'd. To-vnodes are locked. */
vrele(ap->a_fvp);
/* Release parents first, then children. */
vrele(odvp);
if (ap->a_tvp) {
if (ap->a_tvp == ndvp) {
vrele(ap->a_tvp);
} else {
vput(ap->a_tvp);
}
}
if (ap->a_tvp == ndvp)
vrele(ndvp);
else
vput(ndvp);
vput(ap->a_tvp);
} else
vput(ndvp);
vrele(ap->a_fvp);
vput(ndvp);
return(error);
}

View File

@ -1122,10 +1122,10 @@ msdosfs_rename(ap)
*/
if (doingdirectory)
panic("rename: lost dir entry");
vrele(ap->a_fvp);
if (newparent)
VOP_UNLOCK(tdvp, 0, td);
vrele(tdvp);
vrele(ap->a_fvp);
return 0;
}
xp = VTODE(fvp);
@ -1143,10 +1143,10 @@ msdosfs_rename(ap)
if (xp != ip) {
if (doingdirectory)
panic("rename: lost dir entry");
vrele(ap->a_fvp);
VOP_UNLOCK(fvp, 0, td);
if (newparent)
VOP_UNLOCK(fdvp, 0, td);
vrele(ap->a_fvp);
xp = NULL;
} else {
vrele(fvp);

View File

@ -1185,11 +1185,11 @@ kern_mknod(struct thread *td, char *path, enum uio_seg pathseg, int mode,
vp = nd.ni_vp;
if (vp != NULL) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vrele(vp);
if (vp == nd.ni_dvp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vrele(vp);
VFS_UNLOCK_GIANT(vfslocked);
return (EEXIST);
} else {
@ -1288,11 +1288,11 @@ kern_mkfifo(struct thread *td, char *path, enum uio_seg pathseg, int mode)
vfslocked = NDHASGIANT(&nd);
if (nd.ni_vp != NULL) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vrele(nd.ni_vp);
if (nd.ni_vp == nd.ni_dvp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vrele(nd.ni_vp);
VFS_UNLOCK_GIANT(vfslocked);
return (EEXIST);
}
@ -1425,11 +1425,11 @@ kern_link(struct thread *td, char *path, char *link, enum uio_seg segflg)
if ((error = namei(&nd)) == 0) {
lvfslocked = NDHASGIANT(&nd);
if (nd.ni_vp != NULL) {
vrele(nd.ni_vp);
if (nd.ni_dvp == nd.ni_vp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vrele(nd.ni_vp);
error = EEXIST;
} else if ((error = vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td))
== 0) {
@ -1502,11 +1502,11 @@ kern_symlink(struct thread *td, char *path, char *link, enum uio_seg segflg)
vfslocked = NDHASGIANT(&nd);
if (nd.ni_vp) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vrele(nd.ni_vp);
if (nd.ni_vp == nd.ni_dvp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vrele(nd.ni_vp);
VFS_UNLOCK_GIANT(vfslocked);
error = EEXIST;
goto out;
@ -1573,12 +1573,12 @@ undelete(td, uap)
if (nd.ni_vp != NULLVP || !(nd.ni_cnd.cn_flags & ISWHITEOUT)) {
NDFREE(&nd, NDF_ONLY_PNBUF);
if (nd.ni_vp)
vrele(nd.ni_vp);
if (nd.ni_vp == nd.ni_dvp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
if (nd.ni_vp)
vrele(nd.ni_vp);
VFS_UNLOCK_GIANT(vfslocked);
return (EEXIST);
}
@ -1650,11 +1650,11 @@ kern_unlink(struct thread *td, char *path, enum uio_seg pathseg)
if (error == 0) {
if (vn_start_write(nd.ni_dvp, &mp, V_NOWAIT) != 0) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vput(nd.ni_dvp);
if (vp == nd.ni_dvp)
vrele(vp);
else
vput(vp);
vput(nd.ni_dvp);
VFS_UNLOCK_GIANT(vfslocked);
if ((error = vn_start_write(NULL, &mp,
V_XSLEEP | PCATCH)) != 0)
@ -1675,11 +1675,11 @@ kern_unlink(struct thread *td, char *path, enum uio_seg pathseg)
vn_finished_write(mp);
}
NDFREE(&nd, NDF_ONLY_PNBUF);
vput(nd.ni_dvp);
if (vp == nd.ni_dvp)
vrele(vp);
else
vput(vp);
vput(nd.ni_dvp);
VFS_UNLOCK_GIANT(vfslocked);
return (error);
}
@ -3322,7 +3322,6 @@ kern_mkdir(struct thread *td, char *path, enum uio_seg segflg, int mode)
vp = nd.ni_vp;
if (vp != NULL) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vrele(vp);
/*
* XXX namei called with LOCKPARENT but not LOCKLEAF has
* the strange behaviour of leaving the vnode unlocked
@ -3332,6 +3331,7 @@ kern_mkdir(struct thread *td, char *path, enum uio_seg segflg, int mode)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vrele(vp);
VFS_UNLOCK_GIANT(vfslocked);
return (EEXIST);
}
@ -3429,11 +3429,11 @@ kern_rmdir(struct thread *td, char *path, enum uio_seg pathseg)
#endif
if (vn_start_write(nd.ni_dvp, &mp, V_NOWAIT) != 0) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vput(vp);
if (nd.ni_dvp == vp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vput(vp);
VFS_UNLOCK_GIANT(vfslocked);
if ((error = vn_start_write(NULL, &mp, V_XSLEEP | PCATCH)) != 0)
return (error);
@ -3445,11 +3445,11 @@ kern_rmdir(struct thread *td, char *path, enum uio_seg pathseg)
vn_finished_write(mp);
out:
NDFREE(&nd, NDF_ONLY_PNBUF);
vput(vp);
if (nd.ni_dvp == vp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vput(vp);
VFS_UNLOCK_GIANT(vfslocked);
return (error);
}

View File

@ -1185,11 +1185,11 @@ kern_mknod(struct thread *td, char *path, enum uio_seg pathseg, int mode,
vp = nd.ni_vp;
if (vp != NULL) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vrele(vp);
if (vp == nd.ni_dvp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vrele(vp);
VFS_UNLOCK_GIANT(vfslocked);
return (EEXIST);
} else {
@ -1288,11 +1288,11 @@ kern_mkfifo(struct thread *td, char *path, enum uio_seg pathseg, int mode)
vfslocked = NDHASGIANT(&nd);
if (nd.ni_vp != NULL) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vrele(nd.ni_vp);
if (nd.ni_vp == nd.ni_dvp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vrele(nd.ni_vp);
VFS_UNLOCK_GIANT(vfslocked);
return (EEXIST);
}
@ -1425,11 +1425,11 @@ kern_link(struct thread *td, char *path, char *link, enum uio_seg segflg)
if ((error = namei(&nd)) == 0) {
lvfslocked = NDHASGIANT(&nd);
if (nd.ni_vp != NULL) {
vrele(nd.ni_vp);
if (nd.ni_dvp == nd.ni_vp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vrele(nd.ni_vp);
error = EEXIST;
} else if ((error = vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td))
== 0) {
@ -1502,11 +1502,11 @@ kern_symlink(struct thread *td, char *path, char *link, enum uio_seg segflg)
vfslocked = NDHASGIANT(&nd);
if (nd.ni_vp) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vrele(nd.ni_vp);
if (nd.ni_vp == nd.ni_dvp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vrele(nd.ni_vp);
VFS_UNLOCK_GIANT(vfslocked);
error = EEXIST;
goto out;
@ -1573,12 +1573,12 @@ undelete(td, uap)
if (nd.ni_vp != NULLVP || !(nd.ni_cnd.cn_flags & ISWHITEOUT)) {
NDFREE(&nd, NDF_ONLY_PNBUF);
if (nd.ni_vp)
vrele(nd.ni_vp);
if (nd.ni_vp == nd.ni_dvp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
if (nd.ni_vp)
vrele(nd.ni_vp);
VFS_UNLOCK_GIANT(vfslocked);
return (EEXIST);
}
@ -1650,11 +1650,11 @@ kern_unlink(struct thread *td, char *path, enum uio_seg pathseg)
if (error == 0) {
if (vn_start_write(nd.ni_dvp, &mp, V_NOWAIT) != 0) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vput(nd.ni_dvp);
if (vp == nd.ni_dvp)
vrele(vp);
else
vput(vp);
vput(nd.ni_dvp);
VFS_UNLOCK_GIANT(vfslocked);
if ((error = vn_start_write(NULL, &mp,
V_XSLEEP | PCATCH)) != 0)
@ -1675,11 +1675,11 @@ kern_unlink(struct thread *td, char *path, enum uio_seg pathseg)
vn_finished_write(mp);
}
NDFREE(&nd, NDF_ONLY_PNBUF);
vput(nd.ni_dvp);
if (vp == nd.ni_dvp)
vrele(vp);
else
vput(vp);
vput(nd.ni_dvp);
VFS_UNLOCK_GIANT(vfslocked);
return (error);
}
@ -3322,7 +3322,6 @@ kern_mkdir(struct thread *td, char *path, enum uio_seg segflg, int mode)
vp = nd.ni_vp;
if (vp != NULL) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vrele(vp);
/*
* XXX namei called with LOCKPARENT but not LOCKLEAF has
* the strange behaviour of leaving the vnode unlocked
@ -3332,6 +3331,7 @@ kern_mkdir(struct thread *td, char *path, enum uio_seg segflg, int mode)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vrele(vp);
VFS_UNLOCK_GIANT(vfslocked);
return (EEXIST);
}
@ -3429,11 +3429,11 @@ kern_rmdir(struct thread *td, char *path, enum uio_seg pathseg)
#endif
if (vn_start_write(nd.ni_dvp, &mp, V_NOWAIT) != 0) {
NDFREE(&nd, NDF_ONLY_PNBUF);
vput(vp);
if (nd.ni_dvp == vp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vput(vp);
VFS_UNLOCK_GIANT(vfslocked);
if ((error = vn_start_write(NULL, &mp, V_XSLEEP | PCATCH)) != 0)
return (error);
@ -3445,11 +3445,11 @@ kern_rmdir(struct thread *td, char *path, enum uio_seg pathseg)
vn_finished_write(mp);
out:
NDFREE(&nd, NDF_ONLY_PNBUF);
vput(vp);
if (nd.ni_dvp == vp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
vput(vp);
VFS_UNLOCK_GIANT(vfslocked);
return (error);
}

View File

@ -664,13 +664,13 @@ nfsrv_lookup(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
NFSD_LOCK_ASSERT();
NFSD_UNLOCK();
mtx_lock(&Giant); /* VFS */
if (ndp->ni_vp)
vput(ndp->ni_vp);
if (dirp)
vrele(dirp);
NDFREE(&nd, NDF_ONLY_PNBUF);
if (ndp->ni_startdir)
vrele(ndp->ni_startdir);
if (ndp->ni_vp)
vput(ndp->ni_vp);
mtx_unlock(&Giant); /* VFS */
NFSD_LOCK();
return (error);
@ -2004,13 +2004,6 @@ nfsrv_create(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
NFSD_LOCK_ASSERT();
NFSD_UNLOCK();
mtx_lock(&Giant); /* VFS */
if (nd.ni_startdir) {
vrele(nd.ni_startdir);
nd.ni_startdir = NULL;
}
if (dirp)
vrele(dirp);
NDFREE(&nd, NDF_ONLY_PNBUF);
if (nd.ni_dvp) {
if (nd.ni_dvp == nd.ni_vp)
vrele(nd.ni_dvp);
@ -2019,6 +2012,13 @@ nfsrv_create(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
}
if (nd.ni_vp)
vput(nd.ni_vp);
if (nd.ni_startdir) {
vrele(nd.ni_startdir);
nd.ni_startdir = NULL;
}
if (dirp)
vrele(dirp);
NDFREE(&nd, NDF_ONLY_PNBUF);
vn_finished_write(mp);
mtx_unlock(&Giant); /* VFS */
NFSD_LOCK();
@ -2162,18 +2162,6 @@ nfsrv_mknod(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
*/
out:
NFSD_UNLOCK_ASSERT();
if (nd.ni_startdir) {
vrele(nd.ni_startdir);
nd.ni_startdir = NULL;
}
NDFREE(&nd, NDF_ONLY_PNBUF);
if (nd.ni_dvp) {
if (nd.ni_dvp == nd.ni_vp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
nd.ni_dvp = NULL;
}
vp = nd.ni_vp;
if (!error) {
bzero((caddr_t)fhp, sizeof(nfh));
@ -2182,11 +2170,23 @@ nfsrv_mknod(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
if (!error)
error = VOP_GETATTR(vp, vap, cred, td);
}
if (nd.ni_dvp) {
if (nd.ni_dvp == nd.ni_vp)
vrele(nd.ni_dvp);
else
vput(nd.ni_dvp);
nd.ni_dvp = NULL;
}
if (vp) {
vput(vp);
vp = NULL;
nd.ni_vp = NULL;
}
if (nd.ni_startdir) {
vrele(nd.ni_startdir);
nd.ni_startdir = NULL;
}
NDFREE(&nd, NDF_ONLY_PNBUF);
if (dirp) {
vn_lock(dirp, LK_EXCLUSIVE | LK_RETRY, td);
diraft_ret = VOP_GETATTR(dirp, &diraft, cred, td);
@ -2214,11 +2214,6 @@ nfsrv_mknod(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
NFSD_LOCK_ASSERT();
NFSD_UNLOCK();
mtx_lock(&Giant); /* VFS */
if (dirp)
vrele(dirp);
if (nd.ni_startdir)
vrele(nd.ni_startdir);
NDFREE(&nd, NDF_ONLY_PNBUF);
if (nd.ni_dvp) {
if (nd.ni_dvp == nd.ni_vp)
vrele(nd.ni_dvp);
@ -2227,6 +2222,11 @@ nfsrv_mknod(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
}
if (nd.ni_vp)
vput(nd.ni_vp);
if (dirp)
vrele(dirp);
if (nd.ni_startdir)
vrele(nd.ni_startdir);
NDFREE(&nd, NDF_ONLY_PNBUF);
vn_finished_write(mp);
mtx_unlock(&Giant); /* VFS */
NFSD_LOCK();
@ -2577,11 +2577,6 @@ nfsrv_rename(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
NFSD_LOCK_ASSERT();
NFSD_UNLOCK();
mtx_lock(&Giant); /* VFS */
if (tdirp)
vrele(tdirp);
if (tond.ni_startdir)
vrele(tond.ni_startdir);
NDFREE(&tond, NDF_ONLY_PNBUF);
if (tond.ni_dvp) {
if (tond.ni_dvp == tond.ni_vp)
vrele(tond.ni_dvp);
@ -2590,7 +2585,11 @@ nfsrv_rename(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
}
if (tond.ni_vp)
vput(tond.ni_vp);
if (tdirp)
vrele(tdirp);
if (tond.ni_startdir)
vrele(tond.ni_startdir);
NDFREE(&tond, NDF_ONLY_PNBUF);
/*
* Clear out fromnd related fields
*/
@ -2751,8 +2750,6 @@ nfsrv_link(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
NFSD_UNLOCK();
mtx_lock(&Giant); /* VFS */
NDFREE(&nd, NDF_ONLY_PNBUF);
if (dirp)
vrele(dirp);
if (vp)
vput(vp);
if (nd.ni_dvp) {
@ -2761,6 +2758,8 @@ nfsrv_link(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
else
vput(nd.ni_dvp);
}
if (dirp)
vrele(dirp);
if (nd.ni_vp)
vrele(nd.ni_vp);
vn_finished_write(mp);
@ -3117,8 +3116,6 @@ nfsrv_mkdir(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
NFSD_LOCK_ASSERT();
NFSD_UNLOCK();
mtx_lock(&Giant); /* VFS */
if (dirp)
vrele(dirp);
if (nd.ni_dvp) {
NDFREE(&nd, NDF_ONLY_PNBUF);
if (nd.ni_dvp == nd.ni_vp && vpexcl)
@ -3132,6 +3129,8 @@ nfsrv_mkdir(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
else
vrele(nd.ni_vp);
}
if (dirp)
vrele(dirp);
vn_finished_write(mp);
mtx_unlock(&Giant); /* VFS */
NFSD_LOCK();
@ -3259,8 +3258,6 @@ nfsrv_rmdir(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
NFSD_UNLOCK();
mtx_lock(&Giant); /* VFS */
NDFREE(&nd, NDF_ONLY_PNBUF);
if (dirp)
vrele(dirp);
if (nd.ni_dvp) {
if (nd.ni_dvp == nd.ni_vp)
vrele(nd.ni_dvp);
@ -3269,6 +3266,8 @@ nfsrv_rmdir(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp,
}
if (nd.ni_vp)
vput(nd.ni_vp);
if (dirp)
vrele(dirp);
vn_finished_write(mp);
mtx_unlock(&Giant); /* VFS */

View File

@ -831,8 +831,8 @@ nfs_namei(struct nameidata *ndp, fhandle_t *fhp, int len,
if (ndp->ni_pathlen > 1)
uma_zfree(namei_zone, cp);
badlink2:
vrele(ndp->ni_dvp);
vput(ndp->ni_vp);
vrele(ndp->ni_dvp);
break;
}
linklen = MAXPATHLEN - auio.uio_resid;

View File

@ -469,8 +469,8 @@ ufs_extattr_autostart(struct mount *mp, struct thread *td)
}
if (rvp == attr_dvp) {
/* Should never happen. */
vrele(attr_dvp);
vput(rvp);
vrele(attr_dvp);
return (EINVAL);
}
vrele(rvp);