[linux-cifs-client] try 4: [PATCH] [CIFS] Fixed parsing of mount
options when doing DFS submount
Igor Mammedov
niallain at gmail.com
Thu Oct 23 10:08:58 GMT 2008
Jeff,
Thank you for review.
Fixed all your notes and a couple of other things.
New patch is attached.
Jeff Layton wrote:
> 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);
>
>
--
Best regards,
-------------------------
Igor Mammedov,
niallain "at" gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001--CIFS-Fixed-parsing-of-mount-options-when-doing-DFS.patch
Type: text/x-patch
Size: 8084 bytes
Desc: not available
Url : http://lists.samba.org/archive/linux-cifs-client/attachments/20081023/aa67b4f6/0001--CIFS-Fixed-parsing-of-mount-options-when-doing-DFS.bin
More information about the linux-cifs-client
mailing list