Christos Zoulas <christos@netbsd.org> writes:

> Module Name:	syssrc
> Committed By:	christos
> Date:		Sun Mar 31 20:53:26 UTC 2002
> 
> Modified Files:
> 	syssrc/sys/ufs/ffs: ffs_vfsops.c
> 
> Log Message:
> PR/16136: Chris Jepeway: Bogus entry in /etc/fstab can panic kernel.
> 
> 
> To generate a diff of this commit:
> cvs rdiff -r1.94 -r1.95 syssrc/sys/ufs/ffs/ffs_vfsops.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.

I suspect how well this change is reviewed.  It at least introduces
one bug and one bogus check and points out some ambiguity.

- If VOP_ACCESS fails when updating mount, we will vrele() twice.

- The check for update-only flags in mp->mnt_flag when not updating
  case is bogus.  If we really want to check, we need to see flags in
  ufs_args, but I'm not sure if it is really necessary.

- The credential passed to ffs_reload was credential of when looking
  up mount point, but now it is credential of when looking up device
  node.  Anyway, it may be current process's credential.

enami.

c.f.:

Index: ffs_vfsops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/ufs/ffs/ffs_vfsops.c,v
retrieving revision 1.95
diff -u -r1.95 ffs_vfsops.c
--- ffs_vfsops.c	2002/03/31 20:53:25	1.95
+++ ffs_vfsops.c	2002/04/01 01:03:53
@@ -207,27 +207,9 @@
 	} else {
 		/* New mounts must have a filename for the device */
 		if (args.fspec == NULL)
-			return EINVAL;
-
-		/* Check for update-only flags */
-		if (mp->mnt_flag &
-		    (MNT_WANTRDWR |	/*
-					 * Upgrading from read-only to
-					 * read-write can only occur after
-					 * the initial mount
-					 */
-		     MNT_EXRDONLY |
-		     MNT_DEFEXPORTED |
-		     MNT_EXPORTANON |
-		     MNT_EXKERB |	/* Only update mounts are allowed */
-		     MNT_EXNORESPORT |  /* to affect exporting		  */
-		     MNT_EXPUBLIC |
-		     MNT_DELEXPORT))
-		return EINVAL;
+			return (EINVAL);
 	}
 
-	error = 0;
-
 	if (args.fspec) {
 		/*
 		 * Look up the name and verify that it's sane.
@@ -252,13 +234,6 @@
 			 */
 			if (devvp != ump->um_devvp)
 				error = EINVAL;
-			else
-				/*
-				 * The initial mount got a reference on this
-				 * device, so drop the one obtained via
-				 * namei(), above
-				 */
-				vrele(devvp);
 		}
 	}
 
@@ -268,8 +243,9 @@
 	 */
 	if (error == 0 && p->p_ucred->cr_uid != 0) {
 		accessmode = VREAD;
-		if ((!update && (mp->mnt_flag & MNT_RDONLY) == 0) ||
-		     (update && (mp->mnt_flag & MNT_WANTRDWR)))
+		if (update ?
+		    (mp->mnt_flag & MNT_WANTRDWR) != 0 :
+		    (mp->mnt_flag & MNT_RDONLY) == 0)
 			accessmode |= VWRITE;
 		vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY);
 		error = VOP_ACCESS(devvp, accessmode, p->p_ucred, p);
@@ -293,14 +269,22 @@
 		if ((mp->mnt_flag & (MNT_SOFTDEP | MNT_ASYNC)) ==
 		    (MNT_SOFTDEP | MNT_ASYNC)) {
 			printf("%s fs uses soft updates, "
-			       "ignoring async mode\n",
-			       fs->fs_fsmnt);
+			    "ignoring async mode\n",
+			    fs->fs_fsmnt);
 			mp->mnt_flag &= ~MNT_ASYNC;
 		}
 	} else {
+		/*
+		 * Update the mount.
+		 */
+
 		/*
-		 * Update the mount
+		 * The initial mount got a reference on this
+		 * device, so drop the one obtained via
+		 * namei(), above.
 		 */
+		vrele(devvp);
+
 		fs = ump->um_fs;
 		if (fs->fs_ronly == 0 && (mp->mnt_flag & MNT_RDONLY)) {
 			/*
@@ -372,7 +356,7 @@
 		}
 
 		if (mp->mnt_flag & MNT_RELOAD) {
-			error = ffs_reload(mp, ndp->ni_cnd.cn_cred, p);
+			error = ffs_reload(mp, p->p_ucred, p);
 			if (error)
 				return (error);
 		}