From patchwork Sun Apr 12 09:13:57 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Suresh Jayaraman X-Patchwork-Id: 17786 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 n3C9F6XG030551 for ; Sun, 12 Apr 2009 09:15:06 GMT Received: from dp.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 51763163BC8 for ; Sun, 12 Apr 2009 09:14: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.5 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 43B42163B70 for ; Sun, 12 Apr 2009 09:14:29 +0000 (GMT) Received: from [192.168.2.101] (prv-ext-foundry1.gns.novell.com [137.65.251.240]) by victor.provo.novell.com with ESMTP (TLS encrypted); Sun, 12 Apr 2009 03:14:32 -0600 Message-ID: <49E1B0D5.7010600@suse.de> Date: Sun, 12 Apr 2009 14:43:57 +0530 From: Suresh Jayaraman User-Agent: Thunderbird 2.0.0.19 (X11/20081227) MIME-Version: 1.0 To: Steve French X-Enigmail-Version: 0.95.7 Cc: "linux-cifs-client@lists.samba.org" , Jeff Layton Subject: [linux-cifs-client] [RFC][PATCH] cifs: add helper to simplify handling of unicode strings and use it 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 Based on the recent discussions on opencoded, inconsistent allocation of memory needed for a unicode string conversion, the consensus is to calculate the memory needed exactly instead of using size assumptions and to consolidate some code that allocates memory and does conversion in a helper function and use it widely. This patch attempts to do the same. Please note that non-unicode cases has not been converted now as they are less error-prone than unicode cases. I've only compile tested this patch and yet to test it completely, but posting this little premature patch as there are lot of discussions happening around this code. This patch requires careful review. Thanks to Jeff, Steve and Simo for useful suggestions. Signed-off-by: Suresh Jayaraman --- fs/cifs/cifs_unicode.h | 22 ++++++++++++++++++++ fs/cifs/cifsproto.h | 2 + fs/cifs/cifssmb.c | 38 +++++++++++++++++++++++++++++++++++ fs/cifs/connect.c | 51 +++++++++++++++-------------------------------- fs/cifs/sess.c | 36 +++++++++++---------------------- 5 files changed, 90 insertions(+), 59 deletions(-) diff --git a/fs/cifs/cifs_unicode.h b/fs/cifs/cifs_unicode.h index 14eb9a2..43c0438 100644 --- a/fs/cifs/cifs_unicode.h +++ b/fs/cifs/cifs_unicode.h @@ -34,6 +34,7 @@ #include #include #include +#include "cifspdu.h" #define UNIUPR_NOLOWER /* Example to not expand lower case tables */ @@ -63,6 +64,7 @@ int cifs_strfromUCS_le(char *, const __le16 *, int, const struct nls_table *); int cifs_strtoUCS(__le16 *, const char *, int, const struct nls_table *); #endif +#define UNISTR_MAX_SIZE (MAX_PATHCONF * (NLS_MAX_CHARSET_SIZE + 1)) /* * UniStrcat: Concatenate the second string to the first * @@ -159,6 +161,26 @@ UniStrnlen(const wchar_t *ucs1, int maxlen) } /* + * UniStrnlenBytes: Return the length in bytes + */ +static inline int +UniStrnlenBytes(const char *str, int maxlen, const struct nls_table *codepage) +{ + int rc; + wchar_t uni; + size_t nbytes = 0; + char buf[UNISTR_MAX_SIZE]; + + while (*str++) { + rc = codepage->char2uni(str, maxlen, &uni); + if (rc == -1) + return -EINVAL; + nbytes += utf8_wcstombs(buf, &uni, maxlen); + } + return nbytes; +} + +/* * UniStrncat: Concatenate length limited string */ static inline wchar_t * diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index 4167716..d154ed1 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -383,4 +383,6 @@ extern int CIFSSMBSetPosixACL(const int xid, struct cifsTconInfo *tcon, const struct nls_table *nls_codepage, int remap_special_chars); extern int CIFSGetExtAttr(const int xid, struct cifsTconInfo *tcon, const int netfid, __u64 *pExtAttrBits, __u64 *pMask); +extern int cifs_nls_to_str(char **dst, const char *src, const int maxlen, + int *plen, const struct nls_table *nls_codepage); #endif /* _CIFSPROTO_H */ diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index bc09c99..963f4f9 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -81,6 +81,44 @@ static struct { #endif /* CONFIG_CIFS_WEAK_PW_HASH */ #endif /* CIFS_POSIX */ + +/* + * Calculates, allocates memory for nls and converts it to character string. + * Note: caller is responsible for freeing dst if function returned 0. + * returns: + * on success - 0 + * on failure - errno + */ +int +cifs_nls_to_str(char **dst, const char *src, const int maxlen, int *plen, + const struct nls_table *nls_codepage) +{ + size_t nbytes; + + *plen = UniStrnlen((wchar_t *)src, maxlen); + nbytes = UniStrnlenBytes(src, maxlen, nls_codepage); + + /* We look for obvious messed up bcc or strings in response so we do + * not go off the end since (at least) WIN2K and * Windows XP have a + * major bug in not null terminating last Unicode string in response + */ + kfree(*dst); + + *dst = kzalloc(nbytes + 2, GFP_KERNEL); + if (!*dst) + goto err_exit; + + cifs_strfromUCS_le(*dst, (__le16 *)src, *plen, nls_codepage); + (*dst)[*plen] = 0; + (*dst)[*plen + 1] = 0; /* harmless for ASCII case, needed for Unicode */ + + return 0; + +err_exit: + cERROR(1, ("Failed to allocate buffer for string\n")); + return -ENOMEM; +} + /* Allocates buffer into dst and copies smb string from src to it. * caller is responsible for freeing dst if function returned 0. * returns: diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 0de3b56..1880537 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -2667,36 +2667,23 @@ CIFSSessSetup(unsigned int xid, struct cifsSesInfo *ses, remaining_words = BCC(smb_buffer_response) / 2; } - len = - UniStrnlen((wchar_t *) bcc_ptr, - remaining_words - 1); -/* We look for obvious messed up bcc or strings in response so we do not go off - the end since (at least) WIN2K and Windows XP have a major bug in not null - terminating last Unicode string in response */ - if (ses->serverOS) - kfree(ses->serverOS); - ses->serverOS = kzalloc(2 * (len + 1), - GFP_KERNEL); - if (ses->serverOS == NULL) + rc = cifs_nls_to_str(&(ses->serverOS), + bcc_ptr, remaining_words - 1, + &len, nls_codepage); + if (rc) goto sesssetup_nomem; - cifs_strfromUCS_le(ses->serverOS, - (__le16 *)bcc_ptr, - len, nls_codepage); bcc_ptr += 2 * (len + 1); remaining_words -= len + 1; ses->serverOS[2 * len] = 0; ses->serverOS[1 + (2 * len)] = 0; if (remaining_words > 0) { - len = UniStrnlen((wchar_t *)bcc_ptr, - remaining_words-1); - kfree(ses->serverNOS); - ses->serverNOS = kzalloc(2 * (len + 1), - GFP_KERNEL); - if (ses->serverNOS == NULL) + rc = cifs_nls_to_str( + &(ses->serverNOS), + bcc_ptr, + remaining_words - 1, + &len, nls_codepage); + if (rc) goto sesssetup_nomem; - cifs_strfromUCS_le(ses->serverNOS, - (__le16 *)bcc_ptr, - len, nls_codepage); bcc_ptr += 2 * (len + 1); ses->serverNOS[2 * len] = 0; ses->serverNOS[1 + (2 * len)] = 0; @@ -2707,19 +2694,13 @@ CIFSSessSetup(unsigned int xid, struct cifsSesInfo *ses, } remaining_words -= len + 1; if (remaining_words > 0) { - len = UniStrnlen((wchar_t *) bcc_ptr, remaining_words); - /* last string is not always null terminated - (for e.g. for Windows XP & 2000) */ - if (ses->serverDomain) - kfree(ses->serverDomain); - ses->serverDomain = - kzalloc(2*(len+1), - GFP_KERNEL); - if (ses->serverDomain == NULL) + rc = cifs_nls_to_str( + &(ses->serverDomain), + bcc_ptr, + remaining_words, &len, + nls_codepage); + if (rc) goto sesssetup_nomem; - cifs_strfromUCS_le(ses->serverDomain, - (__le16 *)bcc_ptr, - len, nls_codepage); bcc_ptr += 2 * (len + 1); ses->serverDomain[2*len] = 0; ses->serverDomain[1+(2*len)] = 0; diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index 5c68b42..1e82b22 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -301,33 +301,26 @@ static int decode_unicode_ssetup(char **pbcc_area, int bleft, words_left = bleft / 2; /* save off server operating system */ - len = UniStrnlen((wchar_t *) data, words_left); + rc = cifs_nls_to_str(&(ses->serverOS), data, words_left, &len, + nls_cp); + if (rc) + return rc; -/* We look for obvious messed up bcc or strings in response so we do not go off - the end since (at least) WIN2K and Windows XP have a major bug in not null - terminating last Unicode string in response */ if (len >= words_left) return rc; - kfree(ses->serverOS); - /* UTF-8 string will not grow more than four times as big as UCS-16 */ - ses->serverOS = kzalloc((4 * len) + 2 /* trailing null */, GFP_KERNEL); - if (ses->serverOS != NULL) - cifs_strfromUCS_le(ses->serverOS, (__le16 *)data, len, nls_cp); data += 2 * (len + 1); words_left -= len + 1; /* save off server network operating system */ - len = UniStrnlen((wchar_t *) data, words_left); + rc = cifs_nls_to_str(&(ses->serverNOS), data, words_left, &len, + nls_cp); + if (rc) + return rc; if (len >= words_left) return rc; - - kfree(ses->serverNOS); - ses->serverNOS = kzalloc((4 * len) + 2 /* trailing null */, GFP_KERNEL); if (ses->serverNOS != NULL) { - cifs_strfromUCS_le(ses->serverNOS, (__le16 *)data, len, - nls_cp); if (strncmp(ses->serverNOS, "NT LAN Manager 4", 16) == 0) { cFYI(1, ("NT4 server")); ses->flags |= CIFS_SES_NT4; @@ -337,19 +330,14 @@ static int decode_unicode_ssetup(char **pbcc_area, int bleft, words_left -= len + 1; /* save off server domain */ - len = UniStrnlen((wchar_t *) data, words_left); + rc = cifs_nls_to_str(&(ses->serverDomain), data, words_left, + &len, nls_cp); + if (rc) + return rc; if (len > words_left) return rc; - kfree(ses->serverDomain); - ses->serverDomain = kzalloc(2 * (len + 1), GFP_KERNEL); /* BB FIXME wrong length */ - if (ses->serverDomain != NULL) { - cifs_strfromUCS_le(ses->serverDomain, (__le16 *)data, len, - nls_cp); - ses->serverDomain[2*len] = 0; - ses->serverDomain[(2*len) + 1] = 0; - } data += 2 * (len + 1); words_left -= len + 1;