Re: [PATCH 1/2] cifs: Move string length definitions to uapi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 25 Jul 2013 13:35:20 -0400
scott.lovenberg@xxxxxxxxx wrote:

> From: Scott Lovenberg <scott.lovenberg@xxxxxxxxx>
> 
> The string lengths for username, domainname, password and share have
> been moved into their own header file in uapi so the mount helper can
> use autoconf to define the size of the string lengths instead of
> trying to keep the two in sync manually.  The names have also been
> changed to be more easily understood, using the "CIFS" prefix and "LEN"
> suffix rather than "SIZE" which could be confusing.
> 
> Signed-off-by: Scott Lovenberg <scott.lovenberg@xxxxxxxxx>
> ---
>  fs/cifs/AUTHORS                      |  2 ++
>  fs/cifs/cifsencrypt.c                |  2 +-
>  fs/cifs/cifsglob.h                   |  7 ++-----
>  fs/cifs/connect.c                    | 20 ++++++++++----------
>  fs/cifs/sess.c                       | 16 ++++++++--------
>  include/uapi/linux/cifs/cifs_mount.h | 25 +++++++++++++++++++++++++
>  6 files changed, 48 insertions(+), 24 deletions(-)
>  create mode 100644 include/uapi/linux/cifs/cifs_mount.h
> 
> diff --git a/fs/cifs/AUTHORS b/fs/cifs/AUTHORS
> index ea940b1..6804e2d 100644
> --- a/fs/cifs/AUTHORS
> +++ b/fs/cifs/AUTHORS
> @@ -39,6 +39,8 @@ Shaggy (Dave Kleikamp) for innumerable small fs suggestions and some good cleanu
>  Gunter Kukkukk (testing and suggestions for support of old servers)
>  Igor Mammedov (DFS support)
>  Jeff Layton (many, many fixes, as well as great work on the cifs Kerberos code)
> +Scott Lovenberg
> +
>  
>  Test case and Bug Report contributors
>  -------------------------------------
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index bb84d7c..194f9cc 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -421,7 +421,7 @@ find_domain_name(struct cifs_ses *ses, const struct nls_table *nls_cp)
>  		if (blobptr + attrsize > blobend)
>  			break;
>  		if (type == NTLMSSP_AV_NB_DOMAIN_NAME) {
> -			if (!attrsize || attrsize >= MAX_DOMAINNAME_SIZE)
> +			if (!attrsize || attrsize >= CIFS_MAX_DOMAINNAME_LEN)
>  				break;
>  			if (!ses->domainName) {
>  				ses->domainName =
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 64bb4c5..b07b122 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -28,6 +28,7 @@
>  #include "cifsacl.h"
>  #include <crypto/internal/hash.h>
>  #include <linux/scatterlist.h>
> +#include <uapi/linux/cifs/cifs_mount.h>
>  #ifdef CONFIG_CIFS_SMB2
>  #include "smb2pdu.h"
>  #endif
> @@ -41,12 +42,8 @@
>  #define MAX_SES_INFO 2
>  #define MAX_TCON_INFO 4
>  
> -#define MAX_TREE_SIZE (2 + MAX_SERVER_SIZE + 1 + MAX_SHARE_SIZE + 1)
> +#define MAX_TREE_SIZE (2 + MAX_SERVER_SIZE + 1 + CIFS_MAX_SHARE_LEN + 1)
>  #define MAX_SERVER_SIZE 15
		^^^^^^^
This looks wrong too. IIUC, that should be the max length of the
"server" portion of the field. The fact that MAX_TREE_SIZE is pretty
long is likely what papers over this...

The userland helper uses NI_MAXHOST for this field, but that's not
defined in the kernel. Perhaps this should be given a name like
CIFS_NI_MAXHOST, and expanded to the same size as the mount helper uses
(1025)?

> -#define MAX_SHARE_SIZE 80
> -#define MAX_DOMAINNAME_SIZE 256	/* maximum fully qualified domain name (FQDN) */
> -#define MAX_USERNAME_SIZE 256	/* reasonable maximum for current servers */
> -#define MAX_PASSWORD_SIZE 512	/* max for windows seems to be 256 wide chars */
>  
>  #define CIFS_MIN_RCV_POOL 4
>  
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 3f9dcf7..20ac9d4 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1575,8 +1575,8 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (strnlen(string, MAX_USERNAME_SIZE) >
> -							MAX_USERNAME_SIZE) {
> +			if (strnlen(string, CIFS_MAX_USERNAME_LEN) >
> +							CIFS_MAX_USERNAME_LEN) {
>  				printk(KERN_WARNING "CIFS: username too long\n");
>  				goto cifs_parse_mount_err;
>  			}
> @@ -1675,8 +1675,8 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>  			if (string == NULL)
>  				goto out_nomem;
>  
> -			if (strnlen(string, MAX_DOMAINNAME_SIZE)
> -					== MAX_DOMAINNAME_SIZE) {
> +			if (strnlen(string, CIFS_MAX_DOMAINNAME_LEN)
> +					== CIFS_MAX_DOMAINNAME_LEN) {
>  				printk(KERN_WARNING "CIFS: domain name too"
>  						    " long\n");
>  				goto cifs_parse_mount_err;
> @@ -2221,13 +2221,13 @@ static int match_session(struct cifs_ses *ses, struct smb_vol *vol)
>  		/* anything else takes username/password */
>  		if (strncmp(ses->user_name,
>  			    vol->username ? vol->username : "",
> -			    MAX_USERNAME_SIZE))
> +			    CIFS_MAX_USERNAME_LEN))
>  			return 0;
>  		if (strlen(vol->username) != 0 &&
>  		    ses->password != NULL &&
>  		    strncmp(ses->password,
>  			    vol->password ? vol->password : "",
> -			    MAX_PASSWORD_SIZE))
> +			    CIFS_MAX_PASSWORD_LEN))
>  			return 0;
>  	}
>  	return 1;
> @@ -2277,8 +2277,8 @@ cifs_put_smb_ses(struct cifs_ses *ses)
>  
>  #ifdef CONFIG_KEYS
>  
> -/* strlen("cifs:a:") + MAX_DOMAINNAME_SIZE + 1 */
> -#define CIFSCREDS_DESC_SIZE (7 + MAX_DOMAINNAME_SIZE + 1)
> +/* strlen("cifs:a:") + CIFS_MAX_DOMAINNAME_LEN + 1 */
> +#define CIFSCREDS_DESC_SIZE (7 + CIFS_MAX_DOMAINNAME_LEN + 1)
>  
>  /* Populate username and pw fields from keyring if possible */
>  static int
> @@ -2352,7 +2352,7 @@ cifs_set_cifscreds(struct smb_vol *vol, struct cifs_ses *ses)
>  	}
>  
>  	len = delim - payload;
> -	if (len > MAX_USERNAME_SIZE || len <= 0) {
> +	if (len > CIFS_MAX_USERNAME_LEN || len <= 0) {
>  		cifs_dbg(FYI, "Bad value from username search (len=%zd)\n",
>  			 len);
>  		rc = -EINVAL;
> @@ -2369,7 +2369,7 @@ cifs_set_cifscreds(struct smb_vol *vol, struct cifs_ses *ses)
>  	cifs_dbg(FYI, "%s: username=%s\n", __func__, vol->username);
>  
>  	len = key->datalen - (len + 1);
> -	if (len > MAX_PASSWORD_SIZE || len <= 0) {
> +	if (len > CIFS_MAX_PASSWORD_LEN || len <= 0) {
>  		cifs_dbg(FYI, "Bad len for password search (len=%zd)\n", len);
>  		rc = -EINVAL;
>  		kfree(vol->username);
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 57b1537..a0a62db 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -197,7 +197,7 @@ static void unicode_domain_string(char **pbcc_area, struct cifs_ses *ses,
>  		bytes_ret = 0;
>  	} else
>  		bytes_ret = cifs_strtoUTF16((__le16 *) bcc_ptr, ses->domainName,
> -					    MAX_DOMAINNAME_SIZE, nls_cp);
> +					    CIFS_MAX_DOMAINNAME_LEN, nls_cp);
>  	bcc_ptr += 2 * bytes_ret;
>  	bcc_ptr += 2;  /* account for null terminator */
>  
> @@ -226,7 +226,7 @@ static void unicode_ssetup_strings(char **pbcc_area, struct cifs_ses *ses,
>  		*(bcc_ptr+1) = 0;
>  	} else {
>  		bytes_ret = cifs_strtoUTF16((__le16 *) bcc_ptr, ses->user_name,
> -					    MAX_USERNAME_SIZE, nls_cp);
> +					    CIFS_MAX_USERNAME_LEN, nls_cp);
>  	}
>  	bcc_ptr += 2 * bytes_ret;
>  	bcc_ptr += 2; /* account for null termination */
> @@ -246,8 +246,8 @@ static void ascii_ssetup_strings(char **pbcc_area, struct cifs_ses *ses,
>  	/* BB what about null user mounts - check that we do this BB */
>  	/* copy user */
>  	if (ses->user_name != NULL) {
> -		strncpy(bcc_ptr, ses->user_name, MAX_USERNAME_SIZE);
> -		bcc_ptr += strnlen(ses->user_name, MAX_USERNAME_SIZE);
> +		strncpy(bcc_ptr, ses->user_name, CIFS_MAX_USERNAME_LEN);
> +		bcc_ptr += strnlen(ses->user_name, CIFS_MAX_USERNAME_LEN);
>  	}
>  	/* else null user mount */
>  	*bcc_ptr = 0;
> @@ -255,8 +255,8 @@ static void ascii_ssetup_strings(char **pbcc_area, struct cifs_ses *ses,
>  
>  	/* copy domain */
>  	if (ses->domainName != NULL) {
> -		strncpy(bcc_ptr, ses->domainName, MAX_DOMAINNAME_SIZE);
> -		bcc_ptr += strnlen(ses->domainName, MAX_DOMAINNAME_SIZE);
> +		strncpy(bcc_ptr, ses->domainName, CIFS_MAX_DOMAINNAME_LEN);
> +		bcc_ptr += strnlen(ses->domainName, CIFS_MAX_DOMAINNAME_LEN);
>  	} /* else we will send a null domain name
>  	     so the server will default to its own domain */
>  	*bcc_ptr = 0;
> @@ -501,7 +501,7 @@ int build_ntlmssp_auth_blob(unsigned char *pbuffer,
>  	} else {
>  		int len;
>  		len = cifs_strtoUTF16((__le16 *)tmp, ses->domainName,
> -				      MAX_USERNAME_SIZE, nls_cp);
> +				      CIFS_MAX_USERNAME_LEN, nls_cp);
>  		len *= 2; /* unicode is 2 bytes each */
>  		sec_blob->DomainName.BufferOffset = cpu_to_le32(tmp - pbuffer);
>  		sec_blob->DomainName.Length = cpu_to_le16(len);
> @@ -517,7 +517,7 @@ int build_ntlmssp_auth_blob(unsigned char *pbuffer,
>  	} else {
>  		int len;
>  		len = cifs_strtoUTF16((__le16 *)tmp, ses->user_name,
> -				      MAX_USERNAME_SIZE, nls_cp);
> +				      CIFS_MAX_USERNAME_LEN, nls_cp);
>  		len *= 2; /* unicode is 2 bytes each */
>  		sec_blob->UserName.BufferOffset = cpu_to_le32(tmp - pbuffer);
>  		sec_blob->UserName.Length = cpu_to_le16(len);
> diff --git a/include/uapi/linux/cifs/cifs_mount.h b/include/uapi/linux/cifs/cifs_mount.h
> new file mode 100644
> index 0000000..1485781
> --- /dev/null
> +++ b/include/uapi/linux/cifs/cifs_mount.h
> @@ -0,0 +1,25 @@
> +/*
> + *   include/uapi/linux/cifs/cifs_mount.h
> + *
> + *   Author(s): Scott Lovenberg (scott.lovenberg@xxxxxxxxx)
> + *
> + *   This library is free software; you can redistribute it and/or modify
> + *   it under the terms of the GNU Lesser General Public License as published
> + *   by the Free Software Foundation; either version 2.1 of the License, or
> + *   (at your option) any later version.
> + *
> + *   This library is distributed in the hope that it will be useful,
> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> + *   the GNU Lesser General Public License for more details.
> + */
> +#ifndef _CIFS_MOUNT_H
> +#define _CIFS_MOUNT_H
> +
> +/* Max string lengths for cifs mounting options. */
> +#define CIFS_MAX_DOMAINNAME_LEN 256 /* max fully qualified domain name */
> +#define CIFS_MAX_USERNAME_LEN   256 /* reasonable max for current servers */
> +#define CIFS_MAX_PASSWORD_LEN   512 /* Windows max seems to be 256 wide chars */
> +#define CIFS_MAX_SHARE_LEN 80
> +
> +#endif /* _CIFS_MOUNT_H */

...other than the above, this looks reasonable to me.

-- 
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux