git: 93fe61afde72 - main - unionfs_mkdir(): handle dvp reclamation

From: Jason A. Harmening <jah_at_FreeBSD.org>
Date: Tue, 18 Apr 2023 01:35:45 UTC
The branch main has been updated by jah:

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

commit 93fe61afde72e6841251ea43551631c30556032d
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2023-01-16 21:50:59 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2023-04-18 01:31:40 +0000

    unionfs_mkdir(): handle dvp reclamation
    
    The underlying VOP_MKDIR() implementation may temporarily drop the
    parent directory vnode's lock.  If the vnode is reclaimed during that
    window, the unionfs vnode will effectively become unlocked because
    the its v_vnlock field will be reset.  To uphold the locking
    requirements of VOP_MKDIR() and to avoid triggering various VFS
    assertions, explicitly re-lock the unionfs vnode before returning
    in this case.
    
    Note that there are almost certainly other cases in which we'll
    similarly need to handle vnode relocking by the underlying FS; this
    is the only one that's caused problems in stress testing so far.
    A more general solution, such as that employed for nullfs in
    null_bypass(), will likely need to be implemented.
    
    Tested by:      pho
    Reviewed by:    kib, markj
    Differential Revision: https://reviews.freebsd.org/D39272
---
 sys/fs/unionfs/union_vnops.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index 0da5ecb61bb2..6c8086a6c7c5 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -1389,6 +1389,7 @@ unionfs_mkdir(struct vop_mkdir_args *ap)
 {
 	struct unionfs_node *dunp;
 	struct componentname *cnp;
+	struct vnode   *dvp;
 	struct vnode   *udvp;
 	struct vnode   *uvp;
 	struct vattr	va;
@@ -1400,17 +1401,19 @@ unionfs_mkdir(struct vop_mkdir_args *ap)
 	KASSERT_UNIONFS_VNODE(ap->a_dvp);
 
 	error = EROFS;
-	dunp = VTOUNIONFS(ap->a_dvp);
+	dvp = ap->a_dvp;
+	dunp = VTOUNIONFS(dvp);
 	cnp = ap->a_cnp;
 	lkflags = cnp->cn_lkflags;
 	udvp = dunp->un_uppervp;
 
 	if (udvp != NULLVP) {
+		vref(udvp);
 		/* check opaque */
 		if (!(cnp->cn_flags & ISWHITEOUT)) {
 			error = VOP_GETATTR(udvp, &va, cnp->cn_cred);
 			if (error != 0)
-				return (error);
+				goto unionfs_mkdir_cleanup;
 			if ((va.va_flags & OPAQUE) != 0)
 				cnp->cn_flags |= ISWHITEOUT;
 		}
@@ -1418,13 +1421,35 @@ unionfs_mkdir(struct vop_mkdir_args *ap)
 		if ((error = VOP_MKDIR(udvp, &uvp, cnp, ap->a_vap)) == 0) {
 			VOP_UNLOCK(uvp);
 			cnp->cn_lkflags = LK_EXCLUSIVE;
-			error = unionfs_nodeget(ap->a_dvp->v_mount, uvp, NULLVP,
-			    ap->a_dvp, ap->a_vpp, cnp);
+			/*
+			 * The underlying VOP_MKDIR() implementation may have
+			 * temporarily dropped the parent directory vnode lock.
+			 * Because the unionfs vnode ordinarily shares that
+			 * lock, this may allow the unionfs vnode to be reclaimed
+			 * and its lock field reset.  In that case, the unionfs
+			 * vnode is effectively no longer locked, and we must
+			 * explicitly lock it before returning in order to meet
+			 * the locking requirements of VOP_MKDIR().
+			 */
+			if (__predict_false(VTOUNIONFS(dvp) == NULL)) {
+				error = ENOENT;
+				goto unionfs_mkdir_cleanup;
+			}
+			error = unionfs_nodeget(dvp->v_mount, uvp, NULLVP,
+			    dvp, ap->a_vpp, cnp);
 			cnp->cn_lkflags = lkflags;
 			vrele(uvp);
 		}
 	}
 
+unionfs_mkdir_cleanup:
+
+	if (__predict_false(VTOUNIONFS(dvp) == NULL)) {
+		vput(udvp);
+		vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY);
+	} else if (udvp != NULLVP)
+		vrele(udvp);
+
 	UNIONFS_INTERNAL_DEBUG("unionfs_mkdir: leave (%d)\n", error);
 
 	return (error);