* [PATCH] usb: gadget: FunctionFS: Fix missing braces in parse_opts @ 2013-01-09 3:57 Benoit Goby 2013-01-09 8:08 ` Michal Nazarewicz 2013-01-09 9:17 ` [PATCH 1/2] usb: gadget: FunctionFS: Use kstrtoul() Michal Nazarewicz 0 siblings, 2 replies; 4+ messages in thread From: Benoit Goby @ 2013-01-09 3:57 UTC (permalink / raw) To: linux-usb, linux-kernel; +Cc: Felipe Balbi, Greg Kroah-Hartman, Benoit Goby Add missing braces around an if block in ffs_fs_parse_opts. This broke parsing the uid/gid mount options and causes mount to fail when using uid/gid. This has been introduced by commit b9b73f7c (userns: Convert usb functionfs to use kuid/kgid where appropriate) in 3.7. Signed-off-by: Benoit Goby <benoit@android.com> --- drivers/usb/gadget/f_fs.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 4a6961c..8c2f251 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -1153,15 +1153,15 @@ static int ffs_fs_parse_opts(struct ffs_sb_fill_data *data, char *opts) pr_err("%s: unmapped value: %lu\n", opts, value); return -EINVAL; } - } - else if (!memcmp(opts, "gid", 3)) + } else if (!memcmp(opts, "gid", 3)) { data->perms.gid = make_kgid(current_user_ns(), value); if (!gid_valid(data->perms.gid)) { pr_err("%s: unmapped value: %lu\n", opts, value); return -EINVAL; } - else + } else { goto invalid; + } break; default: -- 1.7.7.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] usb: gadget: FunctionFS: Fix missing braces in parse_opts 2013-01-09 3:57 [PATCH] usb: gadget: FunctionFS: Fix missing braces in parse_opts Benoit Goby @ 2013-01-09 8:08 ` Michal Nazarewicz 2013-01-09 9:17 ` [PATCH 1/2] usb: gadget: FunctionFS: Use kstrtoul() Michal Nazarewicz 1 sibling, 0 replies; 4+ messages in thread From: Michal Nazarewicz @ 2013-01-09 8:08 UTC (permalink / raw) To: Benoit Goby, linux-usb, linux-kernel Cc: Felipe Balbi, Greg Kroah-Hartman, Benoit Goby [-- Attachment #1: Type: text/plain, Size: 1603 bytes --] On Wed, Jan 09 2013, Benoit Goby <benoit@android.com> wrote: > Add missing braces around an if block in ffs_fs_parse_opts. This broke > parsing the uid/gid mount options and causes mount to fail when using > uid/gid. This has been introduced by commit b9b73f7c (userns: Convert usb > functionfs to use kuid/kgid where appropriate) in 3.7. > > Signed-off-by: Benoit Goby <benoit@android.com> Acked-by: Michal Nazarewicz <mina86@mina86.com> Cc: stable@kernel.org > --- > drivers/usb/gadget/f_fs.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c > index 4a6961c..8c2f251 100644 > --- a/drivers/usb/gadget/f_fs.c > +++ b/drivers/usb/gadget/f_fs.c > @@ -1153,15 +1153,15 @@ static int ffs_fs_parse_opts(struct ffs_sb_fill_data *data, char *opts) > pr_err("%s: unmapped value: %lu\n", opts, value); > return -EINVAL; > } > - } > - else if (!memcmp(opts, "gid", 3)) > + } else if (!memcmp(opts, "gid", 3)) { > data->perms.gid = make_kgid(current_user_ns(), value); > if (!gid_valid(data->perms.gid)) { > pr_err("%s: unmapped value: %lu\n", opts, value); > return -EINVAL; > } > - else > + } else { > goto invalid; > + } > break; > > default: -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz (o o) ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo-- [-- Attachment #2.1: Type: text/plain, Size: 0 bytes --] [-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] usb: gadget: FunctionFS: Use kstrtoul() 2013-01-09 3:57 [PATCH] usb: gadget: FunctionFS: Fix missing braces in parse_opts Benoit Goby 2013-01-09 8:08 ` Michal Nazarewicz @ 2013-01-09 9:17 ` Michal Nazarewicz 2013-01-09 9:17 ` [PATCH 2/2] usb: gadget: FunctionFS: Refactor option parsing Michal Nazarewicz 1 sibling, 1 reply; 4+ messages in thread From: Michal Nazarewicz @ 2013-01-09 9:17 UTC (permalink / raw) To: Felipe Balbi, linux-usb; +Cc: Greg Kroah-Hartman, Benoit Goby, linux-kernel From: Michal Nazarewicz <mina86@mina86.com> kstrtoul() checks for overflow which simple_strtoul() does not pluss it has “*end == 0” check in it as well. As a side effect, a new line character is now accepted, but this should not be an issue. Signed-off-by: Michal Nazarewicz <mina86@mina86.com> --- Patch on top of v3.5 with Benoit Goby's “Fix missing braces in parse_opts” patch (<id:1357703829-26621-1-git-send-email-benoit@android.com>) applied. drivers/usb/gadget/f_fs.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 8c2f251..38388d7 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -1103,8 +1103,8 @@ static int ffs_fs_parse_opts(struct ffs_sb_fill_data *data, char *opts) return 0; for (;;) { - char *end, *eq, *comma; unsigned long value; + char *eq, *comma; /* Option limit */ comma = strchr(opts, ','); @@ -1120,8 +1120,7 @@ static int ffs_fs_parse_opts(struct ffs_sb_fill_data *data, char *opts) *eq = 0; /* Parse value */ - value = simple_strtoul(eq + 1, &end, 0); - if (unlikely(*end != ',' && *end != 0)) { + if (kstrtoul(eq + 1, 0, &value)) { pr_err("%s: invalid value: %s\n", opts, eq + 1); return -EINVAL; } -- 1.7.7.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] usb: gadget: FunctionFS: Refactor option parsing 2013-01-09 9:17 ` [PATCH 1/2] usb: gadget: FunctionFS: Use kstrtoul() Michal Nazarewicz @ 2013-01-09 9:17 ` Michal Nazarewicz 0 siblings, 0 replies; 4+ messages in thread From: Michal Nazarewicz @ 2013-01-09 9:17 UTC (permalink / raw) To: Felipe Balbi, linux-usb; +Cc: Greg Kroah-Hartman, Benoit Goby, linux-kernel From: Michal Nazarewicz <mina86@mina86.com> The use of memcmp() is clever and all and maybe even it makes parsing a bit faster (since only options with given length need to be checked) but option parsing is hardly a critical path and the additional code complexity is not worth it. Signed-off-by: Michal Nazarewicz <mina86@mina86.com> --- drivers/usb/gadget/f_fs.c | 55 ++++++++++++++------------------------------ 1 files changed, 18 insertions(+), 37 deletions(-) diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c index 38388d7..6a7e187 100644 --- a/drivers/usb/gadget/f_fs.c +++ b/drivers/usb/gadget/f_fs.c @@ -1126,45 +1126,26 @@ static int ffs_fs_parse_opts(struct ffs_sb_fill_data *data, char *opts) } /* Interpret option */ - switch (eq - opts) { - case 5: - if (!memcmp(opts, "rmode", 5)) - data->root_mode = (value & 0555) | S_IFDIR; - else if (!memcmp(opts, "fmode", 5)) - data->perms.mode = (value & 0666) | S_IFREG; - else - goto invalid; - break; - - case 4: - if (!memcmp(opts, "mode", 4)) { - data->root_mode = (value & 0555) | S_IFDIR; - data->perms.mode = (value & 0666) | S_IFREG; - } else { - goto invalid; + if (!strcmp(opts, "rmode")) { + data->root_mode = (value & 0555) | S_IFDIR; + } else if (!strcmp(opts, "fmode")) { + data->perms.mode = (value & 0666) | S_IFREG; + } else if (!strcmp(opts, "mode")) { + data->root_mode = (value & 0555) | S_IFDIR; + data->perms.mode = (value & 0666) | S_IFREG; + } else if (!strcmp(opts, "uid")) { + data->perms.uid = make_kuid(current_user_ns(), value); + if (!uid_valid(data->perms.uid)) { + pr_err("%s: unmapped value: %lu\n", opts, value); + return -EINVAL; } - break; - - case 3: - if (!memcmp(opts, "uid", 3)) { - data->perms.uid = make_kuid(current_user_ns(), value); - if (!uid_valid(data->perms.uid)) { - pr_err("%s: unmapped value: %lu\n", opts, value); - return -EINVAL; - } - } else if (!memcmp(opts, "gid", 3)) { - data->perms.gid = make_kgid(current_user_ns(), value); - if (!gid_valid(data->perms.gid)) { - pr_err("%s: unmapped value: %lu\n", opts, value); - return -EINVAL; - } - } else { - goto invalid; + } else if (!strcmp(opts, "gid")) { + data->perms.gid = make_kgid(current_user_ns(), value); + if (!gid_valid(data->perms.gid)) { + pr_err("%s: unmapped value: %lu\n", opts, value); + return -EINVAL; } - break; - - default: -invalid: + } else { pr_err("%s: invalid option\n", opts); return -EINVAL; } -- 1.7.7.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-01-09 9:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-09 3:57 [PATCH] usb: gadget: FunctionFS: Fix missing braces in parse_opts Benoit Goby 2013-01-09 8:08 ` Michal Nazarewicz 2013-01-09 9:17 ` [PATCH 1/2] usb: gadget: FunctionFS: Use kstrtoul() Michal Nazarewicz 2013-01-09 9:17 ` [PATCH 2/2] usb: gadget: FunctionFS: Refactor option parsing Michal Nazarewicz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox