Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 23 Jun 2006 18:22:22 GMT
From:      John Baldwin <jhb@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 99879 for review
Message-ID:  <200606231822.k5NIMMDV040366@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help 
http://perforce.freebsd.org/chv.cgi?CH=99879

Change 99879 by jhb@jhb_mutex on 2006/06/23 18:22:14

	- Expand Giant coverage in mount().  Something has to protect against
	  the vfsp being free'd out from under us while we are doing the
	  unmount.
	- Expand Giant coverage in unmount().  Something has to protect mp
	  from being free'd out from under us by another concurrent unmount()
	  on another CPU (or due to preemption).

Affected files ...

.. //depot/projects/smpng/sys/kern/vfs_mount.c#62 edit

Differences ...

==== //depot/projects/smpng/sys/kern/vfs_mount.c#62 (text+ko) ====

@@ -744,19 +744,23 @@
 
 	fstype = malloc(MFSNAMELEN, M_TEMP, M_WAITOK);
 	error = copyinstr(uap->type, fstype, MFSNAMELEN, NULL);
-	if (!error) {
-		AUDIT_ARG(text, fstype);
-		mtx_lock(&Giant);	/* XXX ? */
-		vfsp = vfs_byname_kld(fstype, td, &error);
-		mtx_unlock(&Giant);
+	if (error) {
+		free(fstype, M_TEMP);
+		return (error);
 	}
+
+	AUDIT_ARG(text, fstype);
+	mtx_lock(&Giant);
+	vfsp = vfs_byname_kld(fstype, td, &error);
 	free(fstype, M_TEMP);
-	if (error)
-		return (error);
-	if (vfsp == NULL)
+	if (vfsp == NULL) {
+		mtx_unlock(&Giant);
 		return (ENOENT);
-	if (vfsp->vfc_vfsops->vfs_cmount == NULL)
+	}
+	if (vfsp->vfc_vfsops->vfs_cmount == NULL) {
+		mtx_unlock(&Giant);
 		return (EOPNOTSUPP);
+	}
 
 	ma = mount_argsu(ma, "fstype", uap->type, MNAMELEN);
 	ma = mount_argsu(ma, "fspath", uap->path, MNAMELEN);
@@ -765,6 +769,7 @@
 	ma = mount_argb(ma, !(uap->flags & MNT_NOEXEC), "noexec");
 
 	error = vfsp->vfc_vfsops->vfs_cmount(ma, uap->data, uap->flags, td);
+	mtx_unlock(&Giant);
 	return (error);
 }
 
@@ -1063,9 +1068,11 @@
 		return (error);
 	}
 	AUDIT_ARG(upath, td, pathbuf, ARG_UPATH1);
+	mtx_lock(&Giant);
 	if (uap->flags & MNT_BYFSID) {
 		/* Decode the filesystem ID. */
 		if (sscanf(pathbuf, "FSID:%d:%d", &id0, &id1) != 2) {
+			mtx_unlock(&Giant);
 			free(pathbuf, M_TEMP);
 			return (EINVAL);
 		}
@@ -1093,6 +1100,7 @@
 		 * now, so in the !MNT_BYFSID case return the more likely
 		 * EINVAL for compatibility.
 		 */
+		mtx_unlock(&Giant);
 		return ((uap->flags & MNT_BYFSID) ? ENOENT : EINVAL);
 	}
 
@@ -1101,15 +1109,18 @@
 	 * original mount is permitted to unmount this filesystem.
 	 */
 	error = vfs_suser(mp, td);
-	if (error)
+	if (error) {
+		mtx_unlock(&Giant);
 		return (error);
+	}
 
 	/*
 	 * Don't allow unmounting the root filesystem.
 	 */
-	if (mp->mnt_flag & MNT_ROOTFS)
+	if (mp->mnt_flag & MNT_ROOTFS) {
+		mtx_unlock(&Giant);
 		return (EINVAL);
-	mtx_lock(&Giant);
+	}
 	error = dounmount(mp, uap->flags, td);
 	mtx_unlock(&Giant);
 	return (error);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200606231822.k5NIMMDV040366>