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

From: Brad Barden <b+suckless_AT_13os.net>
Date: Thu, 11 Feb 2016 01:05:59 -0600

Sorry for the excessive noise, I'm not paying close enough attention.
That was supposed to read *fsopts = '\0';

On Thu Feb 11, 2016 at 12:41:33AM -0600, Brad Barden wrote:
> 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.
> ---
>
> v2 fix: I missed emptying the fsopts string when mounting multiple (line
> 319).
>
> diff --git a/mount.c b/mount.c
> index 8888c60..0cf70ae 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,13 @@ mountall:
> if (hasmntopt(me, MNTOPT_NOAUTO) || mounted(me->mnt_dir))
> continue;
> flags = 0;
> - parseopts(me->mnt_opts, &flags, data, sizeof(data));
> + fsopts = "";
> + 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 - 08:05:59 CET