[hackers] [PATCH] [sbase] mount: fix mount helper fs option handling

From: Brad Barden <b+suckless_AT_13os.net>
Date: Wed, 10 Feb 2016 23:46:02 -0600 (CST)

If a mount helper is used, only argopts (given on commandline) are
passed to the helper via -o parameter. Option strings from fstab are
ignored.

This patch replaces global argopts pointer with a character array
fsopts. A maximum length for filesytem options is #defined (used by
fsopts and data), and argument/mntent options are length-checked to fit.
A filesystem with too long an option string will print an error, cause
mount to exit with an error status, and not attempt to mount the
filesystem. This obviates the need for overflow checking of data in
parseopts(), though it hasn't been removed.

---
Notes:
    Choosing 512 as maximum length for filesystem options was somewhat
    arbitrary. It was already the maximum size of data passed to mount(). 256 is
    used as a line buffer size for mntent (that's musl's default for getmntent,
    too, while glibc looks to use 4096). I think 512 is "long enough" without going
    overboard.
    
    oflag was removed in favor of just checking fsopts for empty string.
 mount.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/mount.c b/mount.c
index 8888c60..8a3332b 100644
--- a/mount.c
+++ b/mount.c
_AT_@ -15,6 +15,8 @@
 #include "text.h"
 #include "util.h"
 
+#define FSOPTS_MAXLEN 512
+
 struct {
 	const char *opt;
 	const char *notopt;
_AT_@ -37,7 +39,7 @@ struct {
 };
 
 static unsigned long argflags = 0;
-static char *argopts = NULL;
+static char fsopts[FSOPTS_MAXLEN] = "";
 
 static char *
 findtype(const char *types, const char *t)
_AT_@ -121,9 +123,9 @@ mounthelper(const char *fsname, const char *dir, const char *fstype)
 		if (argflags & MS_REC)
 			eargv[i++] = "-R";
 
-		if (argopts) {
+		if (*fsopts) {
 			eargv[i++] = "-o";
-			eargv[i++] = argopts;
+			eargv[i++] = fsopts;
 		}
 		eargv[i++] = fsname;
 		eargv[i++] = dir;
_AT_@ -188,11 +190,11 @@ usage(void)
 int
 main(int argc, char *argv[])
 {
-	char *types = NULL, data[512] = "", *resolvpath = NULL;
+	char *types = NULL, data[FSOPTS_MAXLEN] = "", *resolvpath = NULL;
 	char *files[] = { "/proc/mounts", "/etc/fstab", NULL };
 	const char *source, *target;
 	struct mntent *me = NULL;
-	int aflag = 0, oflag = 0, status = 0, i, r;
+	int aflag = 0, status = 0, i, r;
 	unsigned long flags = 0;
 	FILE *fp;
 
_AT_@ -210,9 +212,9 @@ main(int argc, char *argv[])
 		aflag = 1;
 		break;
 	case 'o':
-		oflag = 1;
-		argopts = EARGF(usage());
-		parseopts(argopts, &flags, data, sizeof(data));
+		if (strlcat(fsopts, EARGF(usage()), sizeof(fsopts)) > sizeof(fsopts))
+			eprintf("option string too long\n");
+		parseopts(fsopts, &flags, data, sizeof(data));
 		break;
 	case 't':
 		types = EARGF(usage());
_AT_@ -260,8 +262,10 @@ main(int argc, char *argv[])
 					target = me->mnt_dir;
 					source = me->mnt_fsname;
 				}
-				if (!oflag)
-					parseopts(me->mnt_opts, &flags, data, sizeof(data));
+				if (!*fsopts)
+					if (strlcat(fsopts, me->mnt_opts, sizeof(fsopts)) > sizeof(fsopts))
+						eprintf("%s: option string too long\n", target);
+					parseopts(fsopts, &flags, data, sizeof(data));
 				if (!types)
 					types = me->mnt_type;
 				goto mountsingle;
_AT_@ -294,7 +298,12 @@ mountall:
 		if (hasmntopt(me, MNTOPT_NOAUTO) || mounted(me->mnt_dir))
 			continue;
 		flags = 0;
-		parseopts(me->mnt_opts, &flags, data, sizeof(data));
+		if (strlcat(fsopts, me->mnt_opts, sizeof(fsopts)) > sizeof(fsopts)) {
+			weprintf("%s: option string too long\n", me->mnt_dir);
+			status = 1;
+			continue;
+		}
+		parseopts(fsopts, &flags, data, sizeof(data));
 		/* if -t types specified:
 		 * if non-match, skip
 		 * if match and prefixed with "no", skip */
-- 
2.3.6

Received on Thu Feb 11 2016 - 06:46:02 CET