git: a01ca46b9b86 - main - unionfs: use VV_ROOT to check for root vnode in unionfs_lock()

The branch main has been updated by jah:

URL: https://cgit.FreeBSD.org/src/commit/?id=a01ca46b9b86608bfa080a4d86d586beb9e756c9

commit a01ca46b9b86608bfa080a4d86d586beb9e756c9
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2022-01-17 01:03:54 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2022-01-30 04:38:44 +0000

    unionfs: use VV_ROOT to check for root vnode in unionfs_lock()
    
    This avoids a potentially wild reference to the mount object.
    Additionally, simplify some of the checks around VV_ROOT in
    unionfs_nodeget().
    
    Reviewed by:    kib
    Differential Revision: https://reviews.freebsd.org/D33914
---
 sys/fs/unionfs/union_subr.c   | 19 +++++++++++--------
 sys/fs/unionfs/union_vfsops.c |  5 +++++
 sys/fs/unionfs/union_vnops.c  | 11 ++---------
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c
index e051d42c07cc..2b5754a560c7 100644
--- a/sys/fs/unionfs/union_subr.c
+++ b/sys/fs/unionfs/union_subr.c
@@ -334,12 +334,6 @@ unionfs_nodeget(struct mount *mp, struct vnode *uppervp,
 		}
 	}
 
-	if ((uppervp == NULLVP || ump->um_uppervp != uppervp) ||
-	    (lowervp == NULLVP || ump->um_lowervp != lowervp)) {
-		/* dvp will be NULLVP only in case of root vnode. */
-		if (dvp == NULLVP)
-			return (EINVAL);
-	}
 	unp = malloc(sizeof(struct unionfs_node),
 	    M_UNIONFSNODE, M_WAITOK | M_ZERO);
 
@@ -381,9 +375,18 @@ unionfs_nodeget(struct mount *mp, struct vnode *uppervp,
 	vp->v_type = vt;
 	vp->v_data = unp;
 
-	if ((uppervp != NULLVP && ump->um_uppervp == uppervp) &&
-	    (lowervp != NULLVP && ump->um_lowervp == lowervp))
+	/*
+	 * TODO: This is an imperfect check, as there's no guarantee that
+	 * the underlying filesystems will always return vnode pointers
+	 * for the root inodes that match our cached values.  To reduce
+	 * the likelihood of failure, for example in the case where either
+	 * vnode has been forcibly doomed, we check both pointers and set
+	 * VV_ROOT if either matches.
+	 */
+	if (ump->um_uppervp == uppervp || ump->um_lowervp == lowervp)
 		vp->v_vflag |= VV_ROOT;
+	KASSERT(dvp != NULL || (vp->v_vflag & VV_ROOT) != 0,
+	    ("%s: NULL dvp for non-root vp %p", __func__, vp));
 
 	vn_lock_pair(lowervp, false, uppervp, false); 
 	error = insmntque1(vp, mp, NULL, NULL);
diff --git a/sys/fs/unionfs/union_vfsops.c b/sys/fs/unionfs/union_vfsops.c
index ea49d1e189da..dd6e7084ebc1 100644
--- a/sys/fs/unionfs/union_vfsops.c
+++ b/sys/fs/unionfs/union_vfsops.c
@@ -241,6 +241,8 @@ unionfs_domount(struct mount *mp)
 	/* get root vnodes */
 	lowerrootvp = mp->mnt_vnodecovered;
 	upperrootvp = ndp->ni_vp;
+	KASSERT(lowerrootvp != NULL, ("%s: NULL lower root vp", __func__));
+	KASSERT(upperrootvp != NULL, ("%s: NULL upper root vp", __func__));
 
 	/* create unionfs_mount */
 	ump = malloc(sizeof(struct unionfs_mount), M_UNIONFSMNT,
@@ -289,6 +291,9 @@ unionfs_domount(struct mount *mp)
 		mp->mnt_data = NULL;
 		return (error);
 	}
+	KASSERT(ump->um_rootvp != NULL, ("rootvp cannot be NULL"));
+	KASSERT((ump->um_rootvp->v_vflag & VV_ROOT) != 0,
+	    ("%s: rootvp without VV_ROOT", __func__));
 
 	lowermp = vfs_register_upper_from_vp(ump->um_lowervp, mp,
 	    &ump->um_lower_link);
diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index 19779112941e..c9d8aac12362 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -1908,8 +1908,6 @@ unionfs_revlock(struct vnode *vp, int flags)
 static int
 unionfs_lock(struct vop_lock1_args *ap)
 {
-	struct mount   *mp;
-	struct unionfs_mount *ump;
 	struct unionfs_node *unp;
 	struct vnode   *vp;
 	struct vnode   *uvp;
@@ -1944,13 +1942,8 @@ unionfs_lock(struct vop_lock1_args *ap)
 	if ((flags & LK_INTERLOCK) == 0)
 		VI_LOCK(vp);
 
-	mp = vp->v_mount;
-	if (mp == NULL)
-		goto unionfs_lock_null_vnode;
-
-	ump = MOUNTTOUNIONFSMOUNT(mp);
 	unp = VTOUNIONFS(vp);
-	if (ump == NULL || unp == NULL)
+	if (unp == NULL)
 		goto unionfs_lock_null_vnode;
 	lvp = unp->un_lowervp;
 	uvp = unp->un_uppervp;
@@ -1967,7 +1960,7 @@ unionfs_lock(struct vop_lock1_args *ap)
 	 * (ex. vfs_domount: mounted vnode is already locked.)
 	 */
 	if ((flags & LK_TYPE_MASK) == LK_EXCLUSIVE &&
-	    vp == ump->um_rootvp)
+	    (vp->v_vflag & VV_ROOT) != 0)
 		flags |= LK_CANRECURSE;
 
 	if (lvp != NULLVP) {