svn commit: r357199 - head/sys/fs/nullfs

Konstantin Belousov kib at FreeBSD.org
Tue Jan 28 11:29:07 UTC 2020


Author: kib
Date: Tue Jan 28 11:29:06 2020
New Revision: 357199
URL: https://svnweb.freebsd.org/changeset/base/357199

Log:
  Save lower root vnode in nullfs mnt data instead of upper.
  
  Nullfs needs to know the root vnode of the lower fs during the
  operation.  Currently it caches the upper vnode of it, which is also
  the root of the nullfs mount.  On unmount, nullfs calls vflush() with
  rootrefs == 1, and aborts non-forced unmount if there are any more
  vnodes instantiated during vflush().  This means that the reference to
  the root vnode after failed non-forced unmount could be lost and
  nullm_rootvp points to the freed memory.
  
  Fix it by storing the reference for lower vnode instead, which is kept
  intact during vflush().  nullfs_root() now instantiates the upper
  vnode of lower root.  Care about VV_ROOT flag in null_nodeget().
  
  Reported and tested by:	pho
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

Modified:
  head/sys/fs/nullfs/null.h
  head/sys/fs/nullfs/null_subr.c
  head/sys/fs/nullfs/null_vfsops.c

Modified: head/sys/fs/nullfs/null.h
==============================================================================
--- head/sys/fs/nullfs/null.h	Tue Jan 28 11:22:20 2020	(r357198)
+++ head/sys/fs/nullfs/null.h	Tue Jan 28 11:29:06 2020	(r357199)
@@ -43,7 +43,7 @@
 
 struct null_mount {
 	struct mount	*nullm_vfs;
-	struct vnode	*nullm_rootvp;	/* Reference to root null_node */
+	struct vnode	*nullm_lowerrootvp;	/* Ref to lower root vnode */
 	uint64_t	nullm_flags;
 };
 

Modified: head/sys/fs/nullfs/null_subr.c
==============================================================================
--- head/sys/fs/nullfs/null_subr.c	Tue Jan 28 11:22:20 2020	(r357198)
+++ head/sys/fs/nullfs/null_subr.c	Tue Jan 28 11:29:06 2020	(r357199)
@@ -252,6 +252,8 @@ null_nodeget(mp, lowervp, vpp)
 	vp->v_type = lowervp->v_type;
 	vp->v_data = xp;
 	vp->v_vnlock = lowervp->v_vnlock;
+	if (lowervp == MOUNTTONULLMOUNT(mp)->nullm_lowerrootvp)
+		vp->v_vflag |= VV_ROOT;
 	error = insmntque1(vp, mp, null_insmntque_dtr, xp);
 	if (error != 0)
 		return (error);

Modified: head/sys/fs/nullfs/null_vfsops.c
==============================================================================
--- head/sys/fs/nullfs/null_vfsops.c	Tue Jan 28 11:22:20 2020	(r357198)
+++ head/sys/fs/nullfs/null_vfsops.c	Tue Jan 28 11:29:06 2020	(r357199)
@@ -74,7 +74,7 @@ static vfs_extattrctl_t	nullfs_extattrctl;
 static int
 nullfs_mount(struct mount *mp)
 {
-	struct vnode *lowerrootvp, *vp;
+	struct vnode *lowerrootvp;
 	struct vnode *nullm_rootvp;
 	struct null_mount *xmp;
 	struct null_node *nn;
@@ -158,36 +158,24 @@ nullfs_mount(struct mount *mp)
 	    M_NULLFSMNT, M_WAITOK | M_ZERO);
 
 	/*
-	 * Save reference to underlying FS
+	 * Save pointer to underlying FS and the reference to the
+	 * lower root vnode.
 	 */
 	xmp->nullm_vfs = lowerrootvp->v_mount;
+	vref(lowerrootvp);
+	xmp->nullm_lowerrootvp = lowerrootvp;
+	mp->mnt_data = xmp;
 
 	/*
-	 * Save reference.  Each mount also holds
-	 * a reference on the root vnode.
+	 * Make sure the node alias worked.
 	 */
