git: a498c5e1a111 - main - unionfs: avoid vdrop()ing a locked but doomed vnode

From: Jason A. Harmening <jah_at_FreeBSD.org>
Date: Thu, 16 Oct 2025 15:31:28 UTC
The branch main has been updated by jah:

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

commit a498c5e1a111ae8c48b8e7028daff861785c2bf2
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2025-10-14 19:40:39 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2025-10-16 15:30:34 +0000

    unionfs: avoid vdrop()ing a locked but doomed vnode
    
    unionfs_lock() unconditionally calls vdrop() on the target vnode
    after locking it, but it's possible this vnode may be doomed.
    In that case, vdrop() may free the vnode, which in certain cases
    requires taking the vnode lock.  Commit a7aac8c20497d added an
    assert to this effect, which unionfs_lock() now trips over.
    
    Fix this by lightly reworking the flow of unionfs_lock() so that
    the target vnode is vdrop()ed after being unlocked in the case
    where the unionfs lock operation needs to be restarted (which
    will happen if the unionfs vnode has been doomed, which is a
    prerequisite for the target vnode in the underlying filesystem
    to have been doomed).
    
    While here, get rid of a superfluous vhold/vdrop sequence in
    unionfs_unlock() that was probably inherited from nullfs and whose
    nullfs equivalent was recently removed.
    
    MFC after:      1 week
    Reviewed by:    kib, markj, olce
    Tested by:      pho
    Differential Revision: https://reviews.freebsd.org/D53107
---
 sys/fs/unionfs/union_vnops.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index 26fa14603c85..66fee97a07d5 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -2208,7 +2208,6 @@ unionfs_lock_restart:
 	vholdnz(tvp);
 	VI_UNLOCK(vp);
 	error = VOP_LOCK(tvp, flags);
-	vdrop(tvp);
 	if (error == 0 && (lvp_locked || VTOUNIONFS(vp) == NULL)) {
 		/*
 		 * After dropping the interlock above, there exists a window
@@ -2234,6 +2233,7 @@ unionfs_lock_restart:
 		unp = VTOUNIONFS(vp);
 		if (unp == NULL || unp->un_uppervp != NULL) {
 			VOP_UNLOCK(tvp);
+			vdrop(tvp);
 			/*
 			 * If we previously held the lock, the upgrade may
 			 * have temporarily dropped the lock, in which case
@@ -2249,6 +2249,7 @@ unionfs_lock_restart:
 			goto unionfs_lock_restart;
 		}
 	}
+	vdrop(tvp);
 
 	return (error);
 }
@@ -2259,7 +2260,6 @@ unionfs_unlock(struct vop_unlock_args *ap)
 	struct vnode   *vp;
 	struct vnode   *tvp;
 	struct unionfs_node *unp;
-	int		error;
 
 	KASSERT_UNIONFS_VNODE(ap->a_vp);
 
@@ -2271,11 +2271,7 @@ unionfs_unlock(struct vop_unlock_args *ap)
 
 	tvp = (unp->un_uppervp != NULL ? unp->un_uppervp : unp->un_lowervp);
 
-	vholdnz(tvp);
-	error = VOP_UNLOCK(tvp);
-	vdrop(tvp);
-
-	return (error);
+	return (VOP_UNLOCK(tvp));
 }
 
 static int