Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 29 Nov 2008 13:34:59 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r185432 - in head/sys: kern nfsserver sys
Message-ID:  <200811291334.mATDYxV4052363@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Sat Nov 29 13:34:59 2008
New Revision: 185432
URL: http://svn.freebsd.org/changeset/base/185432

Log:
  In the nfsrv_fhtovp(), after the vfs_getvfs() function found the pointer
  to the fs, but before a vnode on the fs is locked, unmount may free fs
  structures, causing access to destroyed data and freed memory.
  
  Introduce a vfs_busymp() function that looks up and busies found
  fs while mountlist_mtx is held. Use it in nfsrv_fhtovp() and in the
  implementation of the handle syscalls.
  
  Two other uses of the vfs_getvfs() in the vfs_subr.c, namely in
  sysctl_vfs_ctl and vfs_getnewfsid seems to be ok. In particular,
  sysctl_vfs_ctl is protected by Giant by being a non-sleeping sysctl
  handler, that prevents Giant-locked unmount code to interfere with it.
  
  Noted by:	tegge
  Reviewed by:	dfr
  Tested by:	pho
  MFC after:	1 month

Modified:
  head/sys/kern/vfs_subr.c
  head/sys/kern/vfs_syscalls.c
  head/sys/nfsserver/nfs_srvsubs.c
  head/sys/sys/mount.h

Modified: head/sys/kern/vfs_subr.c
==============================================================================
--- head/sys/kern/vfs_subr.c	Sat Nov 29 12:40:14 2008	(r185431)
+++ head/sys/kern/vfs_subr.c	Sat Nov 29 13:34:59 2008	(r185432)
@@ -407,6 +407,32 @@ vfs_getvfs(fsid_t *fsid)
 }
 
 /*
+ * Lookup a mount point by filesystem identifier, busying it before
+ * returning.
+ */
+struct mount *
+vfs_busyfs(fsid_t *fsid)
+{
+	struct mount *mp;
+	int error;
+
+	mtx_lock(&mountlist_mtx);
+	TAILQ_FOREACH(mp, &mountlist, mnt_list) {
+		if (mp->mnt_stat.f_fsid.val[0] == fsid->val[0] &&
+		    mp->mnt_stat.f_fsid.val[1] == fsid->val[1]) {
+			error = vfs_busy(mp, MBF_MNTLSTLOCK);
+			if (error) {
+				mtx_unlock(&mountlist_mtx);
+				return (NULL);
+			}
+			return (mp);
+		}
+	}
+	mtx_unlock(&mountlist_mtx);
+	return ((struct mount *) 0);
+}
+
+/*
  * Check if a user can access privileged mount options.
  */
 int

Modified: head/sys/kern/vfs_syscalls.c
==============================================================================
--- head/sys/kern/vfs_syscalls.c	Sat Nov 29 12:40:14 2008	(r185431)
+++ head/sys/kern/vfs_syscalls.c	Sat Nov 29 13:34:59 2008	(r185432)
@@ -4382,12 +4382,13 @@ fhopen(td, uap)
 	if (error)
 		return(error);
 	/* find the mount point */
-	mp = vfs_getvfs(&fhp.fh_fsid);
+	mp = vfs_busyfs(&fhp.fh_fsid);
 	if (mp == NULL)
 		return (ESTALE);
 	vfslocked = VFS_LOCK_GIANT(mp);
 	/* now give me my vnode, it gets returned to me locked */
 	error = VFS_FHTOVP(mp, &fhp.fh_fid, &vp);
