From patchwork Thu Apr 23 05:56:19 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 19859 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 n3OIgSui032171 for ; Fri, 24 Apr 2009 18:42:28 GMT Received: from dp.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id CC885163BC9 for ; Thu, 23 Apr 2009 05:57:05 +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=-3.8 required=3.8 tests=AWL,BAYES_00, FORGED_RCVD_HELO,SPF_HELO_PASS,SPF_PASS 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 mx2.redhat.com (mx2.redhat.com [66.187.237.31]) by lists.samba.org (Postfix) with ESMTP id 61295163BCA for ; Thu, 23 Apr 2009 05:56:31 +0000 (GMT) Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n3N5un8L025362; Thu, 23 Apr 2009 01:56:49 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n3N5umTq009097; Thu, 23 Apr 2009 01:56:48 -0400 Received: from tupile.poochiereds.net (vpn-10-10.str.redhat.com [10.32.10.10]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n3N5uj4F024563; Thu, 23 Apr 2009 01:56:46 -0400 Date: Thu, 23 Apr 2009 01:56:19 -0400 From: Jeff Layton To: =?UTF-8?B?R8O8bnRlcg==?= Kukkukk Subject: Re: [linux-cifs-client] [PATCH 1/3] cifs: Introduce helper to compute length of nls string in bytes Message-ID: <20090423015619.42a29615@tupile.poochiereds.net> In-Reply-To: <200904230249.22270.linux@kukkukk.com> References: <49EDBB06.3060409@suse.de> <20090422021726.55a836b5@tupile.poochiereds.net> <200904230206.08030.linux@kukkukk.com> <200904230249.22270.linux@kukkukk.com> Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.58 on 172.16.27.26 Cc: Steve French , linux-cifs-client@lists.samba.org, Suresh Jayaraman 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 On Thu, 23 Apr 2009 02:49:21 +0200 Günter Kukkukk 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. From 14cb6c2194c02a9d0197e454b32d75edeb11c050 Mon Sep 17 00:00:00 2001 From: Jeff Layton 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 Signed-off-by: Jeff Layton --- 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