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

1 files changed, 18 insertions, 12 deletions

diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c
index 94ba43ff0645..4c7e8781d1ba 100644
--- a/sys/kern/vfs_default.c
+++ b/sys/kern/vfs_default.c
@@ -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);
 }