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
This commit is contained in:
Jason A. Harmening 2021-11-13 23:28:29 -08:00
parent 8660813153
commit 06f79675b7

View File

@ -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);