svn commit: r230124 - in stable/9/sys: kern ufs/ufs

[Konstantin Belousov]

Author: kib
Date: Sun Jan 15 00:46:29 2012
New Revision: 230124
URL: http://svn.freebsd.org/changeset/base/230124

Log:
  MFC r229828:
  Avoid LOR between vfs_busy() lock and covered vnode lock on quotaon().

Modified:
  stable/9/sys/kern/vfs_syscalls.c
  stable/9/sys/ufs/ufs/ufs_quota.c
Directory Properties:
  stable/9/sys/   (props changed)

Modified: stable/9/sys/kern/vfs_syscalls.c
==============================================================================
--- stable/9/sys/kern/vfs_syscalls.c	Sun Jan 15 00:08:14 2012	(r230123)
+++ stable/9/sys/kern/vfs_syscalls.c	Sun Jan 15 00:46:29 2012	(r230124)
@@ -86,6 +86,8 @@ __FBSDID("$FreeBSD$");
 #include <vm/vm_page.h>
 #include <vm/uma.h>
 
+#include <ufs/ufs/quota.h>
+
 static MALLOC_DEFINE(M_FADVISE, "fadvise", "posix_fadvise(2) information");
 
 SDT_PROVIDER_DEFINE(vfs);
@@ -212,7 +214,20 @@ sys_quotactl(td, uap)
 		return (error);
 	}
 	error = VFS_QUOTACTL(mp, uap->cmd, uap->uid, uap->arg);
-	vfs_unbusy(mp);
+
+	/*
+	 * Since quota on operation typically needs to open quota
+	 * file, the Q_QUOTAON handler needs to unbusy the mount point
+	 * before calling into namei.  Otherwise, unmount might be
+	 * started between two vfs_busy() invocations (first is our,
+	 * second is from mount point cross-walk code in lookup()),
+	 * causing deadlock.
+	 *
+	 * Require that Q_QUOTAON handles the vfs_busy() reference on
+	 * its own, always returning with ubusied mount point.
+	 */
+	if ((uap->cmd >> SUBCMDSHIFT) != Q_QUOTAON)
+		vfs_unbusy(mp);
 	VFS_UNLOCK_GIANT(vfslocked);
 	return (error);
 }

Modified: stable/9/sys/ufs/ufs/ufs_quota.c
==============================================================================
--- stable/9/sys/ufs/ufs/ufs_quota.c	Sun Jan 15 00:08:14 2012	(r230123)
+++ stable/9/sys/ufs/ufs/ufs_quota.c	Sun Jan 15 00:46:29 2012	(r230124)
@@ -512,17 +512,29 @@ quotaon(struct thread *td, struct mount 
 
 	NDINIT(&nd, LOOKUP, FOLLOW | MPSAFE, UIO_USERSPACE, fname, td);
 	flags = FREAD | FWRITE;
+	vfs_ref(mp);
+	vfs_unbusy(mp);
 	error = vn_open(&nd, &flags, 0, NULL);
-	if (error)
+	if (error != 0) {
+		vfs_rel(mp);
 		return (error);
+	}
 	vfslocked = NDHASGIANT(&nd);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	vp = nd.ni_vp;
-	if (vp->v_type != VREG) {
+	error = vfs_busy(mp, MBF_NOWAIT);
+	vfs_rel(mp);
+	if (error == 0) {
+		if (vp->v_type != VREG) {
+			error = EACCES;
+			vfs_unbusy(mp);
+		}
+	}
+	if (error != 0) {
 		VOP_UNLOCK(vp, 0);
 		(void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td);
 		VFS_UNLOCK_GIANT(vfslocked);
-		return (EACCES);
+		return (error);
 	}
 
 	UFS_LOCK(ump);
@@ -531,6 +543,7 @@ quotaon(struct thread *td, struct mount 
 		VOP_UNLOCK(vp, 0);
 		(void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td);
 		VFS_UNLOCK_GIANT(vfslocked);
+		vfs_unbusy(mp);
 		return (EALREADY);
 	}
 	ump->um_qflags[type] |= QTF_OPENING|QTF_CLOSING;
@@ -542,6 +555,7 @@ quotaon(struct thread *td, struct mount 
 		UFS_UNLOCK(ump);
 		(void) vn_close(vp, FREAD|FWRITE, td->td_ucred, td);
 		VFS_UNLOCK_GIANT(vfslocked);
+		vfs_unbusy(mp);
 		return (error);
 	}
 	VOP_UNLOCK(vp, 0);
@@ -619,6 +633,7 @@ again:
 		("quotaon: leaking flags"));
 	UFS_UNLOCK(ump);
 
+	vfs_unbusy(mp);
 	return (error);
 }