Date: Thu, 28 Nov 1996 22:54:22 -0800 (PST) From: vax@linkdead.paranoia.com To: freebsd-gnats-submit@freebsd.org Subject: bin/2119: mount lies to child about argv0, which causes crunch to lose Message-ID: <199611290654.WAA01756@freefall.freebsd.org> Resent-Message-ID: <199611290700.XAA01988@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 2119
>Category: bin
>Synopsis: mount lies to child about argv0, which causes crunch to lose
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Nov 28 23:00:01 PST 1996
>Last-Modified:
>Originator: VaX#n8
>Organization:
League of Non-aligned Wizards
>Release: HEAD revisions
>Environment:
I'm using your CVS/WWW interface and notice you have the same problem
as NetBSD.
>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 NetBSD's 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
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:
Patch for NetBSD but should give an idea
--- 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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199611290654.WAA01756>