svn commit: r186504 - head/sbin/mount

[Christoph Mallon]

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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/svn-src-all
> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"
> 

-------------- next part --------------
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;
 }