public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [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