Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 5 Aug 2011 11:12:50 +0000 (UTC)
From:      Martin Matuska <mm@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r224655 - head/sys/kern
Message-ID:  <201108051112.p75BCoDr003862@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mm
Date: Fri Aug  5 11:12:50 2011
New Revision: 224655
URL: http://svn.freebsd.org/changeset/base/224655

Log:
  The change in r224615 didn't take into account that vn_fullpath_global()
  doesn't operate on locked vnode. This could cause a panic.
  
  Fix by unlocking vnode, re-locking afterwards and verifying that it wasn't
  renamed or deleted. To improve readability and reduce code size, move code
  to a new static function vfs_verify_global_path().
  
  In addition, fix missing giant unlock in unmount().
  
  Reported by:	David Wolfskill <david@catwhisker.org>
  Reviewed by:	kib
  Approved by:	re (bz)
  MFC after:	2 weeks

Modified:
  head/sys/kern/vfs_mount.c

Modified: head/sys/kern/vfs_mount.c
==============================================================================
--- head/sys/kern/vfs_mount.c	Fri Aug  5 06:57:44 2011	(r224654)
+++ head/sys/kern/vfs_mount.c	Fri Aug  5 11:12:50 2011	(r224655)
@@ -362,6 +362,60 @@ vfs_mergeopts(struct vfsoptlist *toopts,
 }
 
 /*
+ * Verify vnode's global path
+ */
+static int
+vfs_verify_global_path(struct thread *td, struct vnode *vp, char *fspath)
+{
+	struct nameidata nd;
+	struct vnode *vp1;
+	char *rpath, *fbuf;
+	int error;
+
+	ASSERT_VOP_ELOCKED(vp, __func__);
+
+	/* Construct global filesystem path from vp. */
+	VOP_UNLOCK(vp, 0);
+	error = vn_fullpath_global(td, vp, &rpath, &fbuf);
+	if (error != 0) {
+		vrele(vp);
+		return (error);
+	}
+	if (strlen(rpath) >= MNAMELEN) {
+		vrele(vp);
+		error = ENAMETOOLONG;
+		goto out;
+	}
+
+	/*
+	 * Re-lookup the vnode by path. As a side effect, the vnode is
+	 * relocked.  If vnode was renamed, return ENOENT.
+	 */
+	NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF | MPSAFE | AUDITVNODE1,
+	    UIO_SYSSPACE, fspath, td);
+	error = namei(&nd);
+	if (error != 0) {
+		vrele(vp);
+		goto out;
+	}
+	if (NDHASGIANT(&nd))
+		mtx_unlock(&Giant);
+	NDFREE(&nd, NDF_ONLY_PNBUF);
+	vp1 = nd.ni_vp;
+	vrele(vp);
+	if (vp1 != vp) {
+		vput(vp1);
+		error = ENOENT;
+		goto out;
+	}
+
+	strlcpy(fspath,rpath,MNAMELEN);
+out:
+	free(fbuf, M_TEMP);
+	return (error);
+}
+
+/*
  * Mount a filesystem.
  */
 int
@@ -745,6 +799,7 @@ static int
 vfs_domount_first(
 	struct thread *td,		/* Calling thread. */
 	struct vfsconf *vfsp,		/* File system type. */
+	char *fspath,			/* Mount path. */
 	struct vnode *vp,		/* Vnode to be covered. */
 	int fsflags,			/* Flags common to all filesystems. */
 	struct vfsoptlist **optlist	/* Options local to the filesystem. */
@@ -753,25 +808,12 @@ vfs_domount_first(
 	struct vattr va;
 	struct mount *mp;
 	struct vnode *newdp;
-	char *fspath, *fbuf;
 	int error;
 
 	mtx_assert(&Giant, MA_OWNED);
 	ASSERT_VOP_ELOCKED(vp, __func__);
 	KASSERT((fsflags & MNT_UPDATE) == 0, ("MNT_UPDATE shouldn't be here"));
 
-	/* Construct global filesystem path from vp. */
-	error = vn_fullpath_global(td, vp, &fspath, &fbuf);
-	if (error != 0) {
-		vput(vp);
-		return (error);
-	}
-	if (strlen(fspath) >= MNAMELEN) {
-		vput(vp);
-		free(fbuf, M_TEMP);
-		return (ENAMETOOLONG);
-	}
-
 	/*
 	 * If the user is not root, ensure that they own the directory
 	 * onto which we are attempting to mount.
@@ -793,14 +835,12 @@ vfs_domount_first(
 	}
 	if (error != 0) {
 		vput(vp);
-		free(fbuf, M_TEMP);
 		return (error);
 	}
 	VOP_UNLOCK(vp, 0);
 
 	/* Allocate and initialize the filesystem. */
 	mp = vfs_mount_alloc(vp, vfsp, fspath, td->td_ucred);
-	free(fbuf, M_TEMP);
 	/* XXXMAC: pass to vfs_mount_alloc? */
 	mp->mnt_optnew = *optlist;
 	/* Set the mount level flags. */
@@ -1083,15 +1123,15 @@ vfs_domount(
 		mtx_lock(&Giant);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	vp = nd.ni_vp;
-	if ((fsflags & MNT_UPDATE) == 0)
-		error = vfs_domount_first(td, vfsp, vp, fsflags, optlist);
-	else
+	if ((fsflags & MNT_UPDATE) == 0) {
+		error = vfs_verify_global_path(td, vp, fspath);
+		if (error == 0)
+			error = vfs_domount_first(td, vfsp, fspath, vp,
+			    fsflags, optlist);
+	} else
 		error = vfs_domount_update(td, vp, fsflags, optlist);
 	mtx_unlock(&Giant);
 
-	ASSERT_VI_UNLOCKED(vp, __func__);
-	ASSERT_VOP_UNLOCKED(vp, __func__);
-
 	return (error);
 }
 
@@ -1118,7 +1158,7 @@ unmount(td, uap)
 {
 	struct mount *mp;
 	struct nameidata nd;
-	char *pathbuf, *rpathbuf, *fbuf;
+	char *pathbuf;
 	int error, id0, id1;
 
 	AUDIT_ARG_VALUE(uap->flags);
@@ -1163,16 +1203,13 @@ unmount(td, uap)
 			    FOLLOW | LOCKLEAF | MPSAFE | AUDITVNODE1,
 			    UIO_SYSSPACE, pathbuf, td);
 			if (namei(&nd) == 0) {
+				if (NDHASGIANT(&nd))
+					mtx_unlock(&Giant);
 				NDFREE(&nd, NDF_ONLY_PNBUF);
-				if (vn_fullpath_global(td, nd.ni_vp, &rpathbuf,
-				    &fbuf) == 0) {
-					if (strlen(rpathbuf) < MNAMELEN) {
-						strlcpy(pathbuf, rpathbuf,
-						    MNAMELEN);
-					}
-					free(fbuf, M_TEMP);
-				}
-				vput(nd.ni_vp);
+				error = vfs_verify_global_path(td, nd.ni_vp,
+				    pathbuf);
+				if (error == 0)
+					vput(nd.ni_vp);
 			}
 		}
 		mtx_lock(&mountlist_mtx);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201108051112.p75BCoDr003862>