[linux-cifs-client,1/3] cifs: Introduce helper to compute length of nls string in bytes

Commit Message

Jeff Layton April 23, 2009, 5:56 a.m. UTC

On Thu, 23 Apr 2009 02:49:21 +0200
Günter Kukkukk <[e-mail]> wrote:

> just some further notes. 
> With "it's heavily used" i didn't mean the number of callers using this
> function (only 1 in readdir.c) - i meant "the number of times" cifs_convertUCSpath()
> is called in daily usage.... (readdir results)
> 
> The current focus was mostly on cifs_strfromUCS_le() - but the _same_ applies
> to cifs_convertUCSpath()!
> 
> See the following code snippet: 
> 
> readdir.c --> static int cifs_get_name_from_search_buf()
> ....
> 
> 	if (unicode) {
> 		/* BB fixme - test with long names */
> 		/* Note converted filename can be longer than in unicode */
> 		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR)
> 			pqst->len = cifs_convertUCSpath((char *)pqst->name,
> 					(__le16 *)filename, len/2, nlt);
> 		else
> 			pqst->len = cifs_strfromUCS_le((char *)pqst->name,
> 					(__le16 *)filename, len/2, nlt);
> 
> ....

I see what you mean. Good catch. That function also has broken buffer
length checking logic too.

This patch is only compile-tested, but it should fix those problems. In
the long run, we probably need to make all of these functions take an
argument with the length of the destination buffer.

Let's plan that overhaul after Suresh's latest set goes in though.

Patch

From 14cb6c2194c02a9d0197e454b32d75edeb11c050 Mon Sep 17 00:00:00 2001
From: Jeff Layton <[e-mail]>
Date: Thu, 23 Apr 2009 01:51:58 -0400
Subject: [PATCH] cifs: fix length checks and null termination in cifs_convertUCSpath
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

This patch fixes several problems in this function:

cifs_convertUCSpath doesn't yet account for the new buffer size
allocated in cifs_readdir. It doesn't properly terminate the string with
a double-NULL. It only checks whether it's getting close to overrunning
the buffer when it hits a character that needs to be escaped or finds
one that it can't convert.

Reported-by: Günter Kukkukk <[e-mail]>
Signed-off-by: Jeff Layton <[e-mail]>
---
 fs/cifs/misc.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 4c89c57..4896f27 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -654,10 +654,11 @@  int
 cifs_convertUCSpath(char *target, const __le16 *source, int maxlen,
 		    const struct nls_table *cp)
 {
-	int i, j, len;
+	int i, j, len, charlen;
 	__u16 src_char;
 
 	for (i = 0, j = 0; i < maxlen; i++) {
+		charlen = 1;
 		src_char = le16_to_cpu(source[i]);
 		switch (src_char) {
 			case 0:
@@ -689,20 +690,19 @@  cifs_convertUCSpath(char *target, const __le16 *source, int maxlen,
 			default:
 				len = cp->uni2char(src_char, &target[j],
 						NLS_MAX_CHARSET_SIZE);
-				if (len > 0) {
-					j += len;
-					continue;
-				} else {
+				if (len > 0)
+					charlen = len;
+				else
 					target[j] = '?';
-				}
 		}
-		j++;
+		j += charlen;
 		/* make sure we do not overrun callers allocated temp buffer */
-		if (j >= (2 * NAME_MAX))
+		if (j >= (4 * (NAME_MAX - 1)))
 			break;
 	}
 cUCS_out:
 	target[j] = 0;
+	target[j+1] = 0;
 	return j;
 }
 
-- 
1.6.2.2