From 30a231055bbecc02f17f1f4fe10c32564b6bcb22 Mon Sep 17 00:00:00 2001 From: jeff Date: Wed, 1 Feb 2006 00:25:26 +0000 Subject: - 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 --- sys/nfsserver/nfs_serv.c | 75 ++++++++++++++++++++++----------------------- sys/nfsserver/nfs_srvsubs.c | 2 +- 2 files changed, 38 insertions(+), 39 deletions(-) (limited to 'sys/nfsserver') diff --git a/sys/nfsserver/nfs_serv.c b/sys/nfsserver/nfs_serv.c index 0b0aaa3..8117cbb 100644 --- a/sys/nfsserver/nfs_serv.c +++ b/sys/nfsserver/nfs_serv.c @@ -664,13 +664,13 @@ nfsmout: 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 @@ nfsmout: 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 @@ nfsmout: } 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 @@ out: 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 @@ nfsmout: 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 @@ nfsmout: } 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 @@ nfsmout: 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 @@ nfsmout: } 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 @@ nfsmout: 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 @@ nfsmout: 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 @@ nfsmout: 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 @@ nfsmout: else vrele(nd.ni_vp); } + if (dirp) + vrele(dirp); vn_finished_write(mp); mtx_unlock(&Giant); /* VFS */ NFSD_LOCK(); @@ -3259,8 +3258,6 @@ nfsmout: 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 @@ nfsmout: } if (nd.ni_vp) vput(nd.ni_vp); + if (dirp) + vrele(dirp); vn_finished_write(mp); mtx_unlock(&Giant); /* VFS */ diff --git a/sys/nfsserver/nfs_srvsubs.c b/sys/nfsserver/nfs_srvsubs.c index 437dd63..1aa976a 100644 --- a/sys/nfsserver/nfs_srvsubs.c +++ b/sys/nfsserver/nfs_srvsubs.c @@ -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; -- cgit v1.1