Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 11 Jan 2009 08:56:22 +0100
From:      Christoph Mallon <christoph.mallon@gmx.de>
To:        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:  <4969A626.6070908@gmx.de>
In-Reply-To: <20090111041543.GB17602@dragon.NUXI.org>
References:  <200812262254.mBQMsrbR052676@svn.freebsd.org>	<4960FA9A.1090509@gmx.de> <20090111041543.GB17602@dragon.NUXI.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------070304040904040408080004
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

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? 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.

--------------070304040904040408080004
Content-Type: text/plain;
 name="mount.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="mount.diff"

Index: mount.c
===================================================================
--- mount.c	(Revision 187044)
+++ mount.c	(Arbeitskopie)
@@ -69,20 +69,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;
 
-#define	MAX_ARGS 100
-struct cpa {
-	char	*a[MAX_ARGS];
-	ssize_t	m;
-	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 *);
@@ -505,19 +501,21 @@
 }
 
 static void
-append_arg(struct cpa *sa, char *arg)
+append_argv(char *arg)
 {
-	if (sa->c >= sa->m)
-		errx(1, "Cannot process more than %zd mount arguments", sa->m);
-
-	sa->a[++sa->c] = 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)
 {
-	struct cpa mnt_argv;
 	struct statfs sf;
 	int i, ret;
 	char *optbuf, execname[PATH_MAX], mntpath[PATH_MAX];
@@ -555,29 +553,28 @@
 	/* Construct the name of the appropriate mount command */
 	(void)snprintf(execname, sizeof(execname), "mount_%s", vfstype);
 
-	mnt_argv.m = MAX_ARGS;
-	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);
+	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);
@@ -679,8 +676,8 @@
 	return (cp);
 }
 
-void
-mangle(char *options, struct cpa *a)
+static void
+mangle(char *options)
 {
 	char *p, *s;
 
@@ -715,15 +712,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);
 			}
 		}
 }

--------------070304040904040408080004--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4969A626.6070908>