Date: Sun, 04 Jan 2009 19:06:18 +0100 From: Christoph Mallon <christoph.mallon@gmx.de> To: "David E. O'Brien" <obrien@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r186504 - head/sbin/mount Message-ID: <4960FA9A.1090509@gmx.de> In-Reply-To: <200812262254.mBQMsrbR052676@svn.freebsd.org> References: <200812262254.mBQMsrbR052676@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] Hi David, 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. Christoph David E. O'Brien schrieb: > Author: obrien > Date: Fri Dec 26 22:54:53 2008 > New Revision: 186504 > URL: http://svn.freebsd.org/changeset/base/186504 > > Log: > Make the sub-'argc' static to make it harder to overwrite thru a buffer > overflow. > > Modified: > head/sbin/mount/mount.c > > Modified: head/sbin/mount/mount.c > ============================================================================== > --- head/sbin/mount/mount.c Fri Dec 26 22:47:11 2008 (r186503) > +++ head/sbin/mount/mount.c Fri Dec 26 22:54:53 2008 (r186504) > @@ -503,9 +503,10 @@ int > mountfs(const char *vfstype, const char *spec, const char *name, int flags, > const char *options, const char *mntopts) > { > + static int argc; > char *argv[MAX_ARGS]; > struct statfs sf; > - int argc, i, ret; > + int i, ret; > char *optbuf, execname[PATH_MAX], mntpath[PATH_MAX]; > > /* resolve the mountpoint with realpath(3) */ > _______________________________________________ > 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-unsubscribe@freebsd.org" > [-- Attachment #2 --] Index: mount.c =================================================================== --- mount.c (Revision 186740) +++ mount.c (Arbeitskopie) @@ -68,16 +68,17 @@ #define MOUNT_META_OPTION_FSTAB "fstab" #define MOUNT_META_OPTION_CURRENT "current" -#define MAX_ARGS 100 - int debug, fstab_style, verbose; +static char **mnt_argv; +static int mnt_argv_size; +static int mnt_argc; 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 *, int *, char *[]); +static void mangle(char *); char *update_options(char *, char *, int); int mountfs(const char *, const char *, const char *, int, const char *, const char *); @@ -499,12 +500,22 @@ return (found); } +static void +append_argv(char *arg) +{ + 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"); + } + mnt_argv[mnt_argc++] = arg; +} + int mountfs(const char *vfstype, const char *spec, const char *name, int flags, const char *options, const char *mntopts) { - static int argc; - char *argv[MAX_ARGS]; struct statfs sf; int i, ret; char *optbuf, execname[PATH_MAX], mntpath[PATH_MAX]; @@ -542,32 +553,27 @@ /* Construct the name of the appropriate mount command */ (void)snprintf(execname, sizeof(execname), "mount_%s", vfstype); - argc = 0; - argv[argc++] = execname; - mangle(optbuf, &argc, argv); - argv[argc++] = strdup(spec); - argv[argc++] = strdup(name); - argv[argc] = NULL; + append_argv(execname); + mangle(optbuf); + append_argv(strdup(spec)); + append_argv(strdup(name)); + append_argv(NULL); - if (MAX_ARGS <= argc ) - errx(1, "Cannot process more than %d mount arguments", - MAX_ARGS); - if (debug) { if (use_mountprog(vfstype)) printf("exec: mount_%s", vfstype); else printf("mount -t %s", vfstype); - for (i = 1; i < argc; i++) - (void)printf(" %s", argv[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, argv); + ret = exec_mountprog(name, execname, mnt_argv); } else { - ret = mount_fs(vfstype, argc, argv); + ret = mount_fs(vfstype, mnt_argc, mnt_argv); } free(optbuf); @@ -669,13 +675,11 @@ return (cp); } -void -mangle(char *options, int *argcp, char *argv[]) +static void +mangle(char *options) { char *p, *s; - int argc; - argc = *argcp; for (s = options; (p = strsep(&s, ",")) != NULL;) if (*p != '\0') { if (strcmp(p, "noauto") == 0) { @@ -707,19 +711,17 @@ sizeof(groupquotaeq) - 1) == 0) { continue; } else if (*p == '-') { - argv[argc++] = p; + append_argv(p); p = strchr(p, '='); if (p != NULL) { *p = '\0'; - argv[argc++] = p+1; + append_argv(p + 1); } } else { - argv[argc++] = strdup("-o"); - argv[argc++] = p; + append_argv(strdup("-o")); + append_argv(p); } } - - *argcp = argc; }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4960FA9A.1090509>