Re: [PATCH 1/3] cifs: Introduce helper to compute length of nls string in bytes
Jeff Layton <[email protected]>
| Newsgroups | gmane.linux.file-systems.cifs |
|---|---|
| Message-ID | <[email protected]> |
On Thu, 23 Apr 2009 02:49:21 +0200 Günter Kukkukk <[email protected]> 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. -- Jeff Layton <[email protected]> _______________________________________________ linux-cifs-client mailing list [email protected] https://lists.samba.org/mailman/listinfo/linux-cifs-client
0001-cifs-fix-length-checks-and-null-termination-in-cifs.patch
(text/x-patch, 1.8 KB)
From 14cb6c2194c02a9d0197e454b32d75edeb11c050 Mon Sep 17 00:00:00 2001 From: Jeff Layton <[email protected]> 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 <[email protected]> Signed-off-by: Jeff Layton <[email protected]> --- 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