| Message ID | 49EF6758.7020303@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 23 Apr 2009 00:22:08 +0530 Suresh Jayaraman <sjayaraman@suse.de> wrote: > Jeff Layton wrote: > > On Wed, 22 Apr 2009 19:14:16 +0530 > > Suresh Jayaraman <sjayaraman@suse.de> wrote: > > > >> Move the memory allocation from cifs_follow_link to > >> CIFSSMBUnixQuerySymLink and make use of cifs_strlcpy_to_host(). Also > >> cleaned up a bit while at it. > >> > >> - if (pTcon->ses->capabilities & CAP_UNIX) > >> + if (pTcon->ses->capabilities & CAP_UNIX) { > >> rc = CIFSSMBUnixQuerySymLink(xid, pTcon, full_path, > >> target_path, > >> PATH_MAX-1, > >> cifs_sb->local_nls); > >> + if (rc) { > >> + kfree(target_path); > >> + target_path = ERR_PTR(rc); > >> + } > >> + } > > > > > > Ummmm....that won't work. You're now allocating the buffer in > > CIFSSMBUnixQuerySymLink, but target_path is just a normal char pointer. > > Here's the fixed and tested patch: > > > > fs/cifs/cifsproto.h | 2 +- > fs/cifs/cifssmb.c | 26 +++++++++++--------------- > fs/cifs/link.c | 25 +++++++------------------ > 3 files changed, 19 insertions(+), 34 deletions(-) > > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h > index 4167716..46b926d 100644 > --- a/fs/cifs/cifsproto.h > +++ b/fs/cifs/cifsproto.h > @@ -261,7 +261,7 @@ extern int CIFSUnixCreateSymLink(const int xid, > extern int CIFSSMBUnixQuerySymLink(const int xid, > struct cifsTconInfo *tcon, > const unsigned char *searchName, > - char *syminfo, const int buflen, > + char **syminfo, const int buflen, > const struct nls_table *nls_codepage); > extern int CIFSSMBQueryReparseLinkInfo(const int xid, > struct cifsTconInfo *tcon, > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index a02c43b..e64edea 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -2418,7 +2418,7 @@ winCreateHardLinkRetry: > int > CIFSSMBUnixQuerySymLink(const int xid, struct cifsTconInfo *tcon, > const unsigned char *searchName, > - char *symlinkinfo, const int buflen, > + char **symlinkinfo, const int buflen, > const struct nls_table *nls_codepage) > { > /* SMB_QUERY_FILE_UNIX_LINK */ > @@ -2488,24 +2488,20 @@ querySymLinkRetry: > else { > __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset); > __u16 count = le16_to_cpu(pSMBr->t2.DataCount); > + char *src = (char *) &pSMBr->hdr.Protocol + data_offset; > + int src_len = min_t(const int, buflen, count); > > if (pSMBr->hdr.Flags2 & SMBFLG2_UNICODE) { > - name_len = UniStrnlen((wchar_t *) ((char *) > - &pSMBr->hdr.Protocol + data_offset), > - min_t(const int, buflen, count) / 2); > - /* BB FIXME investigate remapping reserved chars here */ > - cifs_strfromUCS_le(symlinkinfo, > - (__le16 *) ((char *)&pSMBr->hdr.Protocol > - + data_offset), > - name_len, nls_codepage); > + rc = cifs_strlcpy_to_host(symlinkinfo, src, > + src_len / 2, 1, nls_codepage); > + cFYI(1, ("symlinkinfo = %s", *symlinkinfo)); > } else { > - strncpy(symlinkinfo, > - (char *) &pSMBr->hdr.Protocol + > - data_offset, > - min_t(const int, buflen, count)); > + *symlinkinfo = kmalloc(src_len + 1, GFP_KERNEL); > + if (*symlinkinfo) > + strlcpy(*symlinkinfo, src, src_len + 1); > + else > + rc = -ENOMEM; > } > - symlinkinfo[buflen] = 0; > - /* just in case so calling code does not go off the end of buffer */ > } > } > cifs_buf_release(pSMB); > diff --git a/fs/cifs/link.c b/fs/cifs/link.c > index 63f6440..03b3793 100644 > --- a/fs/cifs/link.c > +++ b/fs/cifs/link.c > @@ -124,11 +124,6 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd) > cFYI(1, ("Full path: %s inode = 0x%p", full_path, inode)); > cifs_sb = CIFS_SB(inode->i_sb); > pTcon = cifs_sb->tcon; > - target_path = kmalloc(PATH_MAX, GFP_KERNEL); > - if (!target_path) { > - target_path = ERR_PTR(-ENOMEM); > - goto out; > - } > > /* We could change this to: > if (pTcon->unix_ext) > @@ -136,11 +131,16 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd) > get symlink info if we can, even if unix extensions > turned off for this mount */ > > - if (pTcon->ses->capabilities & CAP_UNIX) > + if (pTcon->ses->capabilities & CAP_UNIX) { > rc = CIFSSMBUnixQuerySymLink(xid, pTcon, full_path, > - target_path, > + &target_path, > PATH_MAX-1, > cifs_sb->local_nls); > + if (rc) { > + kfree(target_path); > + target_path = ERR_PTR(rc); > + } > + } > else { > /* BB add read reparse point symlink code here */ > /* rc = CIFSSMBQueryReparseLinkInfo */ > @@ -148,17 +148,6 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd) > /* BB Add MAC style xsymlink check here if enabled */ > } > > - if (rc == 0) { > - > -/* BB Add special case check for Samba DFS symlinks */ > - > - target_path[PATH_MAX-1] = 0; > - } else { > - kfree(target_path); > - target_path = ERR_PTR(rc); > - } > - > -out: > kfree(full_path); > out_no_free: > FreeXid(xid); Much better. Acked-by: Jeff Layton <jlayton@redhat.com> ...you can also add my Acked-by to patches 1-4 in the earlier set as well.
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index 4167716..46b926d 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -261,7 +261,7 @@ extern int CIFSUnixCreateSymLink(const int xid, extern int CIFSSMBUnixQuerySymLink(const int xid, struct cifsTconInfo *tcon, const unsigned char *searchName, - char *syminfo, const int buflen, + char **syminfo, const int buflen, const struct nls_table *nls_codepage); extern int CIFSSMBQueryReparseLinkInfo(const int xid, struct cifsTconInfo *tcon, diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index a02c43b..e64edea 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -2418,7 +2418,7 @@ winCreateHardLinkRetry: int CIFSSMBUnixQuerySymLink(const int xid, struct cifsTconInfo *tcon, const unsigned char *searchName, - char *symlinkinfo, const int buflen, + char **symlinkinfo, const int buflen, const struct nls_table *nls_codepage) { /* SMB_QUERY_FILE_UNIX_LINK */ @@ -2488,24 +2488,20 @@ querySymLinkRetry: else { __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset); __u16 count = le16_to_cpu(pSMBr->t2.DataCount); + char *src = (char *) &pSMBr->hdr.Protocol + data_offset; + int src_len = min_t(const int, buflen, count); if (pSMBr->hdr.Flags2 & SMBFLG2_UNICODE) { - name_len = UniStrnlen((wchar_t *) ((char *) - &pSMBr->hdr.Protocol + data_offset), - min_t(const int, buflen, count) / 2); - /* BB FIXME investigate remapping reserved chars here */ - cifs_strfromUCS_le(symlinkinfo, - (__le16 *) ((char *)&pSMBr->hdr.Protocol - + data_offset), - name_len, nls_codepage); + rc = cifs_strlcpy_to_host(symlinkinfo, src, + src_len / 2, 1, nls_codepage); + cFYI(1, ("symlinkinfo = %s", *symlinkinfo)); } else { - strncpy(symlinkinfo, - (char *) &pSMBr->hdr.Protocol + - data_offset, - min_t(const int, buflen, count)); + *symlinkinfo = kmalloc(src_len + 1, GFP_KERNEL); + if (*symlinkinfo) + strlcpy(*symlinkinfo, src, src_len + 1); + else + rc = -ENOMEM; } - symlinkinfo[buflen] = 0; - /* just in case so calling code does not go off the end of buffer */ } } cifs_buf_release(pSMB); diff --git a/fs/cifs/link.c b/fs/cifs/link.c index 63f6440..03b3793 100644 --- a/fs/cifs/link.c +++ b/fs/cifs/link.c @@ -124,11 +124,6 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd) cFYI(1, ("Full path: %s inode = 0x%p", full_path, inode)); cifs_sb = CIFS_SB(inode->i_sb); pTcon = cifs_sb->tcon; - target_path = kmalloc(PATH_MAX, GFP_KERNEL); - if (!target_path) { - target_path = ERR_PTR(-ENOMEM); - goto out; - } /* We could change this to: if (pTcon->unix_ext) @@ -136,11 +131,16 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd) get symlink info if we can, even if unix extensions turned off for this mount */ - if (pTcon->ses->capabilities & CAP_UNIX) + if (pTcon->ses->capabilities & CAP_UNIX) { rc = CIFSSMBUnixQuerySymLink(xid, pTcon, full_path, - target_path, + &target_path, PATH_MAX-1, cifs_sb->local_nls); + if (rc) { + kfree(target_path); + target_path = ERR_PTR(rc); + } + } else { /* BB add read reparse point symlink code here */ /* rc = CIFSSMBQueryReparseLinkInfo */ @@ -148,17 +148,6 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd) /* BB Add MAC style xsymlink check here if enabled */ } - if (rc == 0) { - -/* BB Add special case check for Samba DFS symlinks */ - - target_path[PATH_MAX-1] = 0; - } else { - kfree(target_path); - target_path = ERR_PTR(rc); - } - -out: kfree(full_path); out_no_free: FreeXid(xid);