From owner-svn-src-head@freebsd.org Mon Feb 10 13:54:34 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id BEDF423B29B; Mon, 10 Feb 2020 13:54:34 +0000 (UTC) (envelope-from mjg@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) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 48GS7t4Wjwz4WbT; Mon, 10 Feb 2020 13:54:34 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 95CBB20171; Mon, 10 Feb 2020 13:54:34 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 01ADsY8L039135; Mon, 10 Feb 2020 13:54:34 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 01ADsYuA039134; Mon, 10 Feb 2020 13:54:34 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <202002101354.01ADsYuA039134@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Mon, 10 Feb 2020 13:54:34 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r357729 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: mjg X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 357729 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Feb 2020 13:54:34 -0000 Author: mjg Date: Mon Feb 10 13:54:34 2020 New Revision: 357729 URL: https://svnweb.freebsd.org/changeset/base/357729 Log: vfs: fix lock recursion in vrele vrele is supposed to be called with an unlocked vnode, but this was never asserted for if v_usecount was > 0. For such counts the lock is never touched by the routine. As a result the kernel has several consumers which expect vunref semantics and get away with calling vrele since they happen to never do it when this is the last reference (and for some of them this may happen to be a guarantee). Work around the problem by changing vrele semantics to tolerate being called with a lock. This eliminates a possible bug where the lock is already held and vputx takes it anyway. Reviewed by: kib Tested by: pho Differential Revision: https://reviews.freebsd.org/D23528 Modified: head/sys/kern/vfs_subr.c Modified: head/sys/kern/vfs_subr.c ============================================================================== --- head/sys/kern/vfs_subr.c Mon Feb 10 13:52:25 2020 (r357728) +++ head/sys/kern/vfs_subr.c Mon Feb 10 13:54:34 2020 (r357729) @@ -3170,6 +3170,7 @@ static void vputx(struct vnode *vp, enum vputx_op func) { int error; + bool want_unlock; KASSERT(vp != NULL, ("vputx: null vp")); if (func == VPUTX_VUNREF) @@ -3221,13 +3222,31 @@ vputx(struct vnode *vp, enum vputx_op func) * as VI_DOINGINACT to avoid recursion. */ vp->v_iflag |= VI_OWEINACT; + want_unlock = false; + error = 0; switch (func) { case VPUTX_VRELE: - error = vn_lock(vp, LK_EXCLUSIVE | LK_INTERLOCK); - VI_LOCK(vp); + switch (VOP_ISLOCKED(vp)) { + case LK_EXCLUSIVE: + break; + case LK_EXCLOTHER: + case 0: + want_unlock = true; + error = vn_lock(vp, LK_EXCLUSIVE | LK_INTERLOCK); + VI_LOCK(vp); + break; + default: + /* + * The lock has at least one sharer, but we have no way + * to conclude whether this is us. Play it safe and + * defer processing. + */ + error = EAGAIN; + break; + } break; case VPUTX_VPUT: - error = 0; + want_unlock = true; if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) { error = VOP_LOCK(vp, LK_UPGRADE | LK_INTERLOCK | LK_NOWAIT); @@ -3235,7 +3254,6 @@ vputx(struct vnode *vp, enum vputx_op func) } break; case VPUTX_VUNREF: - error = 0; if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) { error = VOP_LOCK(vp, LK_TRYUPGRADE | LK_INTERLOCK); VI_LOCK(vp); @@ -3244,7 +3262,7 @@ vputx(struct vnode *vp, enum vputx_op func) } if (error == 0) { vinactive(vp); - if (func != VPUTX_VUNREF) + if (want_unlock) VOP_UNLOCK(vp); vdropl(vp); } else {