git: 06f79675b7a0 - main - unionfs: fix potential deadlock in VOP_RMDIR

From: Jason A. Harmening <jah_at_FreeBSD.org>
Date: Mon, 15 Nov 2021 04:00:35 UTC
The branch main has been updated by jah:

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

commit 06f79675b7a0c8766c536dec686efba9e8447a73
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2021-11-14 07:28:29 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2021-11-15 04:07:42 +0000

    unionfs: fix potential deadlock in VOP_RMDIR
    
    VOP_RMDIR() is called with both parent and child directory vnodes
    locked.  The relookup operation performed by the unionfs implementation
    may relock both vnodes.  Accordingly, unionfs_relookup() drops the
    parent vnode lock, but the child vnode lock is never dropped.
    Although relookup() will very likely try to relock the child vnode
    which is already locked, in most cases this doesn't produce a deadlock
    because unionfs_lock() forces LK_CANRECURSE (!).  However, relocking
    of the parent vnode while the child vnode remains locked effectively
    reverses the expected parent->child lock order, which can produce
    a deadlock e.g. in the presence of a concurrent unionfs_lookup()
    operation.  Address the issue by dropping the child lock around
    the unionfs_relookup() call in unionfs_rmdir().
    
    Reported by:    pho
    Reviewed by:    kib, markj
    Differential Revision: https://reviews.freebsd.org/D32986
---
 sys/fs/unionfs/union_vnops.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index b6c088fb3804..81741dc19349 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -1434,11 +1434,23 @@ unionfs_rmdir(struct vop_rmdir_args *ap)
 		ump = MOUNTTOUNIONFSMOUNT(ap->a_vp->v_mount);
 		if (ump->um_whitemode == UNIONFS_WHITE_ALWAYS || lvp != NULLVP)
 			cnp->cn_flags |= DOWHITEOUT;
+		/*
+		 * The relookup path will need to relock the parent dvp and
+		 * possibly the vp as well.  Locking is expected to be done
+		 * in parent->child order; drop the lock on vp to avoid LOR
+		 * and potential recursion on vp's lock.
+		 * vp is expected to remain referenced during VOP_RMDIR(),
+		 * so vref/vrele should not be necessary here.
+		 */
+		VOP_UNLOCK(ap->a_vp);
+		VNPASS(vrefcnt(ap->a_vp) > 0, ap->a_vp);
 		error = unionfs_relookup_for_delete(ap->a_dvp, cnp, td);
+		vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY);
+		if (error == 0 && VN_IS_DOOMED(uvp))
+			error = ENOENT;
 		if (error == 0)
 			error = VOP_RMDIR(udvp, uvp, cnp);
-	}
-	else if (lvp != NULLVP)
+	} else if (lvp != NULLVP)
 		error = unionfs_mkwhiteout(udvp, cnp, td,
 		    unp->un_path, unp->un_pathlen);