+	vfs_unbusy(mp);
 	if (error)
 		goto out;
 	/*
@@ -4520,7 +4521,6 @@ fhopen(td, uap)
 bad:
 	vput(vp);
 out:
-	vfs_rel(mp);
 	VFS_UNLOCK_GIANT(vfslocked);
 	return (error);
 }
@@ -4555,17 +4555,17 @@ fhstat(td, uap)
 	error = copyin(uap->u_fhp, &fh, sizeof(fhandle_t));
 	if (error)
 		return (error);
-	if ((mp = vfs_getvfs(&fh.fh_fsid)) == NULL)
+	if ((mp = vfs_busyfs(&fh.fh_fsid)) == NULL)
 		return (ESTALE);
 	vfslocked = VFS_LOCK_GIANT(mp);
-	if ((error = VFS_FHTOVP(mp, &fh.fh_fid, &vp))) {
-		vfs_rel(mp);
+	error = VFS_FHTOVP(mp, &fh.fh_fid, &vp);
+	vfs_unbusy(mp);
+	if (error) {
 		VFS_UNLOCK_GIANT(vfslocked);
 		return (error);
 	}
 	error = vn_stat(vp, &sb, td->td_ucred, NOCRED, td);
 	vput(vp);
-	vfs_rel(mp);
 	VFS_UNLOCK_GIANT(vfslocked);
 	if (error)
 		return (error);
@@ -4615,13 +4615,13 @@ kern_fhstatfs(struct thread *td, fhandle
 	error = priv_check(td, PRIV_VFS_FHSTATFS);
 	if (error)
 		return (error);
-	if ((mp = vfs_getvfs(&fh.fh_fsid)) == NULL)
+	if ((mp = vfs_busyfs(&fh.fh_fsid)) == NULL)
 		return (ESTALE);
 	vfslocked = VFS_LOCK_GIANT(mp);
 	error = VFS_FHTOVP(mp, &fh.fh_fid, &vp);
 	if (error) {
+		vfs_unbusy(mp);
 		VFS_UNLOCK_GIANT(vfslocked);
-		vfs_rel(mp);
 		return (error);
 	}
 	vput(vp);
@@ -4644,7 +4644,7 @@ kern_fhstatfs(struct thread *td, fhandle
 	if (error == 0)
 		*buf = *sp;
 out:
-	vfs_rel(mp);
+	vfs_unbusy(mp);
 	VFS_UNLOCK_GIANT(vfslocked);
 	return (error);
 }

Modified: head/sys/nfsserver/nfs_srvsubs.c
==============================================================================
--- head/sys/nfsserver/nfs_srvsubs.c	Sat Nov 29 12:40:14 2008	(r185431)
+++ head/sys/nfsserver/nfs_srvsubs.c	Sat Nov 29 13:34:59 2008	(r185432)
@@ -1119,14 +1119,16 @@ nfsrv_fhtovp(fhandle_t *fhp, int lockfla
 		fhp = &nfs_pub.np_handle;
 	}
 
-	mp = vfs_getvfs(&fhp->fh_fsid);
+	mp = vfs_busyfs(&fhp->fh_fsid);
 	if (!mp)
 		return (ESTALE);
 	vfslocked = VFS_LOCK_GIANT(mp);
 	error = VFS_CHECKEXP(mp, nam, &exflags, &credanon,
 	    &numsecflavors, &secflavors);
-	if (error)
+	if (error) {
+		vfs_unbusy(mp);
 		goto out;
+	}
 	if (numsecflavors == 0) {
 		/*
 		 * This can happen if the system is running with an
@@ -1159,10 +1161,12 @@ nfsrv_fhtovp(fhandle_t *fhp, int lockfla
 		}
 		if (!mountreq) {
 			error = NFSERR_AUTHERR | AUTH_TOOWEAK;
+			vfs_unbusy(mp);
 			goto out;
 		}
 	}
 	error = VFS_FHTOVP(mp, &fhp->fh_fid, vpp);
+	vfs_unbusy(mp);
 	if (error)
 		goto out;
 #ifdef MNT_EXNORESPORT
@@ -1196,7 +1200,6 @@ nfsrv_fhtovp(fhandle_t *fhp, int lockfla
 	if (!lockflag)
 		VOP_UNLOCK(*vpp, 0);
 out:
-	vfs_rel(mp);
 	if (error) {
 		VFS_UNLOCK_GIANT(vfslocked);
 	} else

Modified: head/sys/sys/mount.h
==============================================================================
--- head/sys/sys/mount.h	Sat Nov 29 12:40:14 2008	(r185431)
+++ head/sys/sys/mount.h	Sat Nov 29 13:34:59 2008	(r185432)
@@ -717,6 +717,7 @@ int	vfs_donmount(struct thread *td, int 
 void	vfs_getnewfsid(struct mount *);
 struct cdev *vfs_getrootfsid(struct mount *);
 struct	mount *vfs_getvfs(fsid_t *);      /* return vfs given fsid */
+struct	mount *vfs_busyfs(fsid_t *);
 int	vfs_modevent(module_t, int, void *);
 void	vfs_mount_error(struct mount *, const char *, ...);
 void	vfs_mountroot(void);			/* mount our root filesystem */



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