>Number:         2974
>Category:       bin
>Synopsis:       mount lies to child about argv0, which causes crunch to lose
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people (Utility Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Nov 28 23:05:00 1996
>Last-Modified:
>Originator:     VaX#n8
>Organization:
	
League of Non-Aligned Wizards
>Release:        <NetBSD-current source date> 16 Nov 1996 (approx.)
>Environment:
	
System: NetBSD linkdead.paranoia.com 1.2B NetBSD 1.2B (LINKDEAD) #0: Sun Nov 17 10:42:34 CST 1996 vax@linkdead.paranoia.com:/usr/src/sys/arch/i386/compile/LINKDEAD i386


>Description:
	
The program /sbin/mount lies to child about argv[0], saying that it is "ffs"
instead of "mount_ffs".  This is wrong, doesn't really save much work
(if any), and confuses crunched executables, which branch in main() based
on argv[0].  While it might be possible to put in "ffs" as an alias for
"mount_ffs", this pollutes the namespace and conflicts with an identical
problem in fsck which I will submit seperately.

The code calls exit() rather than _exit() in the child if it fails
to execve().  This is wrong.  Quoth vfork(2):
     Be careful, also, to call _exit
     rather than exit if you can't execve,  since exit will flush and close
     standard I/O channels, and thereby mess up the parent processes standard
     I/O data structures.  (Even with fork it is wrong to call exit since
     buffered data would then be flushed twice.)

One of the tests for the debug flag which enabled some output was
supposed to be a test for the verbose flag --- the debug flag
is supposed to prevent execing the sub-program.

Our fsck had a special GNUC thing to protect optbuf from vfork clobbering.
I assume that is since optbuf is a dynamically allocated object.
Since I do not understand it completely, it is possible that it is
not necessary in this chunk of code.
>How-To-Repeat:
	
bash# mount /usr
ffs: /dev/sd1e on /usr: Device busy

That second line is generated by mount_ffs, and the label is the value
of argv[0].

bash# mount -v /mnt
/dev/fd0a on /mnt type ffs (local)
bash# mount -dv /mnt
exec: mount_ffs -o noauto /dev/fd0a /mnt

Whoops! Option "-v" doesn't really give us information on the exec call.
I was considering moving the printf loop into the child but didn't
know what the implications of writing to stdout in the child would be.

bash# cc -O  -Werror -Wall   -c /usr/src/sbin/mount/mount.c
cc1: warnings being treated as errors
/usr/src/sbin/mount/mount.c: In function `main':
/usr/src/sbin/mount/mount.c:246: warning: long int format, pid_t arg (arg 3)
/usr/src/sbin/mount/mount.c: In function `mountfs':
/usr/src/sbin/mount/mount.c:296: warning: variable `optbuf' might be clobbered by `longjmp' or `vfork'
/usr/src/sbin/mount/mount.c:282: warning: argument `name' might be clobbered by `longjmp' or `vfork'
/usr/src/sbin/mount/mount.c: In function `maketypelist':
/usr/src/sbin/mount/mount.c:506: warning: suggest parentheses around assignment used as truth value
/usr/src/sbin/mount/mount.c:511: warning: null format string
/usr/src/sbin/mount/mount.c:513: warning: suggest parentheses around assignment used as truth value
/usr/src/sbin/mount/mount.c: In function `catopt':
/usr/src/sbin/mount/mount.c:532: warning: null format string
/usr/src/sbin/mount/mount.c: At top level:
/usr/src/sbin/mount/mount.c:37: warning: `copyright' defined but not used
/usr/src/sbin/mount/mount.c:46: warning: `rcsid' defined but not used

I did not fix the rcsid and copyright stuff since I figured this would
be something that should be solved globally and consistently.  Other
than that it should compile with -Werror -Wall.

I evaluated the FreeBSD error handling in the edirs/execve loop and
decided I didn't like it -- it never reports the error if the first
exec generates one (e.g. ENOEXEC).

>Fix:
	

--- sbin/mount/mount.c~	Thu Oct 24 06:13:48 1996
+++ sbin/mount/mount.c	Thu Nov 28 23:35:55 1996
@@ -243,7 +243,7 @@
 	 */
 	if (rval == 0 && getuid() == 0 &&
 	    (mountdfp = fopen(_PATH_MOUNTDPID, "r")) != NULL) {
-		if (fscanf(mountdfp, "%ld", &pid) == 1 &&
+		if (fscanf(mountdfp, "%ld", (long *) &pid) == 1 &&
 		     pid > 0 && kill(pid, SIGHUP) == -1 && errno != ESRCH)
 			err(1, "signal mountd");
 		(void)fclose(mountdfp);
@@ -288,12 +288,19 @@
 		_PATH_USRSBIN,
 		NULL
 	};
-	const char *argv[100], **edir;
+	char execbase[MAXPATHLEN] = "mount_";
+	const char *argv[100] = { (const char *)execbase }, **edir;
 	struct statfs sf;
 	pid_t pid;
-	int argc, i, status;
+	int argc = 1, i, status;
 	char *optbuf, execname[MAXPATHLEN + 1], mntpath[MAXPATHLEN];
 
+#ifdef __GNUC__
+        /* Avoid vfork clobbering */
+        (void) &optbuf;
+        (void) &name;
+#endif
+
 	if (realpath(name, mntpath) == NULL) {
 		warn("realpath %s", name);
 		return (1);
@@ -344,16 +351,23 @@
 	if (flags & MNT_UPDATE)
 		optbuf = catopt(optbuf, "update");
 
-	argc = 0;
-	argv[argc++] = vfstype;
+	/* construct basename of executable and argv[0] simultaneously */
+	(void)strncat(execbase,
+		      (const char *)vfstype,
+		      sizeof(execbase) - 6);  /* strlen("mount_") + \0 */
 	mangle(optbuf, &argc, argv);
 	argv[argc++] = spec;
 	argv[argc++] = name;
 	argv[argc] = NULL;
 
-	if (debug) {
-		(void)printf("exec: mount_%s", vfstype);
-		for (i = 1; i < argc; i++)
+	/*
+	 * XXX
+	 * perhaps this belongs just before each exec call in the child
+	 * if you move it there, move the debug-related _exit(0) too
+	 */
+	if (verbose) {
+		(void)printf("exec:");
+		for (i = 0; i < argc; i++)
 			(void)printf(" %s", argv[i]);
 		(void)printf("\n");
 		return (0);
@@ -365,11 +379,14 @@
 		free(optbuf);
 		return (1);
 	case 0:					/* Child. */
+                if (debug)
+                        _exit(0);
+
 		/* Go find an executable. */
 		edir = edirs;
 		do {
 			(void)snprintf(execname,
-			    sizeof(execname), "%s/mount_%s", *edir, vfstype);
+			    sizeof(execname), "%s/%s", *edir, execbase);
 			execv(execname, (char * const *)argv);
 			if (errno != ENOENT)
 				warn("exec %s for %s", execname, name);
@@ -377,7 +394,7 @@
 
 		if (errno == ENOENT)
 			warn("exec %s for %s", execname, name);
-		exit(1);
+		_exit(1);
 		/* NOTREACHED */
 	default:				/* Parent. */
 		free(optbuf);
@@ -487,14 +504,14 @@
 		which = IN_LIST;
 
 	/* Count the number of types. */
-	for (i = 1, nextcp = fslist; nextcp = strchr(nextcp, ','); i++)
+	for (i = 1, nextcp = fslist; (nextcp = strchr(nextcp, ',')); i++)
 		++nextcp;
 
 	/* Build an array of that many types. */
 	if ((av = typelist = malloc((i + 1) * sizeof(char *))) == NULL)
-		err(1, NULL);
+		err(1, "mallocing typelist");
 	av[0] = fslist;
-	for (i = 1, nextcp = fslist; nextcp = strchr(nextcp, ','); i++) {
+	for (i = 1, nextcp = fslist; (nextcp = strchr(nextcp, ',')); i++) {
 		*nextcp = '\0';
 		av[i] = ++nextcp;
 	}
@@ -513,7 +530,7 @@
 	if (s0 && *s0) {
 		i = strlen(s0) + strlen(s1) + 1 + 1;
 		if ((cp = malloc(i)) == NULL)
-			err(1, NULL);
+			err(1, "mallocing options");
 		(void)snprintf(cp, i, "%s,%s", s0, s1);
 	} else
 		cp = strdup(s1);
>Audit-Trail:
>Unformatted: