git: fcb164742b6f - main - unionfs: rework unionfs_getwritemount()

From: Jason A. Harmening <jah_at_FreeBSD.org>
Date: Thu, 24 Feb 2022 04:00:38 UTC
The branch main has been updated by jah:

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

commit fcb164742b6fb3c0f0d5995f90955acb6706d73e
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2022-02-15 03:52:21 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2022-02-24 04:10:02 +0000

    unionfs: rework unionfs_getwritemount()
    
    VOP_GETWRITEMOUNT() is called on the vn_start_write() path without any
    vnode locks guaranteed to be held.  It's therefore unsafe to blindly
    access per-mount and per-vnode data.  Instead, follow the approach taken
    by nullfs and use the vnode interlock coupled with the hold count to
    ensure the mount and the vnode won't be recycled while they are being
    accessed.
    
    Reviewed by:    kib (earlier version), markj, pho
    Tested by:      pho
    Differential Revision: https://reviews.freebsd.org/D34282
---
 sys/fs/unionfs/union_vnops.c | 49 ++++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index 8e881ffd19bb..dcea0c27abd5 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -1764,33 +1764,52 @@ unionfs_readlink(struct vop_readlink_args *ap)
 static int
 unionfs_getwritemount(struct vop_getwritemount_args *ap)
 {
+	struct unionfs_node *unp;
 	struct vnode   *uvp;
-	struct vnode   *vp;
+	struct vnode   *vp, *ovp;
 	int		error;
 
 	UNIONFS_INTERNAL_DEBUG("unionfs_getwritemount: enter\n");
 
 	error = 0;
 	vp = ap->a_vp;
+	uvp = NULLVP;
 
-	if (vp == NULLVP || (vp->v_mount->mnt_flag & MNT_RDONLY))
-		return (EACCES);
-
-	KASSERT_UNIONFS_VNODE(vp);
-
-	uvp = UNIONFSVPTOUPPERVP(vp);
-	if (uvp == NULLVP && VREG == vp->v_type)
-		uvp = UNIONFSVPTOUPPERVP(VTOUNIONFS(vp)->un_dvp);
+	VI_LOCK(vp);
+	unp = VTOUNIONFS(vp);
+	if (unp != NULL)
+		uvp = unp->un_uppervp;
 
-	if (uvp != NULLVP)
-		error = VOP_GETWRITEMOUNT(uvp, ap->a_mpp);
-	else {
+	/*
+	 * If our node has no upper vnode, check the parent directory.
+	 * We may be initiating a write operation that will produce a
+	 * new upper vnode through CoW.
+	 */
+	if (uvp == NULLVP && unp != NULL) {
+		ovp = vp;
+		vp = unp->un_dvp;
+		/*
+		 * Only the root vnode should have an empty parent, but it
+		 * should not have an empty uppervp, so we shouldn't get here.
+		 */
+		VNASSERT(vp != NULL, ovp, ("%s: NULL parent vnode", __func__));
+		VI_UNLOCK(ovp);
 		VI_LOCK(vp);
-		if (vp->v_holdcnt == 0)
-			error = EOPNOTSUPP;
-		else
+		unp = VTOUNIONFS(vp);
+		if (unp != NULL)
+			uvp = unp->un_uppervp;
+		if (uvp == NULLVP)
 			error = EACCES;
+	}
+
+	if (uvp != NULLVP) {
+		vholdnz(uvp);
+		VI_UNLOCK(vp);
+		error = VOP_GETWRITEMOUNT(uvp, ap->a_mpp);
+		vdrop(uvp);
+	} else {
 		VI_UNLOCK(vp);
+		*(ap->a_mpp) = NULL;
 	}
 
 	UNIONFS_INTERNAL_DEBUG("unionfs_getwritemount: leave (%d)\n", error);