Subject: bin/22338: allow multiple mounts on same directory
To: None <gnats-bugs@gnats.netbsd.org>
From: Alan Barrett <apb@cequrux.com>
List: netbsd-bugs
Date: 08/02/2003 13:43:02
>Number:         22338
>Category:       bin
>Synopsis:       allow multiple mounts on same directory
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sat Aug 02 11:44:00 UTC 2003
>Closed-Date:
>Last-Modified:
>Originator:     Alan Barrett
>Release:        NetBSD 1.6T
>Organization:
Not much
>Environment:
System: NetBSD 1.6T
>Description:
It's sometimes useful to mount several file systems on the same
directory.  This can be especially useful for union mounts.

However, "mount -a" contains a test that prevents this from working.

>How-To-Repeat:
Create the directories /source1, /source2 and /target.

Put the following lines in /etc/fstab:

	/source1	/target		null	rw,union	0 0
	/source2	/target		null	rw,union	0 0

and run "mount -av".  Only the first mount works.  The second mount
falls foul of a duplicate detection test in the mountfs() function near
line 352 of src/sbin/mount/mount.c.

However, if you run the following commands by hand, they work fine:

	mount -t null -o union /source1 /target
	mount -t null -o union /source2 /target

>Fix:
Apply the following patch.  (This was discussed in tech-userlevel around
18 July 2003.)

Index: src/sbin/mount/mount.c
--- mount.c	19 Jan 2003 10:49:12 -0000	1.65
+++ mount.c	2 Aug 2003 11:21:13 -0000
@@ -97,6 +97,30 @@
 
 static char ffs_fstype[] = "ffs";
 
+/*
+ * ISFSTYPE() -- Check whether the file system type described by <variable>
+ * matches the file system type described by <macroname> and <string>.
+ *
+ * <macroname> is a the name of a macro such as MOUNT_MFS.
+ * <string> is a string such as "mfs".
+ *
+ * It's expected that <macroname> will be a macro that expands to <string>.
+ * It's not an error if <macroname> is undefined.
+ * It's an error if <macroname> is defined as something other than <string>,
+ * but we don't detect this error.
+ *
+ * ISFSTYPE() ignores the <macroname> and tests only the <string>.
+ *
+ * The unused <macroname> argument is expected to be useful for
+ * cross-referencing and indexing tools.
+ *
+ * We test the <string> instead of the <macroname> because we want to
+ * reduce dependence on specific kernel versions, so we don't want to
+ * use lots of #ifdef:s.
+ */
+#define ISFSTYPE(variable, macroname, string) \
+		(strncmp(variable, string, MFSNAMELEN) == 0)
+
 int
 main(argc, argv)
 	int argc;
@@ -349,13 +373,79 @@
 		}
 		for(i = 0; i < numfs; i++) {
 			/*
-			 * XXX can't check f_mntfromname,
-			 * thanks to mfs, union, etc.
+			 * If the mounted file system described by
+			 * sfp[i] matches the file system to be mounted,
+			 * then this would be a duplicate, so do not
+			 * mount it.
+			 *
+			 * Note that a duplicate does not always have a
+			 * matching f_mntfromname field.  For example:
+			 *
+			 * fstype	original name	f_mntfromname
+			 * ------	-------------	-------------
+			 * mfs		anything	mfs:123
+			 * kernfs	anything	kernfs
+			 * procfs	anything	procfs
+			 * union (with default UNMNT_ABOVE option)
+			 *		/some/dir	<above>:/some/dir
+			 * union (with "-b" UNMNT_BELOW option)
+			 *		/some/dir	<below>:/some/dir
+			 * union (with "-r" UNMNT_REPLACE option)
+			 *		/some/dir	/some/dir
+			 * smbfs	???		//user@host/service
+			 * coda		???		CODA
+			 *
+			 * If f_mntonname and f_fstypename match, then
+			 * we copy f_mntfromname into a local buffer
+			 * (fromname) and possibly modify the copy
+			 * before checking whether it equals the desired
+			 * name.
 			 */
-			if (strncmp(name, sfp[i].f_mntonname, MNAMELEN) == 0 &&
-			    strncmp(vfstype, sfp[i].f_fstypename,
-				MFSNAMELEN) == 0) {
-				if (verbose)
+			char fromname[MNAMELEN];
+			char *check_fromname = fromname;
+
+			if (strncmp(name, sfp[i].f_mntonname, MNAMELEN) != 0
+			    || strncmp(vfstype, sfp[i].f_fstypename,
+					MFSNAMELEN) != 0)
+				continue; /* not a duplicate */
+			strlcpy(fromname, sfp[i].f_mntfromname, MNAMELEN);
+
+			if (ISFSTYPE(vfstype, MOUNT_MFS, "mfs")
+			    || ISFSTYPE(vfstype, MOUNT_PROCFS, "procfs")
+			    || ISFSTYPE(vfstype, MOUNT_KERNFS, "kernfs")
+			    || ISFSTYPE(vfstype, MOUNT_CODA, "coda")
+			    || ISFSTYPE(vfstype, MOUNT_FDESC, "fdsec")) {
+				/* Ignore fromname for these file systems. */
+				check_fromname = NULL;
+			} else if (ISFSTYPE(vfstype, MOUNT_UNION, "union")) {
+				/*
+				 * Focus after the colon in
+				 * "<above>:/some/dir" or
+				 * "<below>:/some/dir".  Note that
+				 * there is no extra <foo> in the
+				 * UNMNT_REPLACE case.
+				 */
+				char *s;
+
+				if (fromname[0] == '<'
+				    && (s = strchr(fromname, ':')) != NULL)
+					check_fromname = s + 1;
+			} else if (ISFSTYPE(vfstype, MOUNT_SMBFS, "smbfs")) {
+				/*
+				 * XXX: smbfs_mount() in
+				 *	sys/fs/smbfs/smbfs_vfsops.c
+				 *	creates f_mntfromname from
+				 *	several components, and I don't
+				 *	know where the components came
+				 *	from.  It's probably safest to
+				 *	ignore fromname.
+				 */
+				 check_fromname = NULL;
+			}
+			if (check_fromname == NULL
+			    || strncmp(spec, check_fromname, MNAMELEN) == 0) {
+				/* It's a duplicate. */
+				if (verbose) {
 					(void)printf("%s on %s type %.*s: "
 					    "%s\n",
 					    sfp[i].f_mntfromname,
@@ -363,6 +453,7 @@
 					    MFSNAMELEN,
 					    sfp[i].f_fstypename,
 					    "already mounted");
+				}
 				return (0);
 			}
 		}
>Release-Note:
>Audit-Trail:
>Unformatted: