Subject: kern/10312: too much vrele in the msdosfs_rename(), system panics.
To: None <gnats-bugs@gnats.netbsd.org>
From: Martin J. Laubach <mjl@emsi.priv.at>
List: netbsd-bugs
Date: 06/07/2000 14:56:10
>Number:         10312
>Category:       kern
>Synopsis:       too much vrele in the msdosfs_rename(), system panics.
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Jun 07 14:57:00 PDT 2000
>Closed-Date:
>Last-Modified:
>Originator:     Martin J. Laubach
>Release:        1.4ZA
>Organization:
>Environment:
System: NetBSD cactus.emsi.priv.at 1.4Z NetBSD 1.4Z (CACTUS) #0: Wed May 31 22:18:54 CEST 2000 mjl@asparagus:/home/temp/devel/cvs/src/sys/arch/i386/compile/CACTUS i386

>Description:
  This PR comes from OpenBSD PR/1249. I have verified that the problem
described also affects NetBSD. Could someone familiar with the msdosfs
check the include patch please?

  Originally submitted by gluk@ptci.ru (Grigoriy Orlov)

>How-To-Repeat:

compile kernel with option DIAGNOSTIC, reboot.

mount -t msdos /dev/wd0i /mnt
mkdir /mnt/a
mkdir /mnt/a/b
mv /mnt/a /mnt/a/b
vrele: bad ref count: type VDIR ....
ddb>

>Fix:

  [Note: this patch is for openbsd, need probably some adjusting
   for NetBSD]

Index: msdosfs_vnops.c
===================================================================
RCS file: /cvs/src/sys/msdosfs/msdosfs_vnops.c,v
retrieving revision 1.21
diff -u -r1.21 msdosfs_vnops.c
--- msdosfs_vnops.c	1999/02/26 03:28:13	1.21
+++ msdosfs_vnops.c	2000/05/26 22:21:22
@@ -1060,7 +1060,7 @@
 	vrele(fdvp);
 	if (doingdirectory && newparent) {
 		if (error)	/* write access check above */
-			goto bad;
+			goto bad1;
 		if (xp != NULL)
 			vput(tvp);
 		/*
@@ -1086,19 +1086,19 @@
 		if (xp->de_Attributes & ATTR_DIRECTORY) {
 			if (!dosdirempty(xp)) {
 				error = ENOTEMPTY;
-				goto bad;
+				goto bad1;
 			}
 			if (!doingdirectory) {
 				error = ENOTDIR;
-				goto bad;
+				goto bad1;
 			}
 			cache_purge(tdvp);
 		} else if (doingdirectory) {
 			error = EISDIR;
-			goto bad;
+			goto bad1;
 		}
 		if ((error = removede(dp, xp)) != 0)
-			goto bad;
+			goto bad1;
 		vput(tvp);
 		xp = NULL;
 	}
@@ -1109,7 +1109,7 @@
 	 * file/directory.
 	 */
 	if ((error = uniqdosname(VTODE(tdvp), tcnp, toname)) != 0)
-		goto abortit;
+		goto bad1;
 
 	/*
 	 * Since from wasn't locked at various places above,
@@ -1150,7 +1150,6 @@
 		if (doingdirectory)
 			panic("rename: lost dir entry");
 		vrele(ap->a_fvp);
-		VOP_UNLOCK(fvp, 0, p);
 		if (newparent)
 			VOP_UNLOCK(fdvp, 0, p);
 		xp = NULL;
@@ -1175,7 +1174,6 @@
 			bcopy(oldname, ip->de_Name, 11);
 			if (newparent)
 				VOP_UNLOCK(fdvp, 0, p);
-			VOP_UNLOCK(fvp, 0, p);
 			goto bad;
 		}
 		ip->de_refcnt++;
@@ -1184,7 +1182,6 @@
 			/* XXX should really panic here, fs is corrupt */
 			if (newparent)
 				VOP_UNLOCK(fdvp, 0, p);
-			VOP_UNLOCK(fvp, 0, p);
 			goto bad;
 		}
 		if (!doingdirectory) {
@@ -1194,7 +1191,6 @@
 				/* XXX should really panic here, fs is corrupt */
 				if (newparent)
 					VOP_UNLOCK(fdvp, 0, p);
-				VOP_UNLOCK(fvp, 0, p);
 				goto bad;
 			}
 			if (ip->de_dirclust != MSDOSFSROOT)
@@ -1221,26 +1217,25 @@
 		if (error) {
 			/* XXX should really panic here, fs is corrupt */
 			brelse(bp);
-			VOP_UNLOCK(fvp, 0, p);
 			goto bad;
 		}
 		dotdotp = (struct direntry *)bp->b_data + 1;
 		putushort(dotdotp->deStartCluster, dp->de_StartCluster);
 		if ((error = bwrite(bp)) != 0) {
 			/* XXX should really panic here, fs is corrupt */
-			VOP_UNLOCK(fvp, 0, p);
 			goto bad;
 		}
 	}
 
-	VOP_UNLOCK(fvp, 0, p);
 bad:
+	VOP_UNLOCK(fvp, 0, p);
+	vrele(fdvp);
+bad1:
 	if (xp)
 		vput(tvp);
 	vput(tdvp);
 out:
 	ip->de_flag &= ~DE_RENAME;
-	vrele(fdvp);
 	vrele(fvp);
 	return (error);
 
>Release-Note:
>Audit-Trail:
>Unformatted: