1 files changed, 48 insertions, 27 deletions

diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
index a6e38be89291..73e110c05bc1 100644
--- a/sys/kern/vfs_subr.c
+++ b/sys/kern/vfs_subr.c
@@ -2186,6 +2186,8 @@ freevnode(struct vnode *vp)
 {
 	struct bufobj *bo;
 
+	ASSERT_VOP_UNLOCKED(vp, __func__);
+
 	/*
 	 * The vnode has been marked for destruction, so free it.
 	 *
@@ -2222,12 +2224,16 @@ freevnode(struct vnode *vp)
 	mac_vnode_destroy(vp);
 #endif
 	if (vp->v_pollinfo != NULL) {
+		int error __diagused;
+
 		/*
 		 * Use LK_NOWAIT to shut up witness about the lock. We may get
 		 * here while having another vnode locked when trying to
 		 * satisfy a lookup and needing to recycle.
 		 */
-		VOP_LOCK(vp, LK_EXCLUSIVE | LK_NOWAIT);
+		error = VOP_LOCK(vp, LK_EXCLUSIVE | LK_NOWAIT);
+		VNASSERT(error == 0, vp,
+		    ("freevnode: cannot lock vp %p for pollinfo destroy", vp));
 		destroy_vpollinfo(vp->v_pollinfo);
 		VOP_UNLOCK(vp);
 		vp->v_pollinfo = NULL;
@@ -3346,13 +3352,22 @@ vget_abort(struct vnode *vp, enum vgetstate vs)
 	switch (vs) {
 	case VGET_USECOUNT:
 		vrele(vp);
-		break;
+		goto out_ok;
 	case VGET_HOLDCNT:
 		vdrop(vp);
+		goto out_ok;
+	case VGET_NONE:
 		break;
-	default:
-		__assert_unreachable();
 	}
+
+	__assert_unreachable();
+
+	/*
+	 * This is a goto label should the cases above have more in common than
+	 * just the 'return' statement.
+	 */
+out_ok:
+	return;
 }
 
 int
@@ -3561,11 +3576,6 @@ enum vput_op { VRELE, VPUT, VUNREF };
  * exclusive lock on the vnode, while it is legal to call here with only a
  * shared lock (or no locks). If locking the vnode in an expected manner fails,
  * inactive processing gets deferred to the syncer.
- *
- * XXX Some filesystems pass in an exclusively locked vnode and strongly depend
- * on the lock being held all the way until VOP_INACTIVE. This in particular
- * happens with UFS which adds half-constructed vnodes to the hash, where they
- * can be found by other code.
  */
 static void
 vput_final(struct vnode *vp, enum vput_op func)
@@ -3643,26 +3653,26 @@ vput_final(struct vnode *vp, enum vput_op func)
 		}
 		break;
 	}
-	if (error == 0) {
-		if (func == VUNREF) {
-			VNASSERT((vp->v_vflag & VV_UNREF) == 0, vp,
-			    ("recursive vunref"));
-			vp->v_vflag |= VV_UNREF;
-		}
-		for (;;) {
-			error = vinactive(vp);
-			if (want_unlock)
-				VOP_UNLOCK(vp);
-			if (error != ERELOOKUP || !want_unlock)
-				break;
-			VOP_LOCK(vp, LK_EXCLUSIVE);
-		}
-		if (func == VUNREF)
-			vp->v_vflag &= ~VV_UNREF;
-		vdropl(vp);
-	} else {
+	if (error != 0) {
 		vdefer_inactive(vp);
+		return;
+	}
+	if (func == VUNREF) {
+		VNASSERT((vp->v_vflag & VV_UNREF) == 0, vp,
+		    ("recursive vunref"));
+		vp->v_vflag |= VV_UNREF;
+	}
+	for (;;) {
+		error = vinactive(vp);
+		if (want_unlock)
+			VOP_UNLOCK(vp);
+		if (error != ERELOOKUP || !want_unlock)
+			break;
+		VOP_LOCK(vp, LK_EXCLUSIVE);
 	}
+	if (func == VUNREF)
+		vp->v_vflag &= ~VV_UNREF;
+	vdropl(vp);
 	return;
 out:
 	if (func == VPUT)
@@ -4501,6 +4511,17 @@ vgonel(struct vnode *vp)
 	/*
 	 * Done with purge, reset to the standard lock and invalidate
 	 * the vnode.
+	 *
+	 * FIXME: this is buggy for vnode ops with custom locking primitives.
+	 *
+	 * vget used to be gated with a special flag serializing it against vgone,
+	 * which got lost in the process of SMP-ifying the VFS layer.
+	 *
+	 * Suppose a custom locking routine references ->v_data.
+	 *
+	 * Since now it is possible to start executing it as vgone is
+	 * progressing, this very well may crash as ->v_data gets invalidated
+	 * and memory used to back it is freed.
 	 */
 	vp->v_vnlock = &vp->v_lock;
 	vp->v_op = &dead_vnodeops;