[PATCH] cifs-utils: Correct max string lengths

[PATCH] cifs-utils: Correct max string lengths

[scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w]

From: Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

The max size of the username, domain, password and share name strings
are now consistent with the kernel and Microsoft's documentation.

Signed-off-by: Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 AUTHORS      |  2 ++
 mount.cifs.c | 27 ++++++++++++++++++---------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 2807079..2f6a14d 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -5,5 +5,7 @@ Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
 Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
 Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
 Igor Druzhinin <jaxbrigs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+
 
 ...and others.
diff --git a/mount.cifs.c b/mount.cifs.c
index 3b2b89e..8d975b3 100644
--- a/mount.cifs.c
+++ b/mount.cifs.c
@@ -83,22 +83,31 @@
 /* max length of mtab options */
 #define MTAB_OPTIONS_LEN 220
 
-/*
- * Maximum length of "share" portion of a UNC. I have no idea if this is at
- * all valid. According to MSDN, the typical max length of any component is
- * 255, so use that here.
+/* 
+ * Max share name, username, password and domain sizes match the kernel's 
+ * allowances for these string sizes which in turn match Microsoft's 
+ * documentation. 
  */
-#define MAX_SHARE_LEN 256
 
-/* max length of username (somewhat made up here) */
-#define MAX_USERNAME_SIZE 32
+/* Max length of the share name portion of a UNC.  According to Microsoft 
+ * this is correct for Windows 2000/XP. */
+#define MAX_SHARE_LEN 80
+
+/* Max user name length. */
+#define MAX_USERNAME_SIZE 256
+
+/* Max domain size. */
+#define MAX_DOMAIN_SIZE 256
+
+/* Max password size. */
+#define MOUNT_PASSWD_SIZE 512
+
+
 
 #ifndef SAFE_FREE
 #define SAFE_FREE(x) do { if ((x) != NULL) {free(x); x = NULL; } } while (0)
 #endif
 
