Subject: Re: kern/35542: NFS rename(?) panics (panic: lockmgr: release of unlocked lock!)
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Chuck Silvers <chuq@chuq.com>
List: netbsd-bugs
Date: 02/03/2007 23:20:02
The following reply was made to PR kern/35542; it has been noted by GNATS.

From: Chuck Silvers <chuq@chuq.com>
To: Antti Kantee <pooka@cs.hut.fi>, gnats-bugs@NetBSD.org,
	arto@selonen.org, netbsd-bugs@netbsd.org
Cc: 
Subject: Re: kern/35542: NFS rename(?) panics (panic: lockmgr: release of unlocked lock!)
Date: Sat, 3 Feb 2007 15:19:27 -0800

 On Sat, Feb 03, 2007 at 05:15:57PM +0200, Antti Kantee wrote:
 > Seems the problem is caused when attempting to rename stuff without
 > enough permissions.  What's happening under the hood is that nfs_namei()
 > now releases the lock on the directory vnode in case of an error, so
 > releasing it again in nfsrv_rename() causes the panic.
 
 dirp can be non-NULL even when nfs_namei() returns an error,
 so we just need to change the errant vput() back to vrele(),
 like it was before my changes.  the usage of dirp here would then
 match all the other callers of nfs_namei().  the subsequent call
 that I added to unlock dirp should really refer to fromnd.ni_dvp
 (which for non-WebNFS is always the same vnode as dirp, which is
 why we didn't notice).
 
 looking more closely at nfs_namei(), in the WebNFS error cases
 we actually release the hold on the returned dirp, so we ought to
 fix that as well while we're here.
 
 oh, and I see that the fix you made to rename_files() is needed
 in nfsrv_rename() as well.
 
 here's the diff for all these:
 
 Index: nfs/nfs_serv.c
 ===================================================================
 RCS file: /cvsroot/src/sys/nfs/nfs_serv.c,v
 retrieving revision 1.122
 diff -u -p -r1.122 nfs_serv.c
 --- nfs/nfs_serv.c	4 Jan 2007 20:24:08 -0000	1.122
 +++ nfs/nfs_serv.c	3 Feb 2007 17:41:00 -0000
 @@ -1898,11 +1898,13 @@ nfsrv_rename(nfsd, slp, lwp, mrq)
  		nfsm_srvwcc_data(fdirfor_ret, &fdirfor, fdiraft_ret, &fdiraft);
  		nfsm_srvwcc_data(tdirfor_ret, &tdirfor, tdiraft_ret, &tdiraft);
  		if (fdirp)
 -			vput(fdirp);
 +			vrele(fdirp);
  		vn_finished_write(mp, 0);
  		return (0);
  	}
 -	VOP_UNLOCK(fdirp, 0);
 +	if (fromnd.ni_dvp != fromnd.ni_vp) {
 +		VOP_UNLOCK(fromnd.ni_dvp, 0);
 +	}
  	fvp = fromnd.ni_vp;
  	nfsm_srvmtofh(&tnsfh);
  	if (v3) {
 Index: nfs/nfs_subs.c
 ===================================================================
 RCS file: /cvsroot/src/sys/nfs/nfs_subs.c,v
 retrieving revision 1.179
 diff -u -p -r1.179 nfs_subs.c
 --- nfs/nfs_subs.c	27 Dec 2006 12:10:09 -0000	1.179
 +++ nfs/nfs_subs.c	3 Feb 2007 17:41:01 -0000
 @@ -2133,7 +2133,6 @@ nfs_namei(ndp, nsfh, len, slp, nam, mdp,
  			 */
  			default:
  				error = EIO;
 -				vrele(dp);
  				PNBUF_PUT(cp);
  				goto out;
  			}
 @@ -2150,7 +2149,6 @@ nfs_namei(ndp, nsfh, len, slp, nam, mdp,
  					continue;
  				} else {
  					error = ENOENT;
 -					vrele(dp);
  					PNBUF_PUT(cp);
  					goto out;
  				}
 
 
 
 
 -Chuck