ffs_mount(): return early if namei() fails to lookup disk device

With soft updates enabled, an INVARIANTS panic is hit in ffs_unmount().

The problem occurs in ffs_mount() when upgrading a mount from ro->rw.
During a mount update, the soft update code gets set up but doesn't get
cleaned up if namei() fails when looking up the disk device.

Avoid this scenario by looking up the disk device first and bail early
if the namei() lookup fails.

PR:             256511
MFC After:      2 weeks
Reviewed by:	mckusick, kib
Differential Revision:	https://reviews.freebsd.org/D30870

1 files changed, 72 insertions, 76 deletions

diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c
index a2a361e813e4..578e71014a23 100644
--- a/sys/ufs/ffs/ffs_vfsops.c
+++ b/sys/ufs/ffs/ffs_vfsops.c
@@ -409,15 +409,84 @@ ffs_mount(struct mount *mp)
 	mp->mnt_kern_flag &= ~MNTK_FPLOOKUP;
 	mp->mnt_flag |= mntorflags;
 	MNT_IUNLOCK(mp);
+
+	/*
+	 * Must not call namei() while owning busy ref.
+	 */
+	if (mp->mnt_flag & MNT_UPDATE)
+		vfs_unbusy(mp);
+
+	/*
+	 * Not an update, or updating the name: look up the name
+	 * and verify that it refers to a sensible disk device.
+	 */
+	NDINIT(&ndp, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, fspec);
+	error = namei(&ndp);
+	if ((mp->mnt_flag & MNT_UPDATE) != 0) {
+		/*
+		 * Unmount does not start if MNT_UPDATE is set.  Mount
+		 * update busies mp before setting MNT_UPDATE.  We
+		 * must be able to retain our busy ref successfully,
+		 * without sleep.
+		 */
+		error1 = vfs_busy(mp, MBF_NOWAIT);
+		MPASS(error1 == 0);
+	}
+	if (error != 0)
+		return (error);
+	NDFREE(&ndp, NDF_ONLY_PNBUF);
+	if (!vn_isdisk_error(ndp.ni_vp, &error)) {
+		vput(ndp.ni_vp);
+		return (error);
+	}
+
 	/*
-	 * If updating, check whether changing from read-only to
-	 * read/write; if there is no device name, that's all we do.
+	 * If mount by non-root, then verify that user has necessary
+	 * permissions on the device.
+	 */
+	accmode = VREAD;
+	if ((mp->mnt_flag & MNT_RDONLY) == 0)
+		accmode |= VWRITE;
+	error = VOP_ACCESS(ndp.ni_vp, accmode, td->td_ucred, td);
+	if (error)
+		error = priv_check(td, PRIV_VFS_MOUNT_PERM);
+	if (error) {
+		vput(ndp.ni_vp);
+		return (error);
+	}
+
+	/*
+	 * New mount
+	 *
+	 * We need the name for the mount point (also used for
+	 * "last mounted on") copied in. If an error occurs,
+	 * the mount point is discarded by the upper level code.
+	 * Note that vfs_mount_alloc() populates f_mntonname for us.
 	 */
-	if (mp->mnt_flag & MNT_UPDATE) {
+	if ((mp->mnt_flag & MNT_UPDATE) == 0) {
+		if ((error = ffs_mountfs(ndp.ni_vp, mp, td)) != 0) {
+			vrele(ndp.ni_vp);
+			return (error);
+		}
+	} else {
+		/*
+		 * When updating, check whether changing from read-only to
+		 * read/write; if there is no device name, that's all we do.
+		 */
 		ump = VFSTOUFS(mp);
 		fs = ump->um_fs;
 		odevvp = ump->um_odevvp;
 		devvp = ump->um_devvp;
+
+		/*
+		 * If it's not the same vnode, or at least the same device
+		 * then it's not correct.
+		 */
+		if (ndp.ni_vp->v_rdev != ump->um_odevvp->v_rdev)
+			error = EINVAL; /* needs translation */
+		vput(ndp.ni_vp);
+		if (error)
+			return (error);
 		if (fs->fs_ronly == 0 &&
 		    vfs_flagopt(mp->mnt_optnew, "ro", NULL, 0)) {
 			/*
@@ -620,79 +689,6 @@ ffs_mount(struct mount *mp)
 		 */
 		if (mp->mnt_flag & MNT_SNAPSHOT)
 			return (ffs_snapshot(mp, fspec));
-
-		/*
-		 * Must not call namei() while owning busy ref.
-		 */
-		vfs_unbusy(mp);
-	}
-
-	/*
-	 * Not an update, or updating the name: look up the name
-	 * and verify that it refers to a sensible disk device.
-	 */
-	NDINIT(&ndp, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, fspec);
-	error = namei(&ndp);
-	if ((mp->mnt_flag & MNT_UPDATE) != 0) {
-		/*
-		 * Unmount does not start if MNT_UPDATE is set.  Mount
-		 * update busies mp before setting MNT_UPDATE.  We
-		 * must be able to retain our busy ref succesfully,
-		 * without sleep.
-		 */
-		error1 = vfs_busy(mp, MBF_NOWAIT);
-		MPASS(error1 == 0);
-	}
-	if (error != 0)
-		return (error);
-	NDFREE(&ndp, NDF_ONLY_PNBUF);
-	devvp = ndp.ni_vp;
-	if (!vn_isdisk_error(devvp, &error)) {
-		vput(devvp);
-		return (error);
-	}
-
-	/*
-	 * If mount by non-root, then verify that user has necessary
-	 * permissions on the device.
-	 */
-	accmode = VREAD;
-	if ((mp->mnt_flag & MNT_RDONLY) == 0)
-		accmode |= VWRITE;
-	error = VOP_ACCESS(devvp, accmode, td->td_ucred, td);
-	if (error)
-		error = priv_check(td, PRIV_VFS_MOUNT_PERM);
-	if (error) {
-		vput(devvp);
-		return (error);
-	}
-
-	if (mp->mnt_flag & MNT_UPDATE) {
-		/*
-		 * Update only
-		 *
-		 * If it's not the same vnode, or at least the same device
-		 * then it's not correct.
-		 */
-
-		if (devvp->v_rdev != ump->um_devvp->v_rdev)
-			error = EINVAL;	/* needs translation */
-		vput(devvp);
-		if (error)
-			return (error);
-	} else {
-		/*
-		 * New mount
-		 *
-		 * We need the name for the mount point (also used for
-		 * "last mounted on") copied in. If an error occurs,
-		 * the mount point is discarded by the upper level code.
-		 * Note that vfs_mount_alloc() populates f_mntonname for us.
-		 */
-		if ((error = ffs_mountfs(devvp, mp, td)) != 0) {
-			vrele(devvp);
-			return (error);
-		}
 	}
 
 	MNT_ILOCK(mp);