Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 10 Feb 2020 13:54:34 +0000 (UTC)
From:      Mateusz Guzik <mjg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r357729 - head/sys/kern
Message-ID:  <202002101354.01ADsYuA039134@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
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 {

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202002101354.01ADsYuA039134>