From nobody Mon Nov 15 04:00:35 2021 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 5044418659E6; Mon, 15 Nov 2021 04:00:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HswTJ1jBcz4Twj; Mon, 15 Nov 2021 04:00:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 1998612FAB; Mon, 15 Nov 2021 04:00:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1AF40ZMR004531; Mon, 15 Nov 2021 04:00:35 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1AF40ZNI004530; Mon, 15 Nov 2021 04:00:35 GMT (envelope-from git) Date: Mon, 15 Nov 2021 04:00:35 GMT Message-Id: <202111150400.1AF40ZNI004530@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Jason A. Harmening" Subject: git: 06f79675b7a0 - main - unionfs: fix potential deadlock in VOP_RMDIR List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jah X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 06f79675b7a0c8766c536dec686efba9e8447a73 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by jah: URL: https://cgit.FreeBSD.org/src/commit/?id=06f79675b7a0c8766c536dec686efba9e8447a73 commit 06f79675b7a0c8766c536dec686efba9e8447a73 Author: Jason A. Harmening AuthorDate: 2021-11-14 07:28:29 +0000 Commit: Jason A. Harmening 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);