[linux-cifs-client] [PATCH 5/5] cifs: Fix symlink info buffer sizing in cifs_follow_link (try #2)

[Suresh Jayaraman]

Jeff Layton wrote:
> On Wed, 22 Apr 2009 19:14:16 +0530
> Suresh Jayaraman <sjayaraman at 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);