git: 06f79675b7a0 - main - unionfs: fix potential deadlock in VOP_RMDIR
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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);