David O'Brien schrieb:
On Sun, Jan 11, 2009 at 08:56:22AM +0100, Christoph Mallon wrote:
David O'Brien schrieb:
On Sun, Jan 04, 2009 at 07:06:18PM +0100, Christoph Mallon wrote:
I'm pretty sure $SUPERNATURAL_BEING_OF_YOUR_CHOICE killed a kitten for
the ugly hack you added to mount. The moment you overflow a buffer, you
are in no man's land and there's no escape. I appended a patch, which
solves this issue once and for all: The argv array gets dynamically
expanded, when its limit is reached.
Please - for all kittens out there - commit this patch.
Hi Christoph,
Unfortunately your patch doesn't work.
For a 'ufs' file system listed in /etc/fstab
$ umount /foo
$ mount /foo
Does not work.
Why haven't you told me earlier?
Wow, wish I did have the gift to know something before I know something.
Especially tomorrow's lotto #'s. ;-)
I sent you my patch almost a week before you commited your changes. I
think, there was enough time to tell me, that my patch had a flaw.
Alternatively I would have gladly reviewed your solution.
It was a trivial glitch - just a missing
"--mnt_argc;". I would've corrected it right away. I tested with a CD and
this takes a different code path, which does not trigger the problem.
Attached is the corrected patch.
Actually this version doesn't work either.
Trying to mount root from ufs:/dev/ad4s1a
<fsck of all UFS's in /etc/fstab>
..
usage: mount [-t fstype] [-o options] target_fs mount_point
Mounting /etc/fstab filesystems failed, startup aborted
ERROR: ABORTING BOOT (sending SIGTERM to parent)!
Your over use of global variables made my adult cat cry in pain.
Too bad you didn't at least follow the lead of what I committed.
I've committed something in the spirit of your patches. I hope
that suits everyone.
Actually this version doesn't work either.
tron# ./mount -a
usage: mount [-t fstype] [-o options] target_fs mount_point
tron# ./mount -d -a
mount -t ufs -o rw -o update /dev/ad0s1a /
mount -t ufs ��`X (濿��`X /dev/ad0s1f /data
(Sorry for the garbage, it actually printed that. You can turn it into a
"clean" segfault by memset()ing mnt_argv just after its declaration)
Your incorrect use of local variables and the resulting undefined
behaviour made the cat, who visits me on a roof behind the house
sometimes, almost fall from said roof, when he saw your commit: You
expect the local variable "struct cpa mnt_argv" still to have the same
values after mountfs() was left and reentered. Too bad you didn't at
least follow the lead of what I proposed.
I've attached a corrected version of my patch, which has a mnt_argc = 0;
added in order to reset the argument vector on reentry of mountfs()
(instead of appending to the arguments of the last round).
Index: mount.c
===================================================================
--- mount.c (Revision 187093)
+++ mount.c (Arbeitskopie)
@@ -67,18 +67,16 @@
#define MOUNT_META_OPTION_CURRENT "current"
int debug, fstab_style, verbose;
+static char **mnt_argv;
+static int mnt_argv_size;
+static int mnt_argc;
-struct cpa {
- char **a;
- int c;
-};
-
char *catopt(char *, const char *);
struct statfs *getmntpt(const char *);
int hasopt(const char *, const char *);
int ismounted(struct fstab *, struct statfs *, int);
int isremountable(const char *);
-void mangle(char *, struct cpa *);
+static void mangle(char *);
char *update_options(char *, char *, int);
int mountfs(const char *, const char *, const char *,
int, const char *, const char *);
@@ -501,28 +499,24 @@
}
static void
-append_arg(struct cpa *sa, char *arg)
+append_argv(char *arg)
{
- static int a_sz;
-
- if (sa->c + 1 == a_sz) {
- a_sz = a_sz == 0 ? 8 : a_sz * 2;
- sa->a = realloc(sa->a, sizeof(sa->a) * a_sz);
- if (sa->a == NULL)
+ if (mnt_argc == mnt_argv_size) {
+ mnt_argv_size = mnt_argv_size == 0 ? 16 : mnt_argv_size * 2;
+ mnt_argv = realloc(mnt_argv, sizeof(*mnt_argv) * mnt_argv_size);
+ if (mnt_argv == NULL)
errx(1, "realloc failed");
}
- sa->a[++sa->c] = arg;
+ mnt_argv[mnt_argc++] = arg;
}
int
mountfs(const char *vfstype, const char *spec, const char *name, int flags,
const char *options, const char *mntopts)
{
- struct cpa mnt_argv;
struct statfs sf;
int i, ret;
char *optbuf, execname[PATH_MAX], mntpath[PATH_MAX];
- static int mnt_argv_inited;
/* resolve the mountpoint with realpath(3) */
(void)checkpath(name, mntpath);
@@ -557,32 +551,29 @@
/* Construct the name of the appropriate mount command */
(void)snprintf(execname, sizeof(execname), "mount_%s", vfstype);
- if (!mnt_argv_inited) {
- mnt_argv_inited++;
- mnt_argv.a = NULL;
- }
- mnt_argv.c = -1;
- append_arg(&mnt_argv, execname);
- mangle(optbuf, &mnt_argv);
- append_arg(&mnt_argv, strdup(spec));
- append_arg(&mnt_argv, strdup(name));
- append_arg(&mnt_argv, NULL);
+ mnt_argc = 0; /* Reset argument vector */
+ append_argv(execname);
+ mangle(optbuf);
+ append_argv(strdup(spec));
+ append_argv(strdup(name));
+ append_argv(NULL);
+ --mnt_argc; /* Do not count the terminating null pointer */
if (debug) {
if (use_mountprog(vfstype))
printf("exec: mount_%s", vfstype);
else
printf("mount -t %s", vfstype);
- for (i = 1; i < mnt_argv.c; i++)
- (void)printf(" %s", mnt_argv.a[i]);
+ for (i = 1; i < mnt_argc; i++)
+ (void)printf(" %s", mnt_argv[i]);
(void)printf("\n");
return (0);
}
if (use_mountprog(vfstype)) {
- ret = exec_mountprog(name, execname, mnt_argv.a);
+ ret = exec_mountprog(name, execname, mnt_argv);
} else {
- ret = mount_fs(vfstype, mnt_argv.c, mnt_argv.a);
+ ret = mount_fs(vfstype, mnt_argc, mnt_argv);
}
free(optbuf);
@@ -684,8 +675,8 @@
return (cp);
}
-void
-mangle(char *options, struct cpa *a)
+static void
+mangle(char *options)
{
char *p, *s;
@@ -720,15 +711,15 @@
sizeof(groupquotaeq) - 1) == 0) {
continue;
} else if (*p == '-') {
- append_arg(a, p);
+ append_argv(p);
p = strchr(p, '=');
if (p != NULL) {
*p = '\0';
- append_arg(a, p + 1);
+ append_argv(p + 1);
}
} else {
- append_arg(a, strdup("-o"));
- append_arg(a, p);
+ append_argv(strdup("-o"));
+ append_argv(p);
}
}
}
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