-	error = null_nodeget(mp, lowerrootvp, &vp);
-	/*
-	 * Make sure the node alias worked
-	 */
-	if (error) {
+	error = null_nodeget(mp, lowerrootvp, &nullm_rootvp);
+	if (error != 0) {
+		vrele(lowerrootvp);
 		free(xmp, M_NULLFSMNT);
 		return (error);
 	}
 
-	/*
-	 * Keep a held reference to the root vnode.
-	 * It is vrele'd in nullfs_unmount.
-	 */
-	nullm_rootvp = vp;
-	nullm_rootvp->v_vflag |= VV_ROOT;
-	xmp->nullm_rootvp = nullm_rootvp;
-
-	/*
-	 * Unlock the node (either the lower or the alias)
-	 */
-	VOP_UNLOCK(vp);
-
 	if (NULLVPTOLOWERVP(nullm_rootvp)->v_mount->mnt_flag & MNT_LOCAL) {
 		MNT_ILOCK(mp);
 		mp->mnt_flag |= MNT_LOCAL;
@@ -209,7 +197,6 @@ nullfs_mount(struct mount *mp)
 	mp->mnt_kern_flag |= lowerrootvp->v_mount->mnt_kern_flag &
 	    (MNTK_USES_BCACHE | MNTK_NO_IOPF | MNTK_UNMAPPED_BUFS);
 	MNT_IUNLOCK(mp);
-	mp->mnt_data = xmp;
 	vfs_getnewfsid(mp);
 	if ((xmp->nullm_flags & NULLM_CACHE) != 0) {
 		MNT_ILOCK(xmp->nullm_vfs);
@@ -219,6 +206,7 @@ nullfs_mount(struct mount *mp)
 	}
 
 	vfs_mountedfrom(mp, target);
+	vput(nullm_rootvp);
 
 	NULLFSDEBUG("nullfs_mount: lower %s, alias at %s\n",
 		mp->mnt_stat.f_mntfromname, mp->mnt_stat.f_mntonname);
@@ -235,7 +223,7 @@ nullfs_unmount(mp, mntflags)
 {
 	struct null_mount *mntdata;
 	struct mount *ump;
-	int error, flags, rootrefs;
+	int error, flags;
 
 	NULLFSDEBUG("nullfs_unmount: mp = %p\n", (void *)mp);
 
@@ -244,9 +232,9 @@ nullfs_unmount(mp, mntflags)
 	else
 		flags = 0;
 
-	for (rootrefs = 1;; rootrefs = 0) {
+	for (;;) {
 		/* There is 1 extra root vnode reference (nullm_rootvp). */
-		error = vflush(mp, rootrefs, flags, curthread);
+		error = vflush(mp, 0, flags, curthread);
 		if (error)
 			return (error);
 		MNT_ILOCK(mp);
@@ -273,6 +261,7 @@ nullfs_unmount(mp, mntflags)
 		TAILQ_REMOVE(&ump->mnt_uppers, mp, mnt_upper_link);
 		MNT_IUNLOCK(ump);
 	}
+	vrele(mntdata->nullm_lowerrootvp);
 	mp->mnt_data = NULL;
 	free(mntdata, M_NULLFSMNT);
 	return (0);
@@ -285,21 +274,24 @@ nullfs_root(mp, flags, vpp)
 	struct vnode **vpp;
 {
 	struct vnode *vp;
+	struct null_mount *mntdata;
+	int error;
 
-	NULLFSDEBUG("nullfs_root(mp = %p, vp = %p->%p)\n", (void *)mp,
-	    (void *)MOUNTTONULLMOUNT(mp)->nullm_rootvp,
-	    (void *)NULLVPTOLOWERVP(MOUNTTONULLMOUNT(mp)->nullm_rootvp));
+	mntdata = MOUNTTONULLMOUNT(mp);
+	NULLFSDEBUG("nullfs_root(mp = %p, vp = %p)\n", mp,
+	    mntdata->nullm_lowerrootvp);
 
-	/*
-	 * Return locked reference to root.
-	 */
-	vp = MOUNTTONULLMOUNT(mp)->nullm_rootvp;
-	VREF(vp);
-
-	ASSERT_VOP_UNLOCKED(vp, "root vnode is locked");
-	vn_lock(vp, flags | LK_RETRY);
-	*vpp = vp;
-	return 0;
+	error = vget(mntdata->nullm_lowerrootvp, (flags & ~LK_TYPE_MASK) |
+	    LK_EXCLUSIVE, curthread);
+	if (error == 0) {
+		error = null_nodeget(mp, mntdata->nullm_lowerrootvp, &vp);
+		if (error == 0) {
+			if ((flags & LK_TYPE_MASK) == LK_SHARED)
+				vn_lock(vp, LK_DOWNGRADE | LK_RETRY);
+			*vpp = vp;
+		}
+	}
+	return (error);
 }
 
 static int


More information about the svn-src-head mailing list