All of lore.kernel.org
 help / color / mirror / Atom feed
* 3.4.0-rc2+ - CIFS mount failure
@ 2012-04-10  7:23 Chris Clayton
       [not found] ` <201204100823.24207.chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Clayton @ 2012-04-10  7:23 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA

HI,

I'm testing the latest and greatest from Linus' tree and can't mount partitions 
on a nas server. The mounts work fine with 3.3.1. For the failing kernel, git 
describe gives v3.4-rc2-2-g258f742.

What I see is:

[chris:~]$ sudo mount /media/nas/share
mount error(22): Invalid argument
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)

Dmesg shows the message:

CIFS: Unknown mount option "user="

The related entry in /etc/fstab is:

//nas/Share /media/nas/share cifs 
rw,noauto,nosuid,nodev,user,guest,uid=1000,gid=100,file_mode=0666  0 0

Let me know how I can help fix this regression.

Please cc me - I'm not subscribed.

Thanks

Chris
-- 
The more I see, the more I know. The more I know, the less I understand. 
Changing Man - Paul Weller

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: 3.4.0-rc2+ - CIFS mount failure
       [not found] ` <201204100823.24207.chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
@ 2012-04-10  9:39   ` Sachin Prabhu
  2012-04-10 11:16     ` Jeff Layton
  0 siblings, 1 reply; 10+ messages in thread
From: Sachin Prabhu @ 2012-04-10  9:39 UTC (permalink / raw)
  To: chris2553-gM/Ye1E23mwN+BqQ9rBEUg; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 2012-04-10 at 08:23 +0100, Chris Clayton wrote:
> HI,
> 
> I'm testing the latest and greatest from Linus' tree and can't mount partitions 
> on a nas server. The mounts work fine with 3.3.1. For the failing kernel, git 
> describe gives v3.4-rc2-2-g258f742.
> 
> What I see is:
> 
> [chris:~]$ sudo mount /media/nas/share
> mount error(22): Invalid argument
> Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)
> 
> Dmesg shows the message:
> 
> CIFS: Unknown mount option "user="
> 
> The related entry in /etc/fstab is:
> 
> //nas/Share /media/nas/share cifs 
> rw,noauto,nosuid,nodev,user,guest,uid=1000,gid=100,file_mode=0666  0 0
> 
> Let me know how I can help fix this regression.
> 
> Please cc me - I'm not subscribed.
> 
> Thanks
> 
> Chris


Chris,

This was caused due to a regression introduced by the patch with commit
8830d7e07a5e38bc47650a7554b7c1cfd49902bf
which cleaned up the mount options parser.

The patch doesn't handle blank user= mount option passed as a mount
option. The patch is trivial and requires similar handling as the blank
password option.
http://article.gmane.org/gmane.linux.kernel.cifs/5762

Sachin Prabhu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: 3.4.0-rc2+ - CIFS mount failure
  2012-04-10  9:39   ` Sachin Prabhu
@ 2012-04-10 11:16     ` Jeff Layton
       [not found]       ` <20120410071630.567d70cb-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Jeff Layton @ 2012-04-10 11:16 UTC (permalink / raw)
  To: Sachin Prabhu, chris2553-gM/Ye1E23mwN+BqQ9rBEUg
  Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 10 Apr 2012 10:39:32 +0100
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> On Tue, 2012-04-10 at 08:23 +0100, Chris Clayton wrote:
> > HI,
> > 
> > I'm testing the latest and greatest from Linus' tree and can't mount partitions 
> > on a nas server. The mounts work fine with 3.3.1. For the failing kernel, git 
> > describe gives v3.4-rc2-2-g258f742.
> > 
> > What I see is:
> > 
> > [chris:~]$ sudo mount /media/nas/share
> > mount error(22): Invalid argument
> > Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)
> > 
> > Dmesg shows the message:
> > 
> > CIFS: Unknown mount option "user="
> > 
> > The related entry in /etc/fstab is:
> > 
> > //nas/Share /media/nas/share cifs 
> > rw,noauto,nosuid,nodev,user,guest,uid=1000,gid=100,file_mode=0666  0 0
> > 
> > Let me know how I can help fix this regression.
> > 
> > Please cc me - I'm not subscribed.
> > 
> > Thanks
> > 
> > Chris
> 
> 
> Chris,
> 
> This was caused due to a regression introduced by the patch with commit
> 8830d7e07a5e38bc47650a7554b7c1cfd49902bf
> which cleaned up the mount options parser.
> 
> The patch doesn't handle blank user= mount option passed as a mount
> option. The patch is trivial and requires similar handling as the blank
> password option.
> http://article.gmane.org/gmane.linux.kernel.cifs/5762
> 
> Sachin Prabhu
> 

Sachin's working on a patch to fix this. In the meantime you can
replace the "guest" option with "sec=none" and it should work as
expected.

-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: 3.4.0-rc2+ - CIFS mount failure
       [not found]       ` <20120410071630.567d70cb-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
@ 2012-04-10 13:13         ` Chris Clayton
       [not found]           ` <201204101413.27748.chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Clayton @ 2012-04-10 13:13 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Sachin Prabhu, linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tuesday 10 April 2012 12:16:30 Jeff Layton wrote:
