[linux-cifs-client] [RFC][PATCH] cifs: add helper to simplify handling of unicode strings and use it

Suresh Jayaraman sjayaraman at suse.de
Sun Apr 12 09:13:57 GMT 2009


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 <sjayaraman at suse.de>
---

 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 <asm/byteorder.h>
 #include <linux/types.h>
 #include <linux/nls.h>
+#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;
 


More information about the linux-cifs-client mailing list