[linux-cifs-client] try 3: [PATCH] [CIFS] Fixed parsing of
mount options when doing DFS submount
Jeff Layton
jlayton at redhat.com
Tue Oct 21 16:32:20 GMT 2008
On Thu, 02 Oct 2008 17:17:36 +0400
Igor Mammedov <niallain at gmail.com> wrote:
> >From d2dac33f0eb08ef2a7d05e1aa8d9fa4f7eb8c1c4 Mon Sep 17 00:00:00 2001
> From: Igor Mammedov <niallain at gmail.com>
> Date: Thu, 2 Oct 2008 17:07:29 +0400
> Subject: [PATCH] [CIFS] Fixed parsing of mount options when doing DFS submount
> Fixed: Incorrect handling of the last option in some cases
> Fixed: prefixpath handling
> Convert path_consumed into host depended string length (in bytes)
>
> Signed-off-by: Igor Mammedov <niallain at gmail.com>
> ---
I'm still wrapping my brain around the DFS code, so I'll defer to your understanding of the functional aspects of this. Some (fairly trivial) comments inlined below:
> fs/cifs/cifs_dfs_ref.c | 65 +++++++++++++++++++++++++++++++-----------------
> fs/cifs/cifssmb.c | 39 ++++++++++++++++++++++++++--
> 2 files changed, 78 insertions(+), 26 deletions(-)
>
> diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
> index d2c8eef..55f0f01 100644
> --- a/fs/cifs/cifs_dfs_ref.c
> +++ b/fs/cifs/cifs_dfs_ref.c
> @@ -106,7 +106,8 @@ static char *cifs_get_share_name(const char *node_name)
> /**
> * compose_mount_options - creates mount options for refferral
> * @sb_mountdata: parent/root DFS mount options (template)
> - * @ref_unc: refferral server UNC
> + * @dentry: point where we are going to mount
> + * @ref: server's refferral
> * @devname: pointer for saving device name
> *
> * creates mount options for submount based on template options sb_mountdata
> @@ -116,7 +117,8 @@ static char *cifs_get_share_name(const char *node_name)
> * Caller is responcible for freeing retunrned value if it is not error.
> */
> static char *compose_mount_options(const char *sb_mountdata,
> - const char *ref_unc,
> + struct dentry *dentry,
> + const struct dfs_info3_param *ref,
> char **devname)
> {
> int rc;
> @@ -126,11 +128,12 @@ static char *compose_mount_options(const char *sb_mountdata,
> char *srvIP = NULL;
> char sep = ',';
> int off, noff;
> + char *fullpath;
>
> if (sb_mountdata == NULL)
> return ERR_PTR(-EINVAL);
>
> - *devname = cifs_get_share_name(ref_unc);
> + *devname = cifs_get_share_name(ref->node_name);
> rc = dns_resolve_server_name_to_ip(*devname, &srvIP);
> if (rc != 0) {
> cERROR(1, ("%s: Failed to resolve server part of %s to IP",
> @@ -138,7 +141,8 @@ static char *compose_mount_options(const char *sb_mountdata,
> mountdata = ERR_PTR(rc);
> goto compose_mount_options_out;
> }
> - md_len = strlen(sb_mountdata) + strlen(srvIP) + strlen(ref_unc) + 3;
> + md_len = strlen(sb_mountdata) + strlen(srvIP) +
> + strlen(ref->node_name) + 3;
Some comments about the size of this allocation would be nice. Why +3 here?
> mountdata = kzalloc(md_len+1, GFP_KERNEL);
> if (mountdata == NULL) {
> mountdata = ERR_PTR(-ENOMEM);
> @@ -151,42 +155,57 @@ static char *compose_mount_options(const char *sb_mountdata,
> sep = sb_mountdata[4];
> strncpy(mountdata, sb_mountdata, 5);
> off += 5;
> + } else { /* set default sep if not found */
> + sep = ',';
^^^
If you do this, you don't need to set sep in the declaration.
> }
> - while ((tkn_e = strchr(sb_mountdata+off, sep))) {
> - noff = (tkn_e - (sb_mountdata+off)) + 1;
> - if (strnicmp(sb_mountdata+off, "unc=", 4) == 0) {
> +
> + do {
> + tkn_e = strchr(sb_mountdata + off, sep);
> + if (tkn_e == NULL)
> + noff = strlen(sb_mountdata + off);
> + else
> + noff = tkn_e - (sb_mountdata + off) + 1;
> +
> + if (strnicmp(sb_mountdata + off, "unc=", 4) == 0) {
> off += noff;
> continue;
> }
> - if (strnicmp(sb_mountdata+off, "ip=", 3) == 0) {
> + if (strnicmp(sb_mountdata + off, "ip=", 3) == 0) {
> off += noff;
> continue;
> }
> - if (strnicmp(sb_mountdata+off, "prefixpath=", 3) == 0) {
> + if (strnicmp(sb_mountdata + off, "prefixpath=", 3) == 0) {
^^^^
Shouldn't this be bigger?
> off += noff;
> continue;
> }
> - strncat(mountdata, sb_mountdata+off, noff);
> + strncat(mountdata, sb_mountdata + off, noff);
> off += noff;
> - }
> - strcat(mountdata, sb_mountdata+off);
> + } while (tkn_e);
> + strcat(mountdata, sb_mountdata + off);
> mountdata[md_len] = '\0';
>
> /* copy new IP and ref share name */
> - strcat(mountdata, ",ip=");
> + if (mountdata[strlen(mountdata) - 1] != ',')
> + strcat(mountdata, ",");
> + strcat(mountdata, "ip=");
> strcat(mountdata, srvIP);
> strcat(mountdata, ",unc=");
> strcat(mountdata, *devname);
>
> /* find & copy prefixpath */
> - tkn_e = strchr(ref_unc+2, '\\');
> - if (tkn_e) {
> - tkn_e = strchr(tkn_e+1, '\\');
> - if (tkn_e) {
> - strcat(mountdata, ",prefixpath=");
> - strcat(mountdata, tkn_e+1);
> - }
> + tkn_e = strchr(ref->node_name + 2, '\\');
> + if (tkn_e == NULL) /* invalid unc, missing share name*/
> + goto compose_mount_options_out;
> +
> + fullpath = build_path_from_dentry(dentry);
> + tkn_e = strchr(tkn_e + 1, '\\');
> + if (tkn_e || strlen(fullpath) - (ref->path_consumed)) {
> + strcat(mountdata, ",prefixpath=");
> + if (tkn_e)
> + strcat(mountdata, tkn_e + 1);
> + strcat(mountdata, fullpath + (ref->path_consumed));
> }
> + kfree(fullpath);
>
> /*cFYI(1,("%s: parent mountdata: %s", __func__,sb_mountdata));*/
> /*cFYI(1, ("%s: submount mountdata: %s", __func__, mountdata ));*/
> @@ -198,7 +217,7 @@ compose_mount_options_out:
>
>
> static struct vfsmount *cifs_dfs_do_refmount(const struct vfsmount *mnt_parent,
> - struct dentry *dentry, char *ref_unc)
> + struct dentry *dentry, const struct dfs_info3_param *ref)
> {
> struct cifs_sb_info *cifs_sb;
> struct vfsmount *mnt;
> @@ -207,7 +226,7 @@ static struct vfsmount *cifs_dfs_do_refmount(const struct vfsmount *mnt_parent,
>
> cifs_sb = CIFS_SB(dentry->d_inode->i_sb);
> mountdata = compose_mount_options(cifs_sb->mountdata,
> - ref_unc, &devname);
> + dentry, ref, &devname);
>
> if (IS_ERR(mountdata))
> return (struct vfsmount *)mountdata;
> @@ -310,7 +329,7 @@ cifs_dfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd)
> }
> mnt = cifs_dfs_do_refmount(nd->path.mnt,
> nd->path.dentry,
> - referrals[i].node_name);
> + referrals + i);
> cFYI(1, ("%s: cifs_dfs_do_refmount:%s , mnt:%p",
> __func__,
> referrals[i].node_name, mnt));
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 7504d15..9577c10 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -3895,6 +3895,27 @@ GetInodeNumOut:
> return rc;
> }
>
> +/* computes length of UCS string converted to host codepage
> + * @src: UCS string
> + * @maxlen: length of the input string in UCS characters
> + * (not in bytes)
> + *
> + * return: size of input string in host codepage
> + */
> +static int hostlen_fromUCS(const __le16 *src, const int maxlen,
> + const struct nls_table *nls_codepage) {
> + int i;
> + int hostlen = 0;
> + char to[4];
> + int charlen;
> + for (i = 0; (i < maxlen) && src[i]; ++i) {
> + charlen = nls_codepage->uni2char(le16_to_cpu(src[i]),
> + to, NLS_MAX_CHARSET_SIZE);
> + hostlen += charlen > 0 ? charlen : 1;
> + }
> + return hostlen;
> +}
> +
> /* parses DFS refferal V3 structure
> * caller is responsible for freeing target_nodes
> * returns:
> @@ -3905,7 +3926,8 @@ static int
> parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,
> unsigned int *num_of_nodes,
> struct dfs_info3_param **target_nodes,
> - const struct nls_table *nls_codepage)
> + const struct nls_table *nls_codepage, int remap,
> + const char *searchName)
> {
> int i, rc = 0;
> char *data_end;
> @@ -3956,7 +3978,17 @@ parse_DFS_referrals(TRANSACTION2_GET_DFS_REFER_RSP *pSMBr,
> struct dfs_info3_param *node = (*target_nodes)+i;
>
> node->flags = le16_to_cpu(pSMBr->DFSFlags);
> - node->path_consumed = le16_to_cpu(pSMBr->PathConsumed);
> + if (is_unicode) {
> + __le16 *tmp = kmalloc(strlen(searchName)*2, GFP_KERNEL);
> + cifsConvertToUCS((__le16 *) tmp, searchName,
> + PATH_MAX, nls_codepage, remap);
> + node->path_consumed = hostlen_fromUCS(tmp,
> + le16_to_cpu(pSMBr->PathConsumed)/2,
> + nls_codepage);
> + kfree(tmp);
> + } else
> + node->path_consumed = le16_to_cpu(pSMBr->PathConsumed);
> +
> node->server_type = le16_to_cpu(ref->ServerType);
> node->ref_flag = le16_to_cpu(ref->ReferralEntryFlags);
>
> @@ -4089,7 +4121,8 @@ getDFSRetry:
>
> /* parse returned result into more usable form */
> rc = parse_DFS_referrals(pSMBr, num_of_nodes,
> - target_nodes, nls_codepage);
> + target_nodes, nls_codepage, remap,
> + searchName);
>
> GetDFSRefExit:
> cifs_buf_release(pSMB);
--
Jeff Layton <jlayton at redhat.com>
More information about the linux-cifs-client
mailing list