-#define MOUNT_PASSWD_SIZE 128
-#define MAX_DOMAIN_SIZE 64
 
 /*
  * mount.cifs has been the subject of many "security" bugs that have arisen
-- 
1.8.1.4

Re: [PATCH] cifs-utils: Correct max string lengths

[Steve French]

MAX_SHARE_NAME of 80 is not correct.  I verified that Samba supports
much longer (see below), but also found some hits on other servers
with longer limits when searching for this.   Windows CLI does appear
to limit to 80 only in the NET SHARE command (also note that maximum
UNC path is probably limited by MAX_PATH to 32768 characters)

On Fri, Jul 19, 2013 at 4:26 PM,  <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> The max size of the username, domain, password and share name strings
> are now consistent with the kernel and Microsoft's documentation.
>
> Signed-off-by: Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  AUTHORS      |  2 ++
>  mount.cifs.c | 27 ++++++++++++++++++---------
>  2 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/AUTHORS b/AUTHORS
> index 2807079..2f6a14d 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -5,5 +5,7 @@ Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>  Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
>  Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
>  Igor Druzhinin <jaxbrigs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> +Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> +
>
>  ...and others.
> diff --git a/mount.cifs.c b/mount.cifs.c
> index 3b2b89e..8d975b3 100644
> --- a/mount.cifs.c
> +++ b/mount.cifs.c
> @@ -83,22 +83,31 @@
>  /* max length of mtab options */
>  #define MTAB_OPTIONS_LEN 220
>
> -/*
> - * Maximum length of "share" portion of a UNC. I have no idea if this is at
> - * all valid. According to MSDN, the typical max length of any component is
> - * 255, so use that here.
> +/*
> + * Max share name, username, password and domain sizes match the kernel's
> + * allowances for these string sizes which in turn match Microsoft's
> + * documentation.
>   */
> -#define MAX_SHARE_LEN 256
>
> -/* max length of username (somewhat made up here) */
> -#define MAX_USERNAME_SIZE 32
> +/* Max length of the share name portion of a UNC.  According to Microsoft
> + * this is correct for Windows 2000/XP. */
> +#define MAX_SHARE_LEN 80
> +
> +/* Max user name length. */
> +#define MAX_USERNAME_SIZE 256
> +
> +/* Max domain size. */
> +#define MAX_DOMAIN_SIZE 256
> +
> +/* Max password size. */
> +#define MOUNT_PASSWD_SIZE 512
> +
> +
>
>  #ifndef SAFE_FREE
>  #define SAFE_FREE(x) do { if ((x) != NULL) {free(x); x = NULL; } } while (0)
>  #endif
>
> -#define MOUNT_PASSWD_SIZE 128
> -#define MAX_DOMAIN_SIZE 64
>
>  /*
>   * mount.cifs has been the subject of many "security" bugs that have arisen
> --
> 1.8.1.4
>



-- 
Thanks,

Steve

Re: [PATCH] cifs-utils: Correct max string lengths

[Steve French]

To Samba server - note very long share name (other servers also support this)

steven@steven-GA-970A-DS3:~$ smbclient -L //localhost
Enter steven's password:
Domain=[WORKGROUP] OS=[Unix] Server=[Samba 4.1.0pre1-GIT-e3f5e47]

	Sharename       Type      Comment
	---------       ----      -------
	BREALLYLONGSHARENAME0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
Disk      Long name test Directories

On Fri, Jul 19, 2013 at 4:51 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> MAX_SHARE_NAME of 80 is not correct.  I verified that Samba supports
> much longer (see below), but also found some hits on other servers
> with longer limits when searching for this.   Windows CLI does appear
> to limit to 80 only in the NET SHARE command (also note that maximum
> UNC path is probably limited by MAX_PATH to 32768 characters)
>
> On Fri, Jul 19, 2013 at 4:26 PM,  <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> The max size of the username, domain, password and share name strings
>> are now consistent with the kernel and Microsoft's documentation.
>>
>> Signed-off-by: Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  AUTHORS      |  2 ++
>>  mount.cifs.c | 27 ++++++++++++++++++---------
>>  2 files changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/AUTHORS b/AUTHORS
>> index 2807079..2f6a14d 100644
>> --- a/AUTHORS
>> +++ b/AUTHORS
>> @@ -5,5 +5,7 @@ Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>  Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
>>  Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
>>  Igor Druzhinin <jaxbrigs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> +Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> +
>>
>>  ...and others.
>> diff --git a/mount.cifs.c b/mount.cifs.c
>> index 3b2b89e..8d975b3 100644
>> --- a/mount.cifs.c
>> +++ b/mount.cifs.c
>> @@ -83,22 +83,31 @@
>>  /* max length of mtab options */
>>  #define MTAB_OPTIONS_LEN 220
>>
>> -/*
>> - * Maximum length of "share" portion of a UNC. I have no idea if this is at
>> - * all valid. According to MSDN, the typical max length of any component is
>> - * 255, so use that here.
>> +/*
>> + * Max share name, username, password and domain sizes match the kernel's
>> + * allowances for these string sizes which in turn match Microsoft's
>> + * documentation.
>>   */
>> -#define MAX_SHARE_LEN 256
>>
>> -/* max length of username (somewhat made up here) */
>> -#define MAX_USERNAME_SIZE 32
>> +/* Max length of the share name portion of a UNC.  According to Microsoft
>> + * this is correct for Windows 2000/XP. */
>> +#define MAX_SHARE_LEN 80
>> +
>> +/* Max user name length. */
>> +#define MAX_USERNAME_SIZE 256
>> +
>> +/* Max domain size. */
>> +#define MAX_DOMAIN_SIZE 256
>> +
>> +/* Max password size. */
>> +#define MOUNT_PASSWD_SIZE 512
>> +
>> +
>>
>>  #ifndef SAFE_FREE
>>  #define SAFE_FREE(x) do { if ((x) != NULL) {free(x); x = NULL; } } while (0)
>>  #endif
>>
>> -#define MOUNT_PASSWD_SIZE 128
>> -#define MAX_DOMAIN_SIZE 64
>>
>>  /*
>>   * mount.cifs has been the subject of many "security" bugs that have arisen
>> --
>> 1.8.1.4
>>
>
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

Re: [PATCH] cifs-utils: Correct max string lengths

[Scott Lovenberg]

On Jul 19, 2013, at 17:51, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> MAX_SHARE_NAME of 80 is not correct.  I verified that Samba supports
> much longer (see below), but also found some hits on other servers
> with longer limits when searching for this.   Windows CLI does appear
> to limit to 80 only in the NET SHARE command (also note that maximum
> UNC path is probably limited by MAX_PATH to 32768 characters)

That appears to be correct. The full length of the path is the ultimate limit.  How would you like to handle this?

> 
> On Fri, Jul 19, 2013 at 4:26 PM,  <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> 
>> The max size of the username, domain, password and share name strings
>> are now consistent with the kernel and Microsoft's documentation.
>> 
>> Signed-off-by: Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> AUTHORS      |  2 ++
>> mount.cifs.c | 27 ++++++++++++++++++---------
>> 2 files changed, 20 insertions(+), 9 deletions(-)
>> 
>> diff --git a/AUTHORS b/AUTHORS
>> index 2807079..2f6a14d 100644
>> --- a/AUTHORS
>> +++ b/AUTHORS
>> @@ -5,5 +5,7 @@ Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
>> Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
>> Igor Druzhinin <jaxbrigs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> +Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> +
>> 
>> ...and others.
>> diff --git a/mount.cifs.c b/mount.cifs.c
>> index 3b2b89e..8d975b3 100644
>> --- a/mount.cifs.c
>> +++ b/mount.cifs.c
>> @@ -83,22 +83,31 @@
>> /* max length of mtab options */
>> #define MTAB_OPTIONS_LEN 220
>> 
>> -/*
>> - * Maximum length of "share" portion of a UNC. I have no idea if this is at
>> - * all valid. According to MSDN, the typical max length of any component is
>> - * 255, so use that here.
>> +/*
>> + * Max share name, username, password and domain sizes match the kernel's
>> + * allowances for these string sizes which in turn match Microsoft's
>> + * documentation.
>>  */
>> -#define MAX_SHARE_LEN 256
>> 
>> -/* max length of username (somewhat made up here) */
>> -#define MAX_USERNAME_SIZE 32
>> +/* Max length of the share name portion of a UNC.  According to Microsoft
>> + * this is correct for Windows 2000/XP. */
>> +#define MAX_SHARE_LEN 80
>> +
>> +/* Max user name length. */
>> +#define MAX_USERNAME_SIZE 256
>> +
>> +/* Max domain size. */
>> +#define MAX_DOMAIN_SIZE 256
>> +
>> +/* Max password size. */
>> +#define MOUNT_PASSWD_SIZE 512
>> +
>> +
>> 
>> #ifndef SAFE_FREE
>> #define SAFE_FREE(x) do { if ((x) != NULL) {free(x); x = NULL; } } while (0)
>> #endif
>> 
>> -#define MOUNT_PASSWD_SIZE 128
>> -#define MAX_DOMAIN_SIZE 64
>> 
>> /*
>>  * mount.cifs has been the subject of many "security" bugs that have arisen
>> --
>> 1.8.1.4
> 
> 
> 
> -- 
> Thanks,
> 
> Steve

Re: [PATCH] cifs-utils: Correct max string lengths

[Steve French]

Leaving Max share length at 256 is fine with me
On Jul 19, 2013 5:20 PM, "Scott Lovenberg" <scott.lovenberg@gmail.com>
wrote:

> On Jul 19, 2013, at 17:51, Steve French <smfrench@gmail.com> wrote:
>
> > MAX_SHARE_NAME of 80 is not correct.  I verified that Samba supports
> > much longer (see below), but also found some hits on other servers
> > with longer limits when searching for this.   Windows CLI does appear
> > to limit to 80 only in the NET SHARE command (also note that maximum
> > UNC path is probably limited by MAX_PATH to 32768 characters)
>
> That appears to be correct. The full length of the path is the ultimate
> limit.  How would you like to handle this?
>
> >
> > On Fri, Jul 19, 2013 at 4:26 PM,  <scott.lovenberg@gmail.com> wrote:
> >> From: Scott Lovenberg <scott.lovenberg@gmail.com>
> >>
> >> The max size of the username, domain, password and share name strings
> >> are now consistent with the kernel and Microsoft's documentation.
> >>
> >> Signed-off-by: Scott Lovenberg <scott.lovenberg@gmail.com>
> >> ---
> >> AUTHORS      |  2 ++
> >> mount.cifs.c | 27 ++++++++++++++++++---------
> >> 2 files changed, 20 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/AUTHORS b/AUTHORS
> >> index 2807079..2f6a14d 100644
> >> --- a/AUTHORS
> >> +++ b/AUTHORS
> >> @@ -5,5 +5,7 @@ Shirish Pargaonkar <shirishpargaonkar@gmail.com>
> >> Suresh Jayaraman <sjayaraman@suse.de>
> >> Pavel Shilovsky <piastry@etersoft.ru>
> >> Igor Druzhinin <jaxbrigs@gmail.com>
> >> +Scott Lovenberg <scott.lovenberg@gmail.com>
> >> +
> >>
> >> ...and others.
> >> diff --git a/mount.cifs.c b/mount.cifs.c
> >> index 3b2b89e..8d975b3 100644
> >> --- a/mount.cifs.c
> >> +++ b/mount.cifs.c
> >> @@ -83,22 +83,31 @@
> >> /* max length of mtab options */
> >> #define MTAB_OPTIONS_LEN 220
> >>
> >> -/*
> >> - * Maximum length of "share" portion of a UNC. I have no idea if this
> is at
> >> - * all valid. According to MSDN, the typical max length of any
> component is
> >> - * 255, so use that here.
> >> +/*
> >> + * Max share name, username, password and domain sizes match the
> kernel's
> >> + * allowances for these string sizes which in turn match Microsoft's
> >> + * documentation.
> >>  */
> >> -#define MAX_SHARE_LEN 256
> >>
> >> -/* max length of username (somewhat made up here) */
> >> -#define MAX_USERNAME_SIZE 32
> >> +/* Max length of the share name portion of a UNC.  According to
> Microsoft
> >> + * this is correct for Windows 2000/XP. */
> >> +#define MAX_SHARE_LEN 80
> >> +
> >> +/* Max user name length. */
> >> +#define MAX_USERNAME_SIZE 256
> >> +
> >> +/* Max domain size. */
> >> +#define MAX_DOMAIN_SIZE 256
> >> +
> >> +/* Max password size. */
> >> +#define MOUNT_PASSWD_SIZE 512
> >> +
> >> +
> >>
> >> #ifndef SAFE_FREE
> >> #define SAFE_FREE(x) do { if ((x) != NULL) {free(x); x = NULL; } }
> while (0)
> >> #endif
> >>
> >> -#define MOUNT_PASSWD_SIZE 128
> >> -#define MAX_DOMAIN_SIZE 64
> >>
> >> /*
> >>  * mount.cifs has been the subject of many "security" bugs that have
> arisen
> >> --
> >> 1.8.1.4
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
>

Re: [PATCH] cifs-utils: Correct max string lengths

[Scott Lovenberg]

On Jul 19, 2013, at 22:34, Steve French <smfrench@gmail.com> wrote:

> Leaving Max share length at 256 is fine with me
> 
Works for me. Should this be fixed kernel side?

> On Jul 19, 2013 5:20 PM, "Scott Lovenberg" <scott.lovenberg@gmail.com> wrote:
>> On Jul 19, 2013, at 17:51, Steve French <smfrench@gmail.com> wrote:
>> 
>> > MAX_SHARE_NAME of 80 is not correct.  I verified that Samba supports
>> > much longer (see below), but also found some hits on other servers
>> > with longer limits when searching for this.   Windows CLI does appear
>> > to limit to 80 only in the NET SHARE command (also note that maximum
>> > UNC path is probably limited by MAX_PATH to 32768 characters)
>> 
>> That appears to be correct. The full length of the path is the ultimate limit.  How would you like to handle this?
>> 
>> >
>> > On Fri, Jul 19, 2013 at 4:26 PM,  <scott.lovenberg@gmail.com> wrote:
>> >> From: Scott Lovenberg <scott.lovenberg@gmail.com>
>> >>
>> >> The max size of the username, domain, password and share name strings
>> >> are now consistent with the kernel and Microsoft's documentation.
>> >>
>> >> Signed-off-by: Scott Lovenberg <scott.lovenberg@gmail.com>
>> >> ---
>> >> AUTHORS      |  2 ++
>> >> mount.cifs.c | 27 ++++++++++++++++++---------
>> >> 2 files changed, 20 insertions(+), 9 deletions(-)
>> >>
>> >> diff --git a/AUTHORS b/AUTHORS
>> >> index 2807079..2f6a14d 100644
>> >> --- a/AUTHORS
>> >> +++ b/AUTHORS
>> >> @@ -5,5 +5,7 @@ Shirish Pargaonkar <shirishpargaonkar@gmail.com>
>> >> Suresh Jayaraman <sjayaraman@suse.de>
>> >> Pavel Shilovsky <piastry@etersoft.ru>
>> >> Igor Druzhinin <jaxbrigs@gmail.com>
>> >> +Scott Lovenberg <scott.lovenberg@gmail.com>
>> >> +
>> >>
>> >> ...and others.
>> >> diff --git a/mount.cifs.c b/mount.cifs.c
>> >> index 3b2b89e..8d975b3 100644
>> >> --- a/mount.cifs.c
>> >> +++ b/mount.cifs.c
>> >> @@ -83,22 +83,31 @@
>> >> /* max length of mtab options */
>> >> #define MTAB_OPTIONS_LEN 220
>> >>
>> >> -/*
>> >> - * Maximum length of "share" portion of a UNC. I have no idea if this is at
>> >> - * all valid. According to MSDN, the typical max length of any component is
>> >> - * 255, so use that here.
>> >> +/*
>> >> + * Max share name, username, password and domain sizes match the kernel's
>> >> + * allowances for these string sizes which in turn match Microsoft's
>> >> + * documentation.
>> >>  */
>> >> -#define MAX_SHARE_LEN 256
>> >>
>> >> -/* max length of username (somewhat made up here) */
>> >> -#define MAX_USERNAME_SIZE 32
>> >> +/* Max length of the share name portion of a UNC.  According to Microsoft
>> >> + * this is correct for Windows 2000/XP. */
>> >> +#define MAX_SHARE_LEN 80
>> >> +
>> >> +/* Max user name length. */
>> >> +#define MAX_USERNAME_SIZE 256
>> >> +
>> >> +/* Max domain size. */
>> >> +#define MAX_DOMAIN_SIZE 256
>> >> +
>> >> +/* Max password size. */
>> >> +#define MOUNT_PASSWD_SIZE 512
>> >> +
>> >> +
>> >>
>> >> #ifndef SAFE_FREE
>> >> #define SAFE_FREE(x) do { if ((x) != NULL) {free(x); x = NULL; } } while (0)
>> >> #endif
>> >>
>> >> -#define MOUNT_PASSWD_SIZE 128
>> >> -#define MAX_DOMAIN_SIZE 64
>> >>
>> >> /*
>> >>  * mount.cifs has been the subject of many "security" bugs that have arisen
>> >> --
>> >> 1.8.1.4
>> >
>> >
>> >
>> > --
>> > Thanks,
>> >
>> > Steve

Re: [PATCH] cifs-utils: Correct max string lengths

[Jeff Layton]

On Fri, 19 Jul 2013 22:44:22 -0400
Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Jul 19, 2013, at 22:34, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> > Leaving Max share length at 256 is fine with me
> > 
> Works for me. Should this be fixed kernel side?
> 

Yes, I think that would make sense. Scott, I assume you'll send a v2 of
this patch that preserves the share length at 256?

Once that's done, the ideal thing here would be to move these
definitions in the kernel to a separate header file that lives under
include/linux. Then we could have autoconf in cifs-utils look for that
header and use the #defines from that instead if it's present.

That way we won't be so reliant on keeping the kernel and cifs-utils in
sync...

> > On Jul 19, 2013 5:20 PM, "Scott Lovenberg" <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> On Jul 19, 2013, at 17:51, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> 
> >> > MAX_SHARE_NAME of 80 is not correct.  I verified that Samba supports
> >> > much longer (see below), but also found some hits on other servers
> >> > with longer limits when searching for this.   Windows CLI does appear
> >> > to limit to 80 only in the NET SHARE command (also note that maximum
> >> > UNC path is probably limited by MAX_PATH to 32768 characters)
> >> 
> >> That appears to be correct. The full length of the path is the ultimate limit.  How would you like to handle this?
> >> 
> >> >
> >> > On Fri, Jul 19, 2013 at 4:26 PM,  <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> >> From: Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> >>
> >> >> The max size of the username, domain, password and share name strings
> >> >> are now consistent with the kernel and Microsoft's documentation.
> >> >>
> >> >> Signed-off-by: Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> >> ---
> >> >> AUTHORS      |  2 ++
> >> >> mount.cifs.c | 27 ++++++++++++++++++---------
> >> >> 2 files changed, 20 insertions(+), 9 deletions(-)
> >> >>
> >> >> diff --git a/AUTHORS b/AUTHORS
> >> >> index 2807079..2f6a14d 100644
> >> >> --- a/AUTHORS
> >> >> +++ b/AUTHORS
> >> >> @@ -5,5 +5,7 @@ Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> >> Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> >> >> Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> >> >> Igor Druzhinin <jaxbrigs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> >> +Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> >> +
> >> >>
> >> >> ...and others.
> >> >> diff --git a/mount.cifs.c b/mount.cifs.c
> >> >> index 3b2b89e..8d975b3 100644
> >> >> --- a/mount.cifs.c
> >> >> +++ b/mount.cifs.c
> >> >> @@ -83,22 +83,31 @@
> >> >> /* max length of mtab options */
> >> >> #define MTAB_OPTIONS_LEN 220
> >> >>
> >> >> -/*
> >> >> - * Maximum length of "share" portion of a UNC. I have no idea if this is at
> >> >> - * all valid. According to MSDN, the typical max length of any component is
> >> >> - * 255, so use that here.
> >> >> +/*
> >> >> + * Max share name, username, password and domain sizes match the kernel's
> >> >> + * allowances for these string sizes which in turn match Microsoft's
> >> >> + * documentation.
> >> >>  */
> >> >> -#define MAX_SHARE_LEN 256
> >> >>
> >> >> -/* max length of username (somewhat made up here) */
> >> >> -#define MAX_USERNAME_SIZE 32
> >> >> +/* Max length of the share name portion of a UNC.  According to Microsoft
> >> >> + * this is correct for Windows 2000/XP. */
> >> >> +#define MAX_SHARE_LEN 80
> >> >> +
> >> >> +/* Max user name length. */
> >> >> +#define MAX_USERNAME_SIZE 256
> >> >> +
> >> >> +/* Max domain size. */
> >> >> +#define MAX_DOMAIN_SIZE 256
> >> >> +
> >> >> +/* Max password size. */
> >> >> +#define MOUNT_PASSWD_SIZE 512
> >> >> +
> >> >> +
> >> >>
> >> >> #ifndef SAFE_FREE
> >> >> #define SAFE_FREE(x) do { if ((x) != NULL) {free(x); x = NULL; } } while (0)
> >> >> #endif
> >> >>
> >> >> -#define MOUNT_PASSWD_SIZE 128
> >> >> -#define MAX_DOMAIN_SIZE 64
> >> >>
> >> >> /*
> >> >>  * mount.cifs has been the subject of many "security" bugs that have arisen
> >> >> --
> >> >> 1.8.1.4
> >> >
> >> >
> >> >
> >> > --
> >> > Thanks,
> >> >
> >> > Steve


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs-utils: Correct max string lengths

[Scott Lovenberg]

Sent from my iPod

On Jul 20, 2013, at 6:43, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> On Fri, 19 Jul 2013 22:44:22 -0400
> Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
>> On Jul 19, 2013, at 22:34, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> 
>>> Leaving Max share length at 256 is fine with me
>> Works for me. Should this be fixed kernel side?
> 
> Yes, I think that would make sense. Scott, I assume you'll send a v2 of
> this patch that preserves the share length at 256?
> 
> Once that's done, the ideal thing here would be to move these
> definitions in the kernel to a separate header file that lives under
> include/linux. Then we could have autoconf in cifs-utils look for that
> header and use the #defines from that instead if it's present.
> 
> That way we won't be so reliant on keeping the kernel and cifs-utils in
> sync...

Yeah. I'll respin and send tomorrow. I have a wedding to attend this afternoon.  Using autoconf would be terrific and I'm completely on board with that idea. 

> -- 
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs-utils: Correct max string lengths

[Chen Gang]

On 07/20/2013 11:24 PM, Scott Lovenberg wrote:
> 
> 
> Sent from my iPod
> 
> On Jul 20, 2013, at 6:43, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> 
>> On Fri, 19 Jul 2013 22:44:22 -0400
>> Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> On Jul 19, 2013, at 22:34, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>>> Leaving Max share length at 256 is fine with me
>>> Works for me. Should this be fixed kernel side?
>>
>> Yes, I think that would make sense. Scott, I assume you'll send a v2 of
>> this patch that preserves the share length at 256?
>>
>> Once that's done, the ideal thing here would be to move these
>> definitions in the kernel to a separate header file that lives under
>> include/linux. Then we could have autoconf in cifs-utils look for that
>> header and use the #defines from that instead if it's present.
>>
>> That way we won't be so reliant on keeping the kernel and cifs-utils in
>> sync...
> 
> Yeah. I'll respin and send tomorrow. I have a wedding to attend this afternoon.  Using autoconf would be terrific and I'm completely on board with that idea. 
> 

Is it better to let the new header file in "include/uapi/linux" ? It
belongs to the interface between user mode and kernel mode.

>> -- 
>> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 
> 

Thanks.
-- 
Chen Gang

Re: [PATCH] cifs-utils: Correct max string lengths

[Jeff Layton]

On Mon, 22 Jul 2013 09:38:17 +0800
Chen Gang <gang.chen@asianux.com> wrote:

> On 07/20/2013 11:24 PM, Scott Lovenberg wrote:
> > 
> > 
> > Sent from my iPod
> > 
> > On Jul 20, 2013, at 6:43, Jeff Layton <jlayton@redhat.com> wrote:
> > 
> >> On Fri, 19 Jul 2013 22:44:22 -0400
> >> Scott Lovenberg <scott.lovenberg@gmail.com> wrote:
> >>
> >>> On Jul 19, 2013, at 22:34, Steve French <smfrench@gmail.com> wrote:
> >>>
> >>>> Leaving Max share length at 256 is fine with me
> >>> Works for me. Should this be fixed kernel side?
> >>
> >> Yes, I think that would make sense. Scott, I assume you'll send a v2 of
> >> this patch that preserves the share length at 256?
> >>
> >> Once that's done, the ideal thing here would be to move these
> >> definitions in the kernel to a separate header file that lives under
> >> include/linux. Then we could have autoconf in cifs-utils look for that
> >> header and use the #defines from that instead if it's present.
> >>
> >> That way we won't be so reliant on keeping the kernel and cifs-utils in
> >> sync...
> > 
> > Yeah. I'll respin and send tomorrow. I have a wedding to attend this afternoon.  Using autoconf would be terrific and I'm completely on board with that idea. 
> > 
> 
> Is it better to let the new header file in "include/uapi/linux" ? It
> belongs to the interface between user mode and kernel mode.
> 

That's more of a long-term fix. We'll still need to build cifs-utils
against kernel headers that don't contain that header file, so fixing
the lengths here is needed anyway.

Once this patch goes in, adding a header in include/uapi/linux that
holds these lengths (and any others that the mount helper and cifs.ko
need to share) would be helpful. Then we could simply look for that
file at autoconf time, and conditionally include it if it exists.

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] cifs-utils: Correct max string lengths

[Scott Lovenberg]

On Mon, Jul 22, 2013 at 6:53 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 22 Jul 2013 09:38:17 +0800
> Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org> wrote:
>
>> On 07/20/2013 11:24 PM, Scott Lovenberg wrote:
>> >
>> >
>> > Sent from my iPod
>> >
>> > On Jul 20, 2013, at 6:43, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> >
>> >> On Fri, 19 Jul 2013 22:44:22 -0400
>> >> Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >>
>> >>> On Jul 19, 2013, at 22:34, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >>>
>> >>>> Leaving Max share length at 256 is fine with me
>> >>> Works for me. Should this be fixed kernel side?
>> >>
>> >> Yes, I think that would make sense. Scott, I assume you'll send a v2 of
>> >> this patch that preserves the share length at 256?
>> >>
>> >> Once that's done, the ideal thing here would be to move these
>> >> definitions in the kernel to a separate header file that lives under
>> >> include/linux. Then we could have autoconf in cifs-utils look for that
>> >> header and use the #defines from that instead if it's present.
>> >>
>> >> That way we won't be so reliant on keeping the kernel and cifs-utils in
>> >> sync...
>> >
>> > Yeah. I'll respin and send tomorrow. I have a wedding to attend this afternoon.  Using autoconf would be terrific and I'm completely on board with that idea.
>> >
>>
>> Is it better to let the new header file in "include/uapi/linux" ? It
>> belongs to the interface between user mode and kernel mode.
>>
>
> That's more of a long-term fix. We'll still need to build cifs-utils
> against kernel headers that don't contain that header file, so fixing
> the lengths here is needed anyway.
>
> Once this patch goes in, adding a header in include/uapi/linux that
> holds these lengths (and any others that the mount helper and cifs.ko
> need to share) would be helpful. Then we could simply look for that
> file at autoconf time, and conditionally include it if it exists.
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>


Since we're doing that we might as well standardize on the variable
names.  Should we just make the mount helper use the same variable
names as the kernel to avoid confusion?  That will be an external
change (ie, changing a non-static variable) if anyone linked to the
mount helper (doubtful, but possible) and it will break their code.

--
Peace and Blessings,
-Scott.

Re: [PATCH] cifs-utils: Correct max string lengths

[Jeff Layton]

On Mon, 22 Jul 2013 11:07:47 -0400
Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Mon, Jul 22, 2013 at 6:53 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Mon, 22 Jul 2013 09:38:17 +0800
> > Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org> wrote:
> >
> >> On 07/20/2013 11:24 PM, Scott Lovenberg wrote:
> >> >
> >> >
> >> > Sent from my iPod
> >> >
> >> > On Jul 20, 2013, at 6:43, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >> >
> >> >> On Fri, 19 Jul 2013 22:44:22 -0400
> >> >> Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> >>
> >> >>> On Jul 19, 2013, at 22:34, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> >>>
> >> >>>> Leaving Max share length at 256 is fine with me
> >> >>> Works for me. Should this be fixed kernel side?
> >> >>
> >> >> Yes, I think that would make sense. Scott, I assume you'll send a v2 of
> >> >> this patch that preserves the share length at 256?
> >> >>
> >> >> Once that's done, the ideal thing here would be to move these
> >> >> definitions in the kernel to a separate header file that lives under
> >> >> include/linux. Then we could have autoconf in cifs-utils look for that
> >> >> header and use the #defines from that instead if it's present.
> >> >>
> >> >> That way we won't be so reliant on keeping the kernel and cifs-utils in
> >> >> sync...
> >> >
> >> > Yeah. I'll respin and send tomorrow. I have a wedding to attend this afternoon.  Using autoconf would be terrific and I'm completely on board with that idea.
> >> >
> >>
> >> Is it better to let the new header file in "include/uapi/linux" ? It
> >> belongs to the interface between user mode and kernel mode.
> >>
> >
> > That's more of a long-term fix. We'll still need to build cifs-utils
> > against kernel headers that don't contain that header file, so fixing
> > the lengths here is needed anyway.
> >
> > Once this patch goes in, adding a header in include/uapi/linux that
> > holds these lengths (and any others that the mount helper and cifs.ko
> > need to share) would be helpful. Then we could simply look for that
> > file at autoconf time, and conditionally include it if it exists.
> >
> > --
> > Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 
> 
> Since we're doing that we might as well standardize on the variable
> names.  Should we just make the mount helper use the same variable
> names as the kernel to avoid confusion?  That will be an external
> change (ie, changing a non-static variable) if anyone linked to the
> mount helper (doubtful, but possible) and it will break their code.
> 

The variable names aren't terribly important, so I wouldn't go making
any sort of change like that for "cleanup". What matters here is the
names of the preprocessor constants. Those will need to be unified
between userland and kernel in order to make such a scheme work.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH] cifs-utils: Correct max string lengths

[Scott Lovenberg]

On Mon, Jul 22, 2013 at 11:12 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Mon, 22 Jul 2013 11:07:47 -0400
> Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Mon, Jul 22, 2013 at 6:53 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> > On Mon, 22 Jul 2013 09:38:17 +0800
>> > Chen Gang <gang.chen-bOixZGp5f+dBDgjK7y7TUQ@public.gmane.org> wrote:
>> >
>> >> On 07/20/2013 11:24 PM, Scott Lovenberg wrote:
>> >> >
>> >> >
>> >> > Sent from my iPod
>> >> >
>> >> > On Jul 20, 2013, at 6:43, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> >> >
>> >> >> On Fri, 19 Jul 2013 22:44:22 -0400
>> >> >> Scott Lovenberg <scott.lovenberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >> >>
>> >> >>> On Jul 19, 2013, at 22:34, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >> >>>
>> >> >>>> Leaving Max share length at 256 is fine with me
>> >> >>> Works for me. Should this be fixed kernel side?
>> >> >>
>> >> >> Yes, I think that would make sense. Scott, I assume you'll send a v2 of
>> >> >> this patch that preserves the share length at 256?
>> >> >>
>> >> >> Once that's done, the ideal thing here would be to move these
>> >> >> definitions in the kernel to a separate header file that lives under
>> >> >> include/linux. Then we could have autoconf in cifs-utils look for that
>> >> >> header and use the #defines from that instead if it's present.
>> >> >>
>> >> >> That way we won't be so reliant on keeping the kernel and cifs-utils in
>> >> >> sync...
>> >> >
>> >> > Yeah. I'll respin and send tomorrow. I have a wedding to attend this afternoon.  Using autoconf would be terrific and I'm completely on board with that idea.
>> >> >
>> >>
>> >> Is it better to let the new header file in "include/uapi/linux" ? It
>> >> belongs to the interface between user mode and kernel mode.
>> >>
>> >
>> > That's more of a long-term fix. We'll still need to build cifs-utils
>> > against kernel headers that don't contain that header file, so fixing
>> > the lengths here is needed anyway.
>> >
>> > Once this patch goes in, adding a header in include/uapi/linux that
>> > holds these lengths (and any others that the mount helper and cifs.ko
>> > need to share) would be helpful. Then we could simply look for that
>> > file at autoconf time, and conditionally include it if it exists.
>> >
>> > --
>> > Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>
>>
>> Since we're doing that we might as well standardize on the variable
>> names.  Should we just make the mount helper use the same variable
>> names as the kernel to avoid confusion?  That will be an external
>> change (ie, changing a non-static variable) if anyone linked to the
>> mount helper (doubtful, but possible) and it will break their code.
>>
>
> The variable names aren't terribly important, so I wouldn't go making
> any sort of change like that for "cleanup". What matters here is the
> names of the preprocessor constants. Those will need to be unified
> between userland and kernel in order to make such a scheme work.
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Sorry, I articulated that terribly.  I was referring to the
preprocessor constants such that we can just wrap our current
definitions in #ifndef/#endif.  We're on the same page.

--
Peace and Blessings,
-Scott.