3.4.0-rc2+ - CIFS mount failure
Subsystems (2)
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
Signature · 2 lines
The more I see, the more I know. The more I know, the less I understand. Changing Man - Paul Weller
A message that is not in the archive (sent off-list or not yet received): 201204100823.24207.chris2553-gM/[email protected]
Quoted 21 lines · Chris Clayton
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/fstabis://nas/Share /media/nas/share cifs rw,noauto,nosuid,nodev,user,guest,uid=1000,gid=100,file_mode=0666 0 0
Quoted 7 lines
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
Quoted 37 lines · On Tue, 10 Apr 2012 10:39:32 +0100 Sachin Prabhu…
Quoted 21 lines · Chris Clayton
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/fstabis://nas/Share /media/nas/share cifs rw,noauto,nosuid,nodev,user,guest,uid=1000,gid=100,file_mode=0666 0 0
Quoted 7 lines
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.
Quoted 7 lines
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/5762Sachin 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.
Signature · 1 line
Jeff Layton <[email protected]>
A message that is not in the archive (sent off-list or not yet received): 20120410071630.567d70cb-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org
Quoted 44 lines · On Tuesday 10 April 2012 12:16:30 Jeff Layton wrote:
On Tue, 10 Apr 2012 10:39:32 +0100
Quoted 41 lines · Sachin Prabhu
Quoted 21 lines · Chris Clayton
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/fstabis://nas/Share /media/nas/share cifs rw,noauto,nosuid,nodev,user,guest,uid=1000,gid=100,file_mode=0666 0 0
Quoted 7 lines
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
Quoted 5 lines
Quoted 1 line
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.
fs/cifs/connect.c 2012-04-10 12:16:02.000000000 +0100 renamed from fs/cifs/connect.c~ 2012-04-10 11:38:23.000000000 +0100 +8 -0
@@ -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)
Signature · 1 line
The more I see, the more I know. The more I know, the less I understand. Changing Man - Paul Weller
A message that is not in the archive (sent off-list or not yet received): 201204101413.27748.chris2553-gM/[email protected]
Quoted 25 lines
Signed-off-by Chris Clayton <chris2553-gM/[email protected]>fs/cifs/connect.c 2012-04-10 12:16:02.000000000 +0100 renamed from fs/cifs/connect.c~ 2012-04-10 11:38:23.000000000 +0100 +4
-0@@ -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=" },
fs/cifs/connect.c 2012-04-10 12:16:02.000000000 +0100 cifs_parse_mount_options(const char *mou · line 1547
Quoted 10 lines
fs/cifs/connect.c 2012-04-10 12:16:02.000000000 +0100 renamed from fs/cifs/connect.c~ 2012-04-10 11:38:23.000000000 +0100 +4
-0@@ -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
PATCH 1/1 Cleanup handling of NULL value passed for a mount option
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.
--- fs/cifs/connect.c | 80 ++++++++++++---------------------------------------- 1 files changed, 19 insertions(+), 61 deletions(-)
fs/cifs/connect.c +19 -61
@@ -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 ;
Signature · 1 line
1.7.7.6
Re: [PATCH] Cleanup handling of NULL value passed for a mount option
Quoted 198 lines · On Tue, 10 Apr 2012 18:12:27 +0100 Sachin Prabhu…
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 <[email protected]>Reported-by Chris Clayton <chris2553-gM/[email protected]>--- fs/cifs/connect.c | 80 ++++++++++++---------------------------------------- 1 files changed, 19 insertions(+), 61 deletions(-)fs/cifs/connect.c +19
-55@@ -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;
fs/cifs/connect.c cifs_parse_mount_options(const char *mountdata, const char *devname, · line 1837
Quoted 10 lines
fs/cifs/connect.c +0
-6-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.
Re: [PATCH] Cleanup handling of NULL value passed for a mount option
In reply to Sachin PrabhuQuoted 200 lines · 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 <[email protected]>Reported-by Chris Clayton <chris2553-gM/[email protected]>--- fs/cifs/connect.c | 80++++++++++++---------------------------------------- 1 files changed, 19 insertions(+), 61 deletions(-)
fs/cifs/connect.c +12
-4@@ -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" );fs/cifs/connect.c +7
-51goto 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;
fs/cifs/connect.c cifs_parse_mount_options(const char *mountdata, · line 1837
Quoted 10 lines
fs/cifs/connect.c +0
-6-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:
Signature · 1 line
The more I see, the more I know. The more I know, the less I understand. Changing Man - Paul Weller
Re: [PATCH] Cleanup handling of NULL value passed for a mount option
In reply to Sachin PrabhuSachin,
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?
Quoted 217 lines · Sachin Prabhu
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 <[email protected]>Reported-by Chris Clayton <[email protected]>---
fs/cifs/connect.c | 80 ++++++++++++----------------------------------------1 files changed, 19 insertions(+), 61 deletions(-)fs/cifs/connect.c +2
-0@@ -109,6 +109,8 @@ enum { /* Options which could be blank */ Opt_blank_pass, + Opt_blank_user, + Opt_blank_ip,Opt_err
};fs/cifs/connect.c +9
-0@@ -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) gotoout_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"); gotocifs_parse_mount_err;fs/cifs/connect.c +3
-0@@ -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) gotoout_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");fs/cifs/connect.c +1
-11@@ -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;fs/cifs/connect.c +1
-5@@ -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"
fs/cifs/connect.c +0
-5@@ -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 */
fs/cifs/connect.c +1
-5@@ -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;fs/cifs/connect.c +0
-22@@ -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 */fs/cifs/connect.c +0
-7@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
Thanks, Steve
A message that is not in the archive (sent off-list or not yet received): CAH2r5mtrYxLueLL9VDSWE2G+86ZDyqXaFdOJz0FkNp8J0OzEUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Re: [PATCH] Cleanup handling of NULL value passed for a mount option
Quoted 4 lines · Steve French
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
Quoted 222 lines · Sachin Prabhu
Quoted 217 lines
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 <[email protected]>Reported-by Chris Clayton <chris2553-gM/[email protected]>--- fs/cifs/connect.c | 80 ++++++++++++---------------------------------------- 1 files changed, 19 insertions(+), 61 deletions(-)fs/cifs/connect.c +19
-62@@ -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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html