usb: gadget: FunctionFS: Fix missing braces in parse_opts
PATCH 1/1 usb: gadget: FunctionFS: Fix missing braces in parse_opts
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.
--- drivers/usb/gadget/f_fs.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/usb/gadget/f_fs.c +3 -3
@@ -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 :
Signature · 1 line
1.7.7.3
Quoted 6 lines · 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 commitb9b73f7c(userns: Convert usb functionfs to use kuid/kgid where appropriate) in 3.7.Signed-off-by Benoit Goby <[email protected]>
Quoted 27 lines
--- drivers/usb/gadget/f_fs.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)drivers/usb/gadget/f_fs.c +3
-3@@ -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 :
Signature · 4 lines
Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz (o o) ooo +----<email/xmpp: [email protected]>--------------ooO--(_)--Ooo--
PATCH 1/2 usb: gadget: FunctionFS: Use kstrtoul()
In reply to Benoit GobyFrom: Michal Nazarewicz <[email protected]>
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.
---
Patch on top of v3.5 with Benoit Goby's “Fix missing braces in parse_opts” patch (<id:[email protected]>) applied.
drivers/usb/gadget/f_fs.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-)
drivers/usb/gadget/f_fs.c +2 -3
@@ -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 ; }
Signature · 1 line
1.7.7.3
PATCH 2/2 usb: gadget: FunctionFS: Refactor option parsing
From: Michal Nazarewicz <[email protected]>
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.
--- drivers/usb/gadget/f_fs.c | 55 ++++++++++++++------------------------------ 1 files changed, 18 insertions(+), 37 deletions(-)
drivers/usb/gadget/f_fs.c +18 -37
@@ -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 ; }
Signature · 1 line
1.7.7.3