Hi Glauber,
On Sun, Aug 14, 2011 at 19:13 +0400, Glauber Costa wrote:
> +/**
> + * Generic option parsing for the VFS.
> + *
> + * Since most of the filesystems already do their own option parsing, and with
> + * very few code shared between them, this function strips out any options that
> + * we succeed in parsing ourselves. Passing them forward would just give the
> + * underlying fs an option it does not expect, leading it to fail.
> + *
> + * We don't yet have a pointer to the super block as well, since this is
> + * pre-mount. We accumulate in struct vfs_options whatever data we collected,
> + * and act on it later.
> + */
> +static int vfs_parse_options(char *options, struct vfs_options *ops)
> +{
> + substring_t args[MAX_OPT_ARGS];
> + int option;
> + char *p;
> + char *opt;
> + char *start = NULL;
> + int ret;
> +
> + if (!options)
> + return 0;
> +
> + opt = kstrdup(options, GFP_KERNEL);
> + if (!opt)
> + return 1;
> +
> + ret = 1;
> +
> + start = opt;
> + while ((p = strsep(&opt, ",")) != NULL) {
> + int token;
> + if (!*p)
> + continue;
> +
> + /*
> + * Initialize args struct so we know whether arg was
> + * found; some options take optional arguments.
> + */
> + args[0].to = args[0].from = 0;
> + token = match_token(p, tokens, args);
> + switch (token) {
> + case 1:
> + if (!args[0].from)
> + break;
> +
> + if (match_int(&args[0], &option))
> + break;
What if there are 2 passed options and the second fails?
mount -o vfs_dcache_size=XXX,vfs_dcache_size=CRAP <dev> <mntpoint>
In this case you leave the second option and pass it to the fs option
parser (as you already set ret=0), which is wrong. I think you should
explicitly return 1 where you know the option is related to VFS, but you
failed to parse it. It would look even simplier than current code.
(Yes, this is a rare situation, but I can imagine some program that
automatically adds mount options to the existing list and passes it to
mount.)
> + if (option < DCACHE_MIN_SIZE) {
> + printk(KERN_INFO "dcache size %d smaller than "
> + "minimum (%d)\n", option, DCACHE_MIN_SIZE);
> + option = DCACHE_MIN_SIZE;
> + }
> +
> + ops->vfs_dcache_size = option;
> +
> + /*
> + * The actual filesystems don't expect any option
> + * they don't understand to be received in the option
> + * string. So we strip off anything we processed, and
> + * give them a clean options string.
> + */
> + ret = 0;
> + if (!opt) /* it is the last option listed */
> + *(options + (p - start)) = '\0';
> + else
> + strcpy(options + (p - start), opt);
> + break;
> + default:
> + ret = 0;
> + break;
> + }
> + }
> +
> + kfree(start);
> + return ret;
> +}
Thanks,
--
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
