From patchwork Wed Apr 22 18:52:08 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Suresh Jayaraman X-Patchwork-Id: 19409 Received: from lists.samba.org (mail.samba.org [66.70.73.150]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n3MIr73N008836 for ; Wed, 22 Apr 2009 18:53:08 GMT Received: from dp.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id EFE74163C7E for ; Wed, 22 Apr 2009 18:52:45 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.1.7 (2006-10-05) on dp.samba.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.8 tests=AWL, BAYES_00 autolearn=ham version=3.1.7 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from victor.provo.novell.com (victor.provo.novell.com [137.65.250.26]) by lists.samba.org (Postfix) with ESMTP id 7897A163B6E for ; Wed, 22 Apr 2009 18:52:30 +0000 (GMT) Received: from [192.168.2.100] (prv-ext-foundry1.gns.novell.com [137.65.251.240]) by victor.provo.novell.com with ESMTP (TLS encrypted); Wed, 22 Apr 2009 12:52:46 -0600 Message-ID: <49EF6758.7020303@suse.de> Date: Thu, 23 Apr 2009 00:22:08 +0530 From: Suresh Jayaraman User-Agent: Thunderbird 2.0.0.19 (X11/20081227) MIME-Version: 1.0 To: Jeff Layton References: <49EF1F30.1030308@suse.de> <20090422102410.070a9799@tupile.poochiereds.net> In-Reply-To: <20090422102410.070a9799@tupile.poochiereds.net> X-Enigmail-Version: 0.95.7 Cc: Steve French , "linux-cifs-client@lists.samba.org" Subject: [linux-cifs-client] [PATCH 5/5] cifs: Fix symlink info buffer sizing in cifs_follow_link (try #2) X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org Errors-To: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org Jeff Layton wrote: > On Wed, 22 Apr 2009 19:14:16 +0530 > Suresh Jayaraman 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(-) Acked-by: Jeff Layton 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);