Re: [PATCH] cifs: fix composing of mount options for DFS referrals

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

 



On Wed, 22 May 2013 20:54:32 -0400
Jeff Layton <jlayton@xxxxxxxxxx> wrote:

> With the change to ignore the unc= and prefixpath= mount options, there
> is no longer any need to add them to the options string when mounting.
> By the same token, we now need to build a device name that includes the
> prefixpath when mounting.
> 
> To make things neater, the delimiters on the devicename are changed
> to '/' since that's preferred when mounting anyway.
> 
> While we're at it, fix a potential buffer overrun when building the
> mount options string, if the new ip= option is much longer than the
> original one.
> 
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
>  fs/cifs/cifs_dfs_ref.c | 151 +++++++++++++++++++++++++++----------------------
>  fs/cifs/dns_resolve.c  |   4 +-
>  2 files changed, 84 insertions(+), 71 deletions(-)
> 
> diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
> index 8e33ec6..f4a19d1 100644
> --- a/fs/cifs/cifs_dfs_ref.c
> +++ b/fs/cifs/cifs_dfs_ref.c
> @@ -18,6 +18,7 @@
>  #include <linux/slab.h>
>  #include <linux/vfs.h>
>  #include <linux/fs.h>
> +#include <linux/inet.h>
>  #include "cifsglob.h"
>  #include "cifsproto.h"
>  #include "cifsfs.h"
> @@ -48,58 +49,75 @@ void cifs_dfs_release_automount_timer(void)
>  }
>  
>  /**
> - * cifs_get_share_name	-	extracts share name from UNC
> - * @node_name:	pointer to UNC string
> + * cifs_build_devname - build a devicename from a UNC and optional prepath
> + * @nodename:	pointer to UNC string
> + * @prepath:	pointer to prefixpath (or NULL if there isn't one)
>   *
> - * Extracts sharename form full UNC.
> - * i.e. strips from UNC trailing path that is not part of share
> - * name and fixup missing '\' in the beginning of DFS node refferal
> - * if necessary.
> - * Returns pointer to share name on success or ERR_PTR on error.
> - * Caller is responsible for freeing returned string.
> + * Build a new cifs devicename after chasing a DFS referral. Allocate a buffer
> + * big enough to hold the final thing. Copy the UNC from the nodename, trimming
> + * off anything after the sharename. Concatenate the prepath onto the end of it

Sorry, the above comment is wrong and from an earlier version of this
patch. The final version doesn't trim off anything after the sharename.
It just concantenates the unconsumed portion of the fullpath onto the
end of the referral node.

Let me know if you want me to respin this or if I should just do a fix
for the comments on top of this one.

> + * if there is one.
> + *
> + * Returns pointer to the built string, or a ERR_PTR. Caller is responsible
> + * for freeing the returned string.
>   */
> -static char *cifs_get_share_name(const char *node_name)
> +static char *
> +cifs_build_devname(char *nodename, const char *prepath)
>  {
> -	int len;
> -	char *UNC;
> -	char *pSep;
> -
> -	len = strlen(node_name);
> -	UNC = kmalloc(len+2 /*for term null and additional \ if it's missed */,
> -			 GFP_KERNEL);
> -	if (!UNC)
> -		return ERR_PTR(-ENOMEM);
> +	size_t pplen;
> +	size_t unclen;
> +	char *dev;
> +	char *pos;
> +
> +	/* skip over any preceding delimiters */
> +	nodename += strspn(nodename, "\\");
> +	if (!*nodename)
> +		return ERR_PTR(-EINVAL);
>  
> -	/* get share name and server name */
> -	if (node_name[1] != '\\') {
> -		UNC[0] = '\\';
> -		strncpy(UNC+1, node_name, len);
> -		len++;
> -		UNC[len] = 0;
> -	} else {
> -		strncpy(UNC, node_name, len);
> -		UNC[len] = 0;
> -	}
> +	/* get length of UNC and set pos to last char */
> +	unclen = strlen(nodename);
> +	pos = nodename + unclen - 1;
>  
> -	/* find server name end */
> -	pSep = memchr(UNC+2, '\\', len-2);
> -	if (!pSep) {
> -		cifs_dbg(VFS, "%s: no server name end in node name: %s\n",
> -			 __func__, node_name);
> -		kfree(UNC);
> -		return ERR_PTR(-EINVAL);
> +	/* trim off any trailing delimiters */
> +	while(*pos == '\\') {
> +		--pos;
> +		--unclen;
>  	}
>  
> -	/* find sharename end */
> -	pSep++;
> -	pSep = memchr(UNC+(pSep-UNC), '\\', len-(pSep-UNC));
> -	if (pSep) {
> -		/* trim path up to sharename end
> -		 * now we have share name in UNC */
> -		*pSep = 0;
> +	/* allocate a buffer:
> +	 * +2 for preceding "//"
> +	 * +1 for delimiter between UNC and prepath
> +	 * +1 for trailing NULL
> +	 */
> +	pplen = prepath ? strlen(prepath) : 0;
> +	dev = kmalloc(2 + unclen + 1 + pplen + 1, GFP_KERNEL);
> +	if (!dev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	pos = dev;
> +	/* add the initial "//" */
> +	*pos = '/';
> +	++pos;
> +	*pos = '/';
> +	++pos;
> +
> +	/* copy in the UNC portion from referral */
> +	memcpy(pos, nodename, unclen);
> +	pos += unclen;
> +
> +	/* copy the prefixpath remainder (if there is one) */
> +	if (pplen) {
> +		*pos = '/';
> +		++pos;
> +		memcpy(pos, prepath, pplen);
> +		pos += pplen;
>  	}
>  
> -	return UNC;
> +	/* NULL terminator */
> +	*pos = '\0';
> +
> +	convert_delimiter(dev, '/');
> +	return dev;
>  }
>  
>  
> @@ -123,6 +141,7 @@ char *cifs_compose_mount_options(const char *sb_mountdata,
>  {
>  	int rc;
>  	char *mountdata = NULL;
> +	const char *prepath = NULL;
>  	int md_len;
>  	char *tkn_e;
>  	char *srvIP = NULL;
> @@ -132,7 +151,19 @@ char *cifs_compose_mount_options(const char *sb_mountdata,
>  	if (sb_mountdata == NULL)
>  		return ERR_PTR(-EINVAL);
>  
> -	*devname = cifs_get_share_name(ref->node_name);
> +	/* find & copy prefixpath */
> +	tkn_e = strchr(ref->node_name + 2, '\\');
> +	if (tkn_e == NULL) {
> +		/* invalid unc, missing share name*/
> +		rc = -EINVAL;
> +		goto compose_mount_options_err;
> +	}
> +
> +	tkn_e = strchr(tkn_e + 1, '\\');
> +	if (tkn_e || (strlen(fullpath) - ref->path_consumed))
> +		prepath = fullpath + ref->path_consumed;
> +
> +	*devname = cifs_build_devname(ref->node_name, prepath);
>  	if (IS_ERR(*devname)) {
>  		rc = PTR_ERR(*devname);
>  		*devname = NULL;
> @@ -146,12 +177,14 @@ char *cifs_compose_mount_options(const char *sb_mountdata,
>  		goto compose_mount_options_err;
>  	}
>  
> -	/* md_len = strlen(...) + 12 for 'sep+prefixpath='
> -	 * assuming that we have 'unc=' and 'ip=' in
> -	 * the original sb_mountdata
> +	/*
> +	 * In most cases, we'll be building a shorter string than the original,
> +	 * but we do have to assume that the address in the ip= option may be
> +	 * much longer than the original. Add the max length of an address
> +	 * string to the length of the original string to allow for worse case.
>  	 */
> -	md_len = strlen(sb_mountdata) + rc + strlen(ref->node_name) + 12;
> -	mountdata = kzalloc(md_len+1, GFP_KERNEL);
> +	md_len = strlen(sb_mountdata) + INET6_ADDRSTRLEN;
> +	mountdata = kzalloc(md_len + 1, GFP_KERNEL);
>  	if (mountdata == NULL) {
>  		rc = -ENOMEM;
>  		goto compose_mount_options_err;
> @@ -195,26 +228,6 @@ char *cifs_compose_mount_options(const char *sb_mountdata,
>  		strncat(mountdata, &sep, 1);
>  	strcat(mountdata, "ip=");
>  	strcat(mountdata, srvIP);
> -	strncat(mountdata, &sep, 1);
> -	strcat(mountdata, "unc=");
> -	strcat(mountdata, *devname);
> -
> -	/* find & copy prefixpath */
> -	tkn_e = strchr(ref->node_name + 2, '\\');
> -	if (tkn_e == NULL) {
> -		/* invalid unc, missing share name*/
> -		rc = -EINVAL;
> -		goto compose_mount_options_err;
> -	}
> -
> -	tkn_e = strchr(tkn_e + 1, '\\');
> -	if (tkn_e || (strlen(fullpath) - ref->path_consumed)) {
> -		strncat(mountdata, &sep, 1);
> -		strcat(mountdata, "prefixpath=");
> -		if (tkn_e)
> -			strcat(mountdata, tkn_e + 1);
> -		strcat(mountdata, fullpath + ref->path_consumed);
> -	}
>  
>  	/*cifs_dbg(FYI, "%s: parent mountdata: %s\n", __func__, sb_mountdata);*/
>  	/*cifs_dbg(FYI, "%s: submount mountdata: %s\n", __func__, mountdata );*/
> diff --git a/fs/cifs/dns_resolve.c b/fs/cifs/dns_resolve.c
> index e7512e4..7ede730 100644
> --- a/fs/cifs/dns_resolve.c
> +++ b/fs/cifs/dns_resolve.c
> @@ -34,7 +34,7 @@
>  
>  /**
>   * dns_resolve_server_name_to_ip - Resolve UNC server name to ip address.
> - * @unc: UNC path specifying the server
> + * @unc: UNC path specifying the server (with '/' as delimiter)
>   * @ip_addr: Where to return the IP address.
>   *
>   * The IP address will be returned in string form, and the caller is
> @@ -64,7 +64,7 @@ dns_resolve_server_name_to_ip(const char *unc, char **ip_addr)
>  	hostname = unc + 2;
>  
>  	/* Search for server name delimiter */
> -	sep = memchr(hostname, '\\', len);
> +	sep = memchr(hostname, '/', len);
>  	if (sep)
>  		len = sep - hostname;
>  	else


-- 
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