> On Tue, 10 Apr 2012 10:39:32 +0100
>
> Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Tue, 2012-04-10 at 08:23 +0100, Chris Clayton wrote:
> > > HI,
> > >
> > > I'm testing the latest and greatest from Linus' tree and can't mount
> > > partitions on a nas server. The mounts work fine with 3.3.1. For the
> > > failing kernel, git describe gives v3.4-rc2-2-g258f742.
> > >
> > > What I see is:
> > >
> > > [chris:~]$ sudo mount /media/nas/share
> > > mount error(22): Invalid argument
> > > Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)
> > >
> > > Dmesg shows the message:
> > >
> > > CIFS: Unknown mount option "user="
> > >
> > > The related entry in /etc/fstab is:
> > >
> > > //nas/Share /media/nas/share cifs
> > > rw,noauto,nosuid,nodev,user,guest,uid=1000,gid=100,file_mode=0666  0 0
> > >
> > > Let me know how I can help fix this regression.
> > >
> > > Please cc me - I'm not subscribed.
> > >
> > > Thanks
> > >
> > > Chris
> >
> > Chris,
> >
> > This was caused due to a regression introduced by the patch with commit
> > 8830d7e07a5e38bc47650a7554b7c1cfd49902bf
> > which cleaned up the mount options parser.
> >
> > The patch doesn't handle blank user= mount option passed as a mount
> > option. The patch is trivial and requires similar handling as the blank
> > password option.
> > http://article.gmane.org/gmane.linux.kernel.cifs/5762
> >
> > Sachin Prabhu
>
> Sachin's working on a patch to fix this. In the meantime you can
> replace the "guest" option with "sec=none" and it should work as
> expected.

If it helps, here's the patch that I cooked up before I received Jeff's advice. I hadn't finished 
making sure it is a good patch, but my first attempt at a mount did seem to work.

Signed-off-by: Chris Clayton <chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

--- linux/fs/cifs/connect.c~    2012-04-10 11:38:23.000000000 +0100
+++ linux/fs/cifs/connect.c     2012-04-10 12:16:02.000000000 +0100
@@ -110,6 +110,9 @@ enum {
        /* Options which could be blank */
        Opt_blank_pass,

+       /* Usernames which could be blank */
+       Opt_blank_user,
+
        Opt_err
 };

@@ -183,6 +186,7 @@ static const match_table_t cifs_mount_op
        { Opt_wsize, "wsize=%s" },
        { Opt_actimeo, "actimeo=%s" },

+       { Opt_blank_user, "user=" },
        { Opt_user, "user=%s" },
        { Opt_user, "username=%s" },
        { Opt_blank_pass, "pass=" },
@@ -1534,6 +1538,10 @@ cifs_parse_mount_options(const char *mou

                /* String Arguments */

+               case Opt_blank_user:
+                       vol->username = NULL;
+                       vol->nullauth = 1;
+                       break;
                case Opt_user:
                        string = match_strdup(args);
                        if (string == NULL)

-- 
The more I see, the more I know. The more I know, the less I understand. Changing Man - Paul Weller

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: 3.4.0-rc2+ - CIFS mount failure
       [not found]           ` <201204101413.27748.chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
@ 2012-04-10 13:28             ` Sachin Prabhu
  2012-04-10 17:12               ` [PATCH] Cleanup handling of NULL value passed for a mount option Sachin Prabhu
  0 siblings, 1 reply; 10+ messages in thread
From: Sachin Prabhu @ 2012-04-10 13:28 UTC (permalink / raw)
  To: Chris Clayton; +Cc: Jeff Layton, linux-cifs-u79uwXL29TY76Z2rM5mHXA


> Signed-off-by: Chris Clayton <chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> 
> --- linux/fs/cifs/connect.c~    2012-04-10 11:38:23.000000000 +0100
> +++ linux/fs/cifs/connect.c     2012-04-10 12:16:02.000000000 +0100
> @@ -110,6 +110,9 @@ enum {
>         /* Options which could be blank */
>         Opt_blank_pass,
> 
> +       /* Usernames which could be blank */
> +       Opt_blank_user,
> +
>         Opt_err
>  };
> 
> @@ -183,6 +186,7 @@ static const match_table_t cifs_mount_op
>         { Opt_wsize, "wsize=%s" },
>         { Opt_actimeo, "actimeo=%s" },
> 
> +       { Opt_blank_user, "user=" },
>         { Opt_user, "user=%s" },
>         { Opt_user, "username=%s" },
>         { Opt_blank_pass, "pass=" },
> @@ -1534,6 +1538,10 @@ cifs_parse_mount_options(const char *mou
> 
>                 /* String Arguments */
> 
> +               case Opt_blank_user:
> +                       vol->username = NULL;
> +                       vol->nullauth = 1;
> +                       break;
>                 case Opt_user:
>                         string = match_strdup(args);
>                         if (string == NULL)
> 

Hello Chris,

The approach is correct. However I was looking to fix up all such
instances where a NULL mount option could be passed.  

There are a number of cases where a check for a NULL string is made. The
token parser however will never match mount options where a NULL is
passed so these checks for NULL strings are redundant. I plan on
cleaning these up too with the new patch.

Sachin Prabhu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] Cleanup handling of NULL value passed for a mount option
  2012-04-10 13:28             ` Sachin Prabhu
@ 2012-04-10 17:12               ` Sachin Prabhu
  2012-04-10 17:17                 ` Jeff Layton
                                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Sachin Prabhu @ 2012-04-10 17:12 UTC (permalink / raw)
  To: Linux CIFS mailing list; +Cc: Jeff Layton, Chris Clayton

Allow blank user= and ip= mount option. Also clean up redundant
checks for NULL values since the token parser will not actually
match mount options with NULL values unless explicitly specified.

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reported-by: Chris Clayton <chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

---
 fs/cifs/connect.c |   80 ++++++++++++----------------------------------------
 1 files changed, 19 insertions(+), 61 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index d81e933..6a86f3d 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -109,6 +109,8 @@ enum {
 
 	/* Options which could be blank */
 	Opt_blank_pass,
+	Opt_blank_user,
+	Opt_blank_ip,
 
 	Opt_err
 };
@@ -183,11 +185,15 @@ static const match_table_t cifs_mount_option_tokens = {
 	{ Opt_wsize, "wsize=%s" },
 	{ Opt_actimeo, "actimeo=%s" },
 
+	{ Opt_blank_user, "user=" },
+	{ Opt_blank_user, "username=" },
 	{ Opt_user, "user=%s" },
 	{ Opt_user, "username=%s" },
 	{ Opt_blank_pass, "pass=" },
 	{ Opt_pass, "pass=%s" },
 	{ Opt_pass, "password=%s" },
+	{ Opt_blank_ip, "ip=" },
+	{ Opt_blank_ip, "addr=" },
 	{ Opt_ip, "ip=%s" },
 	{ Opt_ip, "addr=%s" },
 	{ Opt_unc, "unc=%s" },
@@ -1534,15 +1540,17 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 
 		/* String Arguments */
 
+		case Opt_blank_user:
+			/* null user, ie. anonymous authentication */
+			vol->nullauth = 1;
+			vol->username = NULL;
+			break;
 		case Opt_user:
 			string = match_strdup(args);
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				/* null user, ie. anonymous authentication */
-				vol->nullauth = 1;
-			} else if (strnlen(string, MAX_USERNAME_SIZE) >
+			if (strnlen(string, MAX_USERNAME_SIZE) >
 							MAX_USERNAME_SIZE) {
 				printk(KERN_WARNING "CIFS: username too long\n");
 				goto cifs_parse_mount_err;
@@ -1611,14 +1619,15 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			}
 			vol->password[j] = '\0';
 			break;
+		case Opt_blank_ip:
+			vol->UNCip = NULL;
+			break;
 		case Opt_ip:
 			string = match_strdup(args);
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				vol->UNCip = NULL;
-			} else if (strnlen(string, INET6_ADDRSTRLEN) >
+			if (strnlen(string, INET6_ADDRSTRLEN) >
 						INET6_ADDRSTRLEN) {
 				printk(KERN_WARNING "CIFS: ip address "
 						    "too long\n");
@@ -1636,12 +1645,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				printk(KERN_WARNING "CIFS: invalid path to "
-						    "network resource\n");
-				goto cifs_parse_mount_err;
-			}
-
 			temp_len = strnlen(string, 300);
 			if (temp_len  == 300) {
 				printk(KERN_WARNING "CIFS: UNC name too long\n");
@@ -1670,11 +1673,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				printk(KERN_WARNING "CIFS: invalid domain"
-						    " name\n");
-				goto cifs_parse_mount_err;
-			} else if (strnlen(string, 256) == 256) {
+			if (strnlen(string, 256) == 256) {
 				printk(KERN_WARNING "CIFS: domain name too"
 						    " long\n");
 				goto cifs_parse_mount_err;
@@ -1693,11 +1692,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				printk(KERN_WARNING "CIFS: srcaddr value not"
-						    " specified\n");
-				goto cifs_parse_mount_err;
-			} else if (!cifs_convert_address(
+			if (!cifs_convert_address(
 					(struct sockaddr *)&vol->srcaddr,
 					string, strlen(string))) {
 				printk(KERN_WARNING "CIFS:  Could not parse"
@@ -1710,11 +1705,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				printk(KERN_WARNING "CIFS: Invalid path"
-						    " prefix\n");
-				goto cifs_parse_mount_err;
-			}
 			temp_len = strnlen(string, 1024);
 			if (string[0] != '/')
 				temp_len++; /* missing leading slash */
@@ -1742,11 +1732,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				printk(KERN_WARNING "CIFS: Invalid iocharset"
-						    " specified\n");
-				goto cifs_parse_mount_err;
-			} else if (strnlen(string, 1024) >= 65) {
+			if (strnlen(string, 1024) >= 65) {
 				printk(KERN_WARNING "CIFS: iocharset name "
 						    "too long.\n");
 				goto cifs_parse_mount_err;
@@ -1771,11 +1757,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				printk(KERN_WARNING "CIFS: No socket option"
-						    " specified\n");
-				goto cifs_parse_mount_err;
-			}
 			if (strnicmp(string, "TCP_NODELAY", 11) == 0)
 				vol->sockopt_tcp_nodelay = 1;
 			break;
@@ -1784,12 +1765,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				printk(KERN_WARNING "CIFS: Invalid (empty)"
-						    " netbiosname\n");
-				break;
-			}
-
 			memset(vol->source_rfc1001_name, 0x20,
 				RFC1001_NAME_LEN);
 			/*
@@ -1817,11 +1792,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				printk(KERN_WARNING "CIFS: Empty server"
-					" netbiosname specified\n");
-				break;
-			}
 			/* last byte, type, is 0x20 for servr type */
 			memset(vol->target_rfc1001_name, 0x20,
 				RFC1001_NAME_LEN_WITH_NULL);
@@ -1848,12 +1818,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				cERROR(1, "no protocol version specified"
-					  " after vers= mount option");
-				goto cifs_parse_mount_err;
-			}
-
 			if (strnicmp(string, "cifs", 4) == 0 ||
 			    strnicmp(string, "1", 1) == 0) {
 				/* This is the default */
@@ -1868,12 +1832,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
 			if (string == NULL)
 				goto out_nomem;
 
-			if (!*string) {
-				printk(KERN_WARNING "CIFS: no security flavor"
-						    " specified\n");
-				break;
-			}
-
 			if (cifs_parse_security_flavors(string, vol) != 0)
 				goto cifs_parse_mount_err;
 			break;
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Cleanup handling of NULL value passed for a mount option
  2012-04-10 17:12               ` [PATCH] Cleanup handling of NULL value passed for a mount option Sachin Prabhu
@ 2012-04-10 17:17                 ` Jeff Layton
  2012-04-10 22:00                 ` Chris Clayton
  2012-04-11  2:17                 ` Steve French
  2 siblings, 0 replies; 10+ messages in thread
From: Jeff Layton @ 2012-04-10 17:17 UTC (permalink / raw)
  To: Sachin Prabhu; +Cc: Linux CIFS mailing list, Chris Clayton

On Tue, 10 Apr 2012 18:12:27 +0100
Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Allow blank user= and ip= mount option. Also clean up redundant
> checks for NULL values since the token parser will not actually
> match mount options with NULL values unless explicitly specified.
> 
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Reported-by: Chris Clayton <chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> 
> ---
>  fs/cifs/connect.c |   80 ++++++++++++----------------------------------------
>  1 files changed, 19 insertions(+), 61 deletions(-)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index d81e933..6a86f3d 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -109,6 +109,8 @@ enum {
>  
>  	/* Options which could be blank */
>  	Opt_blank_pass,
> +	Opt_blank_user,
> +	Opt_blank_ip,
>  
>  	Opt_err
>  };
> @@ -183,11 +185,15 @@ static const match_table_t cifs_mount_option_tokens = {
>  	{ Opt_wsize, "wsize=%s" },
>  	{ Opt_actimeo, "actimeo=%s" },
>  
> +	{ Opt_blank_user, "user=" },
> +	{ Opt_blank_user, "username=" },
>  	{ Opt_user, "user=%s" },
>  	{ Opt_user, "username=%s" },
>  	{ Opt_blank_pass, "pass=" },
>  	{ Opt_pass, "pass=%s" },
>  	{ Opt_pass, "password=%s" },
> +	{ Opt_blank_ip, "ip=" },
> +	{ Opt_blank_ip, "addr=" },
>  	{ Opt_ip, "ip=%s" },
>  	{ Opt_ip, "addr=%s" },
>  	{ Opt_unc, "unc=%s" },
> @@ -1534,15 +1540,17 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  
>  		/* String Arguments */
>  
> +		case Opt_blank_user:
> +			/* null user, ie. anonymous authentication */
> +			vol->nullauth = 1;
> +			vol->username = NULL;
> +			break;
>  		case Opt_user:
>  			string = match_strdup(args);
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				/* null user, ie. anonymous authentication */
> -				vol->nullauth = 1;
> -			} else if (strnlen(string, MAX_USERNAME_SIZE) >
> +			if (strnlen(string, MAX_USERNAME_SIZE) >
>  							MAX_USERNAME_SIZE) {
>  				printk(KERN_WARNING "CIFS: username too long\n");
>  				goto cifs_parse_mount_err;
> @@ -1611,14 +1619,15 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			}
>  			vol->password[j] = '\0';
>  			break;
> +		case Opt_blank_ip:
> +			vol->UNCip = NULL;
> +			break;
>  		case Opt_ip:
>  			string = match_strdup(args);
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				vol->UNCip = NULL;
> -			} else if (strnlen(string, INET6_ADDRSTRLEN) >
> +			if (strnlen(string, INET6_ADDRSTRLEN) >
>  						INET6_ADDRSTRLEN) {
>  				printk(KERN_WARNING "CIFS: ip address "
>  						    "too long\n");
> @@ -1636,12 +1645,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: invalid path to "
> -						    "network resource\n");
> -				goto cifs_parse_mount_err;
> -			}
> -
>  			temp_len = strnlen(string, 300);
>  			if (temp_len  == 300) {
>  				printk(KERN_WARNING "CIFS: UNC name too long\n");
> @@ -1670,11 +1673,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: invalid domain"
> -						    " name\n");
> -				goto cifs_parse_mount_err;
> -			} else if (strnlen(string, 256) == 256) {
> +			if (strnlen(string, 256) == 256) {
>  				printk(KERN_WARNING "CIFS: domain name too"
>  						    " long\n");
>  				goto cifs_parse_mount_err;
> @@ -1693,11 +1692,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: srcaddr value not"
> -						    " specified\n");
> -				goto cifs_parse_mount_err;
> -			} else if (!cifs_convert_address(
> +			if (!cifs_convert_address(
>  					(struct sockaddr *)&vol->srcaddr,
>  					string, strlen(string))) {
>  				printk(KERN_WARNING "CIFS:  Could not parse"
> @@ -1710,11 +1705,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: Invalid path"
> -						    " prefix\n");
> -				goto cifs_parse_mount_err;
> -			}
>  			temp_len = strnlen(string, 1024);
>  			if (string[0] != '/')
>  				temp_len++; /* missing leading slash */
> @@ -1742,11 +1732,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: Invalid iocharset"
> -						    " specified\n");
> -				goto cifs_parse_mount_err;
> -			} else if (strnlen(string, 1024) >= 65) {
> +			if (strnlen(string, 1024) >= 65) {
>  				printk(KERN_WARNING "CIFS: iocharset name "
>  						    "too long.\n");
>  				goto cifs_parse_mount_err;
> @@ -1771,11 +1757,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: No socket option"
> -						    " specified\n");
> -				goto cifs_parse_mount_err;
> -			}
>  			if (strnicmp(string, "TCP_NODELAY", 11) == 0)
>  				vol->sockopt_tcp_nodelay = 1;
>  			break;
> @@ -1784,12 +1765,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: Invalid (empty)"
> -						    " netbiosname\n");
> -				break;
> -			}
> -
>  			memset(vol->source_rfc1001_name, 0x20,
>  				RFC1001_NAME_LEN);
>  			/*
> @@ -1817,11 +1792,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: Empty server"
> -					" netbiosname specified\n");
> -				break;
> -			}
>  			/* last byte, type, is 0x20 for servr type */
>  			memset(vol->target_rfc1001_name, 0x20,
>  				RFC1001_NAME_LEN_WITH_NULL);
> @@ -1848,12 +1818,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				cERROR(1, "no protocol version specified"
> -					  " after vers= mount option");
> -				goto cifs_parse_mount_err;
> -			}
> -
>  			if (strnicmp(string, "cifs", 4) == 0 ||
>  			    strnicmp(string, "1", 1) == 0) {
>  				/* This is the default */
> @@ -1868,12 +1832,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: no security flavor"
> -						    " specified\n");
> -				break;
> -			}
> -
>  			if (cifs_parse_security_flavors(string, vol) != 0)
>  				goto cifs_parse_mount_err;
>  			break;

Looks good.

Acked-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Cleanup handling of NULL value passed for a mount option
  2012-04-10 17:12               ` [PATCH] Cleanup handling of NULL value passed for a mount option Sachin Prabhu
  2012-04-10 17:17                 ` Jeff Layton
@ 2012-04-10 22:00                 ` Chris Clayton
  2012-04-11  2:17                 ` Steve French
  2 siblings, 0 replies; 10+ messages in thread
From: Chris Clayton @ 2012-04-10 22:00 UTC (permalink / raw)
  To: Sachin Prabhu; +Cc: Linux CIFS mailing list, Jeff Layton

On Tuesday 10 April 2012 18:12:27 Sachin Prabhu wrote:
> Allow blank user= and ip= mount option. Also clean up redundant
> checks for NULL values since the token parser will not actually
> match mount options with NULL values unless explicitly specified.
>
> Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Reported-by: Chris Clayton <chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>
> ---
>  fs/cifs/connect.c |   80
> ++++++++++++---------------------------------------- 1 files changed, 19
> insertions(+), 61 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index d81e933..6a86f3d 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -109,6 +109,8 @@ enum {
>
>  	/* Options which could be blank */
>  	Opt_blank_pass,
> +	Opt_blank_user,
> +	Opt_blank_ip,
>
>  	Opt_err
>  };
> @@ -183,11 +185,15 @@ static const match_table_t cifs_mount_option_tokens =
> { { Opt_wsize, "wsize=%s" },
>  	{ Opt_actimeo, "actimeo=%s" },
>
> +	{ Opt_blank_user, "user=" },
> +	{ Opt_blank_user, "username=" },
>  	{ Opt_user, "user=%s" },
>  	{ Opt_user, "username=%s" },
>  	{ Opt_blank_pass, "pass=" },
>  	{ Opt_pass, "pass=%s" },
>  	{ Opt_pass, "password=%s" },
> +	{ Opt_blank_ip, "ip=" },
> +	{ Opt_blank_ip, "addr=" },
>  	{ Opt_ip, "ip=%s" },
>  	{ Opt_ip, "addr=%s" },
>  	{ Opt_unc, "unc=%s" },
> @@ -1534,15 +1540,17 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname,
>
>  		/* String Arguments */
>
> +		case Opt_blank_user:
> +			/* null user, ie. anonymous authentication */
> +			vol->nullauth = 1;
> +			vol->username = NULL;
> +			break;
>  		case Opt_user:
>  			string = match_strdup(args);
>  			if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				/* null user, ie. anonymous authentication */
> -				vol->nullauth = 1;
> -			} else if (strnlen(string, MAX_USERNAME_SIZE) >
> +			if (strnlen(string, MAX_USERNAME_SIZE) >
>  							MAX_USERNAME_SIZE) {
>  				printk(KERN_WARNING "CIFS: username too long\n");
>  				goto cifs_parse_mount_err;
> @@ -1611,14 +1619,15 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, }
>  			vol->password[j] = '\0';
>  			break;
> +		case Opt_blank_ip:
> +			vol->UNCip = NULL;
> +			break;
>  		case Opt_ip:
>  			string = match_strdup(args);
>  			if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				vol->UNCip = NULL;
> -			} else if (strnlen(string, INET6_ADDRSTRLEN) >
> +			if (strnlen(string, INET6_ADDRSTRLEN) >
>  						INET6_ADDRSTRLEN) {
>  				printk(KERN_WARNING "CIFS: ip address "
>  						    "too long\n");
> @@ -1636,12 +1645,6 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: invalid path to "
> -						    "network resource\n");
> -				goto cifs_parse_mount_err;
> -			}
> -
>  			temp_len = strnlen(string, 300);
>  			if (temp_len  == 300) {
>  				printk(KERN_WARNING "CIFS: UNC name too long\n");
> @@ -1670,11 +1673,7 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: invalid domain"
> -						    " name\n");
> -				goto cifs_parse_mount_err;
> -			} else if (strnlen(string, 256) == 256) {
> +			if (strnlen(string, 256) == 256) {
>  				printk(KERN_WARNING "CIFS: domain name too"
>  						    " long\n");
>  				goto cifs_parse_mount_err;
> @@ -1693,11 +1692,7 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: srcaddr value not"
> -						    " specified\n");
> -				goto cifs_parse_mount_err;
> -			} else if (!cifs_convert_address(
> +			if (!cifs_convert_address(
>  					(struct sockaddr *)&vol->srcaddr,
>  					string, strlen(string))) {
>  				printk(KERN_WARNING "CIFS:  Could not parse"
> @@ -1710,11 +1705,6 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: Invalid path"
> -						    " prefix\n");
> -				goto cifs_parse_mount_err;
> -			}
>  			temp_len = strnlen(string, 1024);
>  			if (string[0] != '/')
>  				temp_len++; /* missing leading slash */
> @@ -1742,11 +1732,7 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: Invalid iocharset"
> -						    " specified\n");
> -				goto cifs_parse_mount_err;
> -			} else if (strnlen(string, 1024) >= 65) {
> +			if (strnlen(string, 1024) >= 65) {
>  				printk(KERN_WARNING "CIFS: iocharset name "
>  						    "too long.\n");
>  				goto cifs_parse_mount_err;
> @@ -1771,11 +1757,6 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: No socket option"
> -						    " specified\n");
> -				goto cifs_parse_mount_err;
> -			}
>  			if (strnicmp(string, "TCP_NODELAY", 11) == 0)
>  				vol->sockopt_tcp_nodelay = 1;
>  			break;
> @@ -1784,12 +1765,6 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: Invalid (empty)"
> -						    " netbiosname\n");
> -				break;
> -			}
> -
>  			memset(vol->source_rfc1001_name, 0x20,
>  				RFC1001_NAME_LEN);
>  			/*
> @@ -1817,11 +1792,6 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: Empty server"
> -					" netbiosname specified\n");
> -				break;
> -			}
>  			/* last byte, type, is 0x20 for servr type */
>  			memset(vol->target_rfc1001_name, 0x20,
>  				RFC1001_NAME_LEN_WITH_NULL);
> @@ -1848,12 +1818,6 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				cERROR(1, "no protocol version specified"
> -					  " after vers= mount option");
> -				goto cifs_parse_mount_err;
> -			}
> -
>  			if (strnicmp(string, "cifs", 4) == 0 ||
>  			    strnicmp(string, "1", 1) == 0) {
>  				/* This is the default */
> @@ -1868,12 +1832,6 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname, if (string == NULL)
>  				goto out_nomem;
>
> -			if (!*string) {
> -				printk(KERN_WARNING "CIFS: no security flavor"
> -						    " specified\n");
> -				break;
> -			}
> -
>  			if (cifs_parse_security_flavors(string, vol) != 0)
>  				goto cifs_parse_mount_err;
>  			break;

Works fine here, so:

Tested-by: Chris Clayton <chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>


-- 
The more I see, the more I know. The more I know, the less I understand. Changing Man - Paul Weller

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Cleanup handling of NULL value passed for a mount option
  2012-04-10 17:12               ` [PATCH] Cleanup handling of NULL value passed for a mount option Sachin Prabhu
  2012-04-10 17:17                 ` Jeff Layton
  2012-04-10 22:00                 ` Chris Clayton
@ 2012-04-11  2:17                 ` Steve French
       [not found]                   ` <CAH2r5mtrYxLueLL9VDSWE2G+86ZDyqXaFdOJz0FkNp8J0OzEUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2 siblings, 1 reply; 10+ messages in thread
From: Steve French @ 2012-04-11  2:17 UTC (permalink / raw)
  To: Sachin Prabhu; +Cc: Linux CIFS mailing list, Jeff Layton, Chris Clayton

Sachin,
Did you doublecheck the other 9 string based parms that we can pass in
(domain etc.) that don't have special casing yet?  Do any of them need
changes too?

On Tue, Apr 10, 2012 at 12:12 PM, Sachin Prabhu <sprabhu@redhat.com> wrote:
> Allow blank user= and ip= mount option. Also clean up redundant
> checks for NULL values since the token parser will not actually
> match mount options with NULL values unless explicitly specified.
>
> Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
> Reported-by: Chris Clayton <chris2553@googlemail.com>
>
> ---
>  fs/cifs/connect.c |   80 ++++++++++++----------------------------------------
>  1 files changed, 19 insertions(+), 61 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index d81e933..6a86f3d 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -109,6 +109,8 @@ enum {
>
>        /* Options which could be blank */
>        Opt_blank_pass,
> +       Opt_blank_user,
> +       Opt_blank_ip,
>
>        Opt_err
>  };
> @@ -183,11 +185,15 @@ static const match_table_t cifs_mount_option_tokens = {
>        { Opt_wsize, "wsize=%s" },
>        { Opt_actimeo, "actimeo=%s" },
>
> +       { Opt_blank_user, "user=" },
> +       { Opt_blank_user, "username=" },
>        { Opt_user, "user=%s" },
>        { Opt_user, "username=%s" },
>        { Opt_blank_pass, "pass=" },
>        { Opt_pass, "pass=%s" },
>        { Opt_pass, "password=%s" },
> +       { Opt_blank_ip, "ip=" },
> +       { Opt_blank_ip, "addr=" },
>        { Opt_ip, "ip=%s" },
>        { Opt_ip, "addr=%s" },
>        { Opt_unc, "unc=%s" },
> @@ -1534,15 +1540,17 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>
>                /* String Arguments */
>
> +               case Opt_blank_user:
> +                       /* null user, ie. anonymous authentication */
> +                       vol->nullauth = 1;
> +                       vol->username = NULL;
> +                       break;
>                case Opt_user:
>                        string = match_strdup(args);
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               /* null user, ie. anonymous authentication */
> -                               vol->nullauth = 1;
> -                       } else if (strnlen(string, MAX_USERNAME_SIZE) >
> +                       if (strnlen(string, MAX_USERNAME_SIZE) >
>                                                        MAX_USERNAME_SIZE) {
>                                printk(KERN_WARNING "CIFS: username too long\n");
>                                goto cifs_parse_mount_err;
> @@ -1611,14 +1619,15 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        }
>                        vol->password[j] = '\0';
>                        break;
> +               case Opt_blank_ip:
> +                       vol->UNCip = NULL;
> +                       break;
>                case Opt_ip:
>                        string = match_strdup(args);
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               vol->UNCip = NULL;
> -                       } else if (strnlen(string, INET6_ADDRSTRLEN) >
> +                       if (strnlen(string, INET6_ADDRSTRLEN) >
>                                                INET6_ADDRSTRLEN) {
>                                printk(KERN_WARNING "CIFS: ip address "
>                                                    "too long\n");
> @@ -1636,12 +1645,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               printk(KERN_WARNING "CIFS: invalid path to "
> -                                                   "network resource\n");
> -                               goto cifs_parse_mount_err;
> -                       }
> -
>                        temp_len = strnlen(string, 300);
>                        if (temp_len  == 300) {
>                                printk(KERN_WARNING "CIFS: UNC name too long\n");
> @@ -1670,11 +1673,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               printk(KERN_WARNING "CIFS: invalid domain"
> -                                                   " name\n");
> -                               goto cifs_parse_mount_err;
> -                       } else if (strnlen(string, 256) == 256) {
> +                       if (strnlen(string, 256) == 256) {
>                                printk(KERN_WARNING "CIFS: domain name too"
>                                                    " long\n");
>                                goto cifs_parse_mount_err;
> @@ -1693,11 +1692,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               printk(KERN_WARNING "CIFS: srcaddr value not"
> -                                                   " specified\n");
> -                               goto cifs_parse_mount_err;
> -                       } else if (!cifs_convert_address(
> +                       if (!cifs_convert_address(
>                                        (struct sockaddr *)&vol->srcaddr,
>                                        string, strlen(string))) {
>                                printk(KERN_WARNING "CIFS:  Could not parse"
> @@ -1710,11 +1705,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               printk(KERN_WARNING "CIFS: Invalid path"
> -                                                   " prefix\n");
> -                               goto cifs_parse_mount_err;
> -                       }
>                        temp_len = strnlen(string, 1024);
>                        if (string[0] != '/')
>                                temp_len++; /* missing leading slash */
> @@ -1742,11 +1732,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               printk(KERN_WARNING "CIFS: Invalid iocharset"
> -                                                   " specified\n");
> -                               goto cifs_parse_mount_err;
> -                       } else if (strnlen(string, 1024) >= 65) {
> +                       if (strnlen(string, 1024) >= 65) {
>                                printk(KERN_WARNING "CIFS: iocharset name "
>                                                    "too long.\n");
>                                goto cifs_parse_mount_err;
> @@ -1771,11 +1757,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               printk(KERN_WARNING "CIFS: No socket option"
> -                                                   " specified\n");
> -                               goto cifs_parse_mount_err;
> -                       }
>                        if (strnicmp(string, "TCP_NODELAY", 11) == 0)
>                                vol->sockopt_tcp_nodelay = 1;
>                        break;
> @@ -1784,12 +1765,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               printk(KERN_WARNING "CIFS: Invalid (empty)"
> -                                                   " netbiosname\n");
> -                               break;
> -                       }
> -
>                        memset(vol->source_rfc1001_name, 0x20,
>                                RFC1001_NAME_LEN);
>                        /*
> @@ -1817,11 +1792,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               printk(KERN_WARNING "CIFS: Empty server"
> -                                       " netbiosname specified\n");
> -                               break;
> -                       }
>                        /* last byte, type, is 0x20 for servr type */
>                        memset(vol->target_rfc1001_name, 0x20,
>                                RFC1001_NAME_LEN_WITH_NULL);
> @@ -1848,12 +1818,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               cERROR(1, "no protocol version specified"
> -                                         " after vers= mount option");
> -                               goto cifs_parse_mount_err;
> -                       }
> -
>                        if (strnicmp(string, "cifs", 4) == 0 ||
>                            strnicmp(string, "1", 1) == 0) {
>                                /* This is the default */
> @@ -1868,12 +1832,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>                        if (string == NULL)
>                                goto out_nomem;
>
> -                       if (!*string) {
> -                               printk(KERN_WARNING "CIFS: no security flavor"
> -                                                   " specified\n");
> -                               break;
> -                       }
> -
>                        if (cifs_parse_security_flavors(string, vol) != 0)
>                                goto cifs_parse_mount_err;
>                        break;
> --
> 1.7.7.6
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Cleanup handling of NULL value passed for a mount option
       [not found]                   ` <CAH2r5mtrYxLueLL9VDSWE2G+86ZDyqXaFdOJz0FkNp8J0OzEUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2012-04-11 12:03                     ` Sachin Prabhu
  0 siblings, 0 replies; 10+ messages in thread
From: Sachin Prabhu @ 2012-04-11 12:03 UTC (permalink / raw)
  To: Steve French; +Cc: Linux CIFS mailing list, Jeff Layton, Chris Clayton

On Tue, 2012-04-10 at 21:17 -0500, Steve French wrote:
> Sachin,
> Did you doublecheck the other 9 string based parms that we can pass in
> (domain etc.) that don't have special casing yet?  Do any of them need
> changes too?

Yes. I checked the remaining string and numerical mount options and
compared them to the old code and apart from user, password and ip
address, none of the other mount options allow NULL values. In the old
code, these either result in error or are just ignored. 

The new code returns an error if a blank value is passed when a value is
expected.

Sachin Prabhu

> 
> On Tue, Apr 10, 2012 at 12:12 PM, Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > Allow blank user= and ip= mount option. Also clean up redundant
> > checks for NULL values since the token parser will not actually
> > match mount options with NULL values unless explicitly specified.
> >
> > Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Reported-by: Chris Clayton <chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> >
> > ---
> >  fs/cifs/connect.c |   80 ++++++++++++----------------------------------------
> >  1 files changed, 19 insertions(+), 61 deletions(-)
> >
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index d81e933..6a86f3d 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -109,6 +109,8 @@ enum {
> >
> >        /* Options which could be blank */
> >        Opt_blank_pass,
> > +       Opt_blank_user,
> > +       Opt_blank_ip,
> >
> >        Opt_err
> >  };
> > @@ -183,11 +185,15 @@ static const match_table_t cifs_mount_option_tokens = {
> >        { Opt_wsize, "wsize=%s" },
> >        { Opt_actimeo, "actimeo=%s" },
> >
> > +       { Opt_blank_user, "user=" },
> > +       { Opt_blank_user, "username=" },
> >        { Opt_user, "user=%s" },
> >        { Opt_user, "username=%s" },
> >        { Opt_blank_pass, "pass=" },
> >        { Opt_pass, "pass=%s" },
> >        { Opt_pass, "password=%s" },
> > +       { Opt_blank_ip, "ip=" },
> > +       { Opt_blank_ip, "addr=" },
> >        { Opt_ip, "ip=%s" },
> >        { Opt_ip, "addr=%s" },
> >        { Opt_unc, "unc=%s" },
> > @@ -1534,15 +1540,17 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >
> >                /* String Arguments */
> >
> > +               case Opt_blank_user:
> > +                       /* null user, ie. anonymous authentication */
> > +                       vol->nullauth = 1;
> > +                       vol->username = NULL;
> > +                       break;
> >                case Opt_user:
> >                        string = match_strdup(args);
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               /* null user, ie. anonymous authentication */
> > -                               vol->nullauth = 1;
> > -                       } else if (strnlen(string, MAX_USERNAME_SIZE) >
> > +                       if (strnlen(string, MAX_USERNAME_SIZE) >
> >                                                        MAX_USERNAME_SIZE) {
> >                                printk(KERN_WARNING "CIFS: username too long\n");
> >                                goto cifs_parse_mount_err;
> > @@ -1611,14 +1619,15 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        }
> >                        vol->password[j] = '\0';
> >                        break;
> > +               case Opt_blank_ip:
> > +                       vol->UNCip = NULL;
> > +                       break;
> >                case Opt_ip:
> >                        string = match_strdup(args);
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               vol->UNCip = NULL;
> > -                       } else if (strnlen(string, INET6_ADDRSTRLEN) >
> > +                       if (strnlen(string, INET6_ADDRSTRLEN) >
> >                                                INET6_ADDRSTRLEN) {
> >                                printk(KERN_WARNING "CIFS: ip address "
> >                                                    "too long\n");
> > @@ -1636,12 +1645,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               printk(KERN_WARNING "CIFS: invalid path to "
> > -                                                   "network resource\n");
> > -                               goto cifs_parse_mount_err;
> > -                       }
> > -
> >                        temp_len = strnlen(string, 300);
> >                        if (temp_len  == 300) {
> >                                printk(KERN_WARNING "CIFS: UNC name too long\n");
> > @@ -1670,11 +1673,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               printk(KERN_WARNING "CIFS: invalid domain"
> > -                                                   " name\n");
> > -                               goto cifs_parse_mount_err;
> > -                       } else if (strnlen(string, 256) == 256) {
> > +                       if (strnlen(string, 256) == 256) {
> >                                printk(KERN_WARNING "CIFS: domain name too"
> >                                                    " long\n");
> >                                goto cifs_parse_mount_err;
> > @@ -1693,11 +1692,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               printk(KERN_WARNING "CIFS: srcaddr value not"
> > -                                                   " specified\n");
> > -                               goto cifs_parse_mount_err;
> > -                       } else if (!cifs_convert_address(
> > +                       if (!cifs_convert_address(
> >                                        (struct sockaddr *)&vol->srcaddr,
> >                                        string, strlen(string))) {
> >                                printk(KERN_WARNING "CIFS:  Could not parse"
> > @@ -1710,11 +1705,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               printk(KERN_WARNING "CIFS: Invalid path"
> > -                                                   " prefix\n");
> > -                               goto cifs_parse_mount_err;
> > -                       }
> >                        temp_len = strnlen(string, 1024);
> >                        if (string[0] != '/')
> >                                temp_len++; /* missing leading slash */
> > @@ -1742,11 +1732,7 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               printk(KERN_WARNING "CIFS: Invalid iocharset"
> > -                                                   " specified\n");
> > -                               goto cifs_parse_mount_err;
> > -                       } else if (strnlen(string, 1024) >= 65) {
> > +                       if (strnlen(string, 1024) >= 65) {
> >                                printk(KERN_WARNING "CIFS: iocharset name "
> >                                                    "too long.\n");
> >                                goto cifs_parse_mount_err;
> > @@ -1771,11 +1757,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               printk(KERN_WARNING "CIFS: No socket option"
> > -                                                   " specified\n");
> > -                               goto cifs_parse_mount_err;
> > -                       }
> >                        if (strnicmp(string, "TCP_NODELAY", 11) == 0)
> >                                vol->sockopt_tcp_nodelay = 1;
> >                        break;
> > @@ -1784,12 +1765,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               printk(KERN_WARNING "CIFS: Invalid (empty)"
> > -                                                   " netbiosname\n");
> > -                               break;
> > -                       }
> > -
> >                        memset(vol->source_rfc1001_name, 0x20,
> >                                RFC1001_NAME_LEN);
> >                        /*
> > @@ -1817,11 +1792,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               printk(KERN_WARNING "CIFS: Empty server"
> > -                                       " netbiosname specified\n");
> > -                               break;
> > -                       }
> >                        /* last byte, type, is 0x20 for servr type */
> >                        memset(vol->target_rfc1001_name, 0x20,
> >                                RFC1001_NAME_LEN_WITH_NULL);
> > @@ -1848,12 +1818,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               cERROR(1, "no protocol version specified"
> > -                                         " after vers= mount option");
> > -                               goto cifs_parse_mount_err;
> > -                       }
> > -
> >                        if (strnicmp(string, "cifs", 4) == 0 ||
> >                            strnicmp(string, "1", 1) == 0) {
> >                                /* This is the default */
> > @@ -1868,12 +1832,6 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
> >                        if (string == NULL)
> >                                goto out_nomem;
> >
> > -                       if (!*string) {
> > -                               printk(KERN_WARNING "CIFS: no security flavor"
> > -                                                   " specified\n");
> > -                               break;
> > -                       }
> > -
> >                        if (cifs_parse_security_flavors(string, vol) != 0)
> >                                goto cifs_parse_mount_err;
> >                        break;
> > --
> > 1.7.7.6
> >
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-04-11 12:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-10  7:23 3.4.0-rc2+ - CIFS mount failure Chris Clayton
     [not found] ` <201204100823.24207.chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
2012-04-10  9:39   ` Sachin Prabhu
2012-04-10 11:16     ` Jeff Layton
     [not found]       ` <20120410071630.567d70cb-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2012-04-10 13:13         ` Chris Clayton
     [not found]           ` <201204101413.27748.chris2553-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
2012-04-10 13:28             ` Sachin Prabhu
2012-04-10 17:12               ` [PATCH] Cleanup handling of NULL value passed for a mount option Sachin Prabhu
2012-04-10 17:17                 ` Jeff Layton
2012-04-10 22:00                 ` Chris Clayton
2012-04-11  2:17                 ` Steve French
     [not found]                   ` <CAH2r5mtrYxLueLL9VDSWE2G+86ZDyqXaFdOJz0FkNp8J0OzEUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-04-11 12:03                     ` Sachin Prabhu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.