svn commit: r351656 - head/sys/kern

[Mateusz Guzik]

Author: mjg
Date: Sun Sep  1 14:01:09 2019
New Revision: 351656
URL: https://svnweb.freebsd.org/changeset/base/351656

Log:
  vfs: stop refing freed mount points in vop_stdgetwritemount
  
  The code used blindly ref based on an unsafely red address and then would
  backpedal if necessary. This was safe in terms of memory access since
  mounts are type-stable, but made for a potential a bug where the mount
  was reused and had the count reset to 0 before this code decreased it.
  
  Reviewed by:	kib
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D21411

Modified:
  head/sys/kern/vfs_default.c

Modified: head/sys/kern/vfs_default.c
==============================================================================
--- head/sys/kern/vfs_default.c	Sun Sep  1 10:39:16 2019	(r351655)
+++ head/sys/kern/vfs_default.c	Sun Sep  1 14:01:09 2019	(r351656)
@@ -588,22 +588,28 @@ vop_stdgetwritemount(ap)
 	} */ *ap;
 {
 	struct mount *mp;
+	struct vnode *vp;
 
 	/*
-	 * XXX Since this is called unlocked we may be recycled while
-	 * attempting to ref the mount.  If this is the case or mountpoint
-	 * will be set to NULL.  We only have to prevent this call from
-	 * returning with a ref to an incorrect mountpoint.  It is not
-	 * harmful to return with a ref to our previous mountpoint.
+	 * Note that having a reference does not prevent forced unmount from
+	 * setting ->v_mount to NULL after the lock gets released. This is of
+	 * no consequence for typical consumers (most notably vn_start_write)
+	 * since in this case the vnode is VI_DOOMED. Unmount might have
+	 * progressed far enough that its completion is only delayed by the
+	 * reference obtained here. The consumer only needs to concern itself
+	 * with releasing it.
 	 */
-	mp = ap->a_vp->v_mount;
-	if (mp != NULL) {
-		vfs_ref(mp);
-		if (mp != ap->a_vp->v_mount) {
-			vfs_rel(mp);
-			mp = NULL;
-		}
+	vp = ap->a_vp;
+	mp = vp->v_mount;
+	MNT_ILOCK(mp);
+	if (mp != vp->v_mount) {
+		MNT_IUNLOCK(mp);
+		mp = NULL;
+		goto out;
 	}
+	MNT_REF(mp);
+	MNT_IUNLOCK(mp);
+out:
 	*(ap->a_mpp) = mp;
 	return (0);
 }