[linux-cifs-client] PATCH: alignment changes for CIFS VFS

Commit Message

Craig Matsuura June 26, 2009, 2:18 a.m. UTC

I saw the following patch at http://patchwork.kernel.org/patch/23910/.  I 
applied this patch to help me resolve some issues I was having with the CIFS 
VFS.  So far so good.  I have gone as far as to make several more changes and 
wanted to contribute back the changes and have some review.

I am on a 2.6.28 kernel on a arm9 based system using a gcc-4.3.2 cross 
compiler.  I have been experiencing alignment traps on my system (I viewed 
them from the /proc/cpu/alignment).  To track them down I commented out the 
alignment handler in the arch/arm/mm/alignment.c. (Did this to just track down 
the unalignments)

I have made several changes to the CIFS VFS and I have a patch (that includes 
the patch above).  My patch fixed several unalignments I was seeing in my 
kernel.

c4davinci-cifs-alignment.patch
 
@@ -260,13 +260,10 @@ static int decode_unicode_ssetup(char **
 		return rc;
 
 	kfree(ses->serverDomain);
-	ses->serverDomain = kzalloc(2 * (len + 1), GFP_KERNEL); /* BB FIXME wrong 
length */
-	if (ses->serverDomain != NULL) {
+	ses->serverDomain = kzalloc((4 * len) + 2, GFP_KERNEL);
+	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;
 
@@ -575,7 +572,8 @@ CIFS_SessSetup(unsigned int xid, struct 
 	count = iov[1].iov_len + iov[2].iov_len;
 	smb_buf->smb_buf_length += count;
 
-	BCC_LE(smb_buf) = cpu_to_le16(count);
+	/* BCC_LE(smb_buf) = cpu_to_le16(count); */
+	put_unaligned_le16(count,(char *)smb_buf + sizeof(struct smb_hdr) + (2 * 
smb_buf->WordCount));
 
 	rc = SendReceive2(xid, ses, iov, 3 /* num_iovecs */, &resp_buf_type,
 			  CIFS_STD_OP /* not long */ | CIFS_LOG_ERROR);
@@ -600,7 +598,8 @@ CIFS_SessSetup(unsigned int xid, struct 
 	cFYI(1, ("UID = %d ", ses->Suid));
 	/* response can have either 3 or 4 word count - Samba sends 3 */
 	/* and lanman response is 3 */
-	bytes_remaining = BCC(smb_buf);
+	/*bytes_remaining = BCC(smb_buf);*/
+	bytes_remaining = get_unaligned((int *) ((char *)smb_buf + sizeof(struct 
smb_hdr) + (2 * smb_buf->WordCount)));
 	bcc_ptr = pByteArea(smb_buf);
 
 	if (smb_buf->WordCount == 4) {
@@ -616,12 +615,18 @@ CIFS_SessSetup(unsigned int xid, struct 
 	}
 
 	/* BB check if Unicode and decode strings */
-	if (smb_buf->Flags2 & SMBFLG2_UNICODE)
+	if (smb_buf->Flags2 & SMBFLG2_UNICODE) {
+		/* unicode string area must be word-aligned */
+		if (((unsigned long) bcc_ptr - (unsigned long) smb_buf) % 2) {
+			++bcc_ptr;
+			--bytes_remaining;
+		}
 		rc = decode_unicode_ssetup(&bcc_ptr, bytes_remaining,
 						   ses, nls_cp);
-	else
+	} else {
 		rc = decode_ascii_ssetup(&bcc_ptr, bytes_remaining,
 					 ses, nls_cp);
+	}
 
 ssetup_exit:
 	if (spnego_key) {

Patch

diff -rupw a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
--- a/fs/cifs/cifssmb.c	2009-06-25 16:55:30.000000000 -0600
+++ b/fs/cifs/cifssmb.c	2009-06-25 16:20:45.000000000 -0600
@@ -32,6 +32,7 @@ 
 #include <linux/vfs.h>
 #include <linux/posix_acl_xattr.h>
 #include <asm/uaccess.h>
+#include <asm/unaligned.h>
 #include "cifspdu.h"
 #include "cifsglob.h"
 #include "cifsacl.h"
@@ -431,7 +432,8 @@  static int validate_t2(struct smb_t2_rsp
 				pBCC = (pSMB->hdr.WordCount * 2) +
 					sizeof(struct smb_hdr) +
 					(char *)pSMB;
-				if ((total_size <= (*(u16 *)pBCC)) &&
+/*printk("*pBCC=%x, get_unaligned=%x\n",(*(u16 *)pBCC), get_unaligned((u16 *) 
pBCC));*/
+				if ((total_size <=  get_unaligned_le16((u16 *)pBCC) /*(*(u16 *)pBCC)*/) 
&&
 				   (total_size <
 					CIFSMaxBufSize+MAX_CIFS_HDR_SIZE)) {
 					return 0;
diff -rupw a/fs/cifs/connect.c b/fs/cifs/connect.c
--- a/fs/cifs/connect.c	2009-06-25 16:55:30.000000000 -0600
+++ b/fs/cifs/connect.c	2009-06-25 16:32:02.000000000 -0600
@@ -35,6 +35,7 @@ 
 #include <linux/freezer.h>
 #include <asm/uaccess.h>
 #include <asm/processor.h>
+#include <asm/unaligned.h>
 #include "cifspdu.h"
 #include "cifsglob.h"
 #include "cifsproto.h"
@@ -2548,13 +2549,13 @@  CIFSSessSetup(unsigned int xid, struct c
 			if (smb_buffer->Flags2 & SMBFLG2_UNICODE) {
 				if ((long) (bcc_ptr) % 2) {
 					remaining_words =
-					    (BCC(smb_buffer_response) - 1) / 2;
+					    (get_unaligned((u16 *) ((char *)smb_buffer_response + sizeof(struct 
smb_hdr) + (2 * smb_buffer_response->WordCount)))/*BCC(smb_buffer_response)*/ - 
1) / 2;
 					/* Unicode strings must be word
 					   aligned */
 					bcc_ptr++;
 				} else {
 					remaining_words =
-						BCC(smb_buffer_response) / 2;
+						get_unaligned((u16 *) ((char *)smb_buffer_response + sizeof(struct 
smb_hdr) + (2 * smb_buffer_response->WordCount)))/*BCC(smb_buffer_response)*/ / 
2;
 				}
 				len =
 				    UniStrnlen((wchar_t *) bcc_ptr,
@@ -2636,7 +2637,7 @@  CIFSSessSetup(unsigned int xid, struct c
 				len = strnlen(bcc_ptr, 1024);
 				if (((long) bcc_ptr + len) - (long)
 				    pByteArea(smb_buffer_response)
-					    <= BCC(smb_buffer_response)) {
+					    <= get_unaligned((u16 *) ((char *)smb_buffer_response + sizeof(struct 
smb_hdr) + (2 * smb_buffer_response->WordCount)))/*BCC(smb_buffer_response)*/) {
 					kfree(ses->serverOS);
 					ses->serverOS = kzalloc(len + 1,
 								GFP_KERNEL);
@@ -2890,7 +2891,7 @@  CIFSNTLMSSPNegotiateSessSetup(unsigned i
 				if (smb_buffer->Flags2 & SMBFLG2_UNICODE) {
 					if ((long) (bcc_ptr) % 2) {
 						remaining_words =
-						    (BCC(smb_buffer_response)
+						    (get_unaligned((u16 *) ((char *)smb_buffer_response + sizeof(struct 
smb_hdr) + (2 * smb_buffer_response->WordCount)))/*BCC(smb_buffer_response)*/
 						     - 1) / 2;
 					 /* Must word align unicode strings */
 						bcc_ptr++;
@@ -2977,7 +2978,7 @@  CIFSNTLMSSPNegotiateSessSetup(unsigned i
 					len = strnlen(bcc_ptr, 1024);
 					if (((long) bcc_ptr + len) - (long)
 					    pByteArea(smb_buffer_response)
-					    <= BCC(smb_buffer_response)) {
+					    <= get_unaligned((u16 *) ((char *)smb_buffer_response + sizeof(struct 
smb_hdr) + (2 * smb_buffer_response->WordCount)))/*BCC(smb_buffer_response)*/) {
 						if (ses->serverOS)
 							kfree(ses->serverOS);
 						ses->serverOS =
@@ -3296,11 +3297,11 @@  CIFSNTLMSSPAuthSessSetup(unsigned int xi
 				if (smb_buffer->Flags2 & SMBFLG2_UNICODE) {
 					if ((long) (bcc_ptr) % 2) {
 						remaining_words =
-						    (BCC(smb_buffer_response)
+						    (get_unaligned((u16 *) ((char *)smb_buffer_response + sizeof(struct 
smb_hdr) + (2 * smb_buffer_response->WordCount)))/*BCC(smb_buffer_response)*/
 						     - 1) / 2;
 						bcc_ptr++;	/* Unicode strings must be word aligned */
 					} else {
-						remaining_words = BCC(smb_buffer_response) / 2;
+						remaining_words = get_unaligned((u16 *) ((char *)smb_buffer_response + 
sizeof(struct smb_hdr) + (2 * smb_buffer_response-
>WordCount)))/*BCC(smb_buffer_response)*/ / 2;
 					}
 					len = UniStrnlen((wchar_t *) bcc_ptr,
 							remaining_words - 1);
@@ -3384,7 +3385,7 @@  CIFSNTLMSSPAuthSessSetup(unsigned int xi
 					len = strnlen(bcc_ptr, 1024);
 					if (((long) bcc_ptr + len) -
 					   (long) pByteArea(smb_buffer_response)
-						<= BCC(smb_buffer_response)) {
+						<= get_unaligned((u16 *) ((char *)smb_buffer_response + sizeof(struct 
smb_hdr) + (2 * smb_buffer_response->WordCount)))/*BCC(smb_buffer_response)*/) {
 						if (ses->serverOS)
 							kfree(ses->serverOS);
 						ses->serverOS = kzalloc(len + 1, GFP_KERNEL);
@@ -3542,7 +3543,7 @@  CIFSTCon(unsigned int xid, struct cifsSe
 		tcon->need_reconnect = false;
 		tcon->tid = smb_buffer_response->Tid;
 		bcc_ptr = pByteArea(smb_buffer_response);
-		length = strnlen(bcc_ptr, BCC(smb_buffer_response) - 2);
+		length = strnlen(bcc_ptr, get_unaligned((u16 *) ((char *)smb_buffer_response 
+ sizeof(struct smb_hdr) + (2 * smb_buffer_response-
>WordCount)))/*BCC(smb_buffer_response)*/ - 2);
 		/* skip service field (NB: this field is always ASCII) */
 		if (length == 3) {
 			if ((bcc_ptr[0] == 'I') && (bcc_ptr[1] == 'P') &&
@@ -3562,7 +3563,7 @@  CIFSTCon(unsigned int xid, struct cifsSe
 			length = UniStrnlen((wchar_t *) bcc_ptr, 512);
 			if ((bcc_ptr + (2 * length)) -
 			     pByteArea(smb_buffer_response) <=
-			    BCC(smb_buffer_response)) {
+			    get_unaligned((u16 *) ((char *)smb_buffer_response + sizeof(struct 
smb_hdr) + (2 * smb_buffer_response->WordCount)))/*BCC(smb_buffer_response)*/) {
 				kfree(tcon->nativeFileSystem);
 				tcon->nativeFileSystem =
 				    kzalloc(2*(length + 1), GFP_KERNEL);
@@ -3581,7 +3582,7 @@  CIFSTCon(unsigned int xid, struct cifsSe
 			length = strnlen(bcc_ptr, 1024);
 			if ((bcc_ptr + length) -
 			    pByteArea(smb_buffer_response) <=
-			    BCC(smb_buffer_response)) {
+			    get_unaligned((u16 *) ((char *)smb_buffer_response + sizeof(struct 
smb_hdr) + (2 * smb_buffer_response->WordCount)))/*BCC(smb_buffer_response)*/) {
 				kfree(tcon->nativeFileSystem);
 				tcon->nativeFileSystem =
 				    kzalloc(length + 1, GFP_KERNEL);
diff -rupw a/fs/cifs/netmisc.c b/fs/cifs/netmisc.c
--- a/fs/cifs/netmisc.c	2008-12-19 10:29:33.000000000 -0700
+++ b/fs/cifs/netmisc.c	2009-06-25 09:28:36.000000000 -0600
@@ -29,6 +29,7 @@ 
 #include <linux/fs.h>
 #include <asm/div64.h>
 #include <asm/byteorder.h>
+#include <asm/unaligned.h>
 #include <linux/inet.h>
 #include "cifsfs.h"
 #include "cifspdu.h"
@@ -840,11 +841,12 @@  smbCalcSize(struct smb_hdr *ptr)
 		2 /* size of the bcc field */ + BCC(ptr));
 }
 
+
 unsigned int
 smbCalcSize_LE(struct smb_hdr *ptr)
 {
 	return (sizeof(struct smb_hdr) + (2 * ptr->WordCount) +
-		2 /* size of the bcc field */ + le16_to_cpu(BCC_LE(ptr)));
+		2 /* size of the bcc field */ + get_unaligned_le16((char *)ptr + 
sizeof(struct smb_hdr) + (2 * ptr->WordCount)) /*le16_to_cpu(BCC_LE(ptr)) */ 
);
 }
 
 /* The following are taken from fs/ntfs/util.c */
diff -rupw a/fs/cifs/sess.c b/fs/cifs/sess.c
--- a/fs/cifs/sess.c	2009-06-25 16:55:30.000000000 -0600
+++ b/fs/cifs/sess.c	2009-06-25 13:51:36.000000000 -0600
@@ -29,6 +29,7 @@ 
 #include "ntlmssp.h"
 #include "nterr.h"
 #include <linux/utsname.h>
+#include <asm/unaligned.h>
 #include "cifs_spnego.h"
 
 extern void SMBNTencrypt(unsigned char *passwd, unsigned char *c8,
@@ -131,10 +132,10 @@  static void unicode_ssetup_strings(char 
 	than 335 or will need to send them as arrays */
 
 	/* unicode strings, must be word aligned before the call */
-/*	if ((long) bcc_ptr % 2)	{
+	if ((long) bcc_ptr % 2)	{
 		*bcc_ptr = 0;
 		bcc_ptr++;
-	} */
+	}
 	/* copy user */
 	if (ses->userName == NULL) {
 		/* null user mount */
@@ -202,27 +203,26 @@  static int decode_unicode_ssetup(char **
 	int words_left, len;
 	char *data = *pbcc_area;
 
-
-
 	cFYI(1, ("bleft %d", bleft));
 
-
-	/* SMB header is unaligned, so cifs servers word align start of
-	   Unicode strings */
-	data++;
-	bleft--; /* Windows servers do not always double null terminate
-		    their final Unicode string - in which case we
-		    now will not attempt to decode the byte of junk
-		    which follows it */
+	/*
+	 * Windows servers do not always double null terminate their final
+	 * Unicode string. Check to see if there are an uneven number of bytes
+	 * left. If so, then add an extra NULL pad byte to the end of the
+	 * response.
+	 *
+	 * See section 2.7.2 in "Implementing CIFS" for details
+	 */
+	if (bleft % 2) {
+		data[bleft] = 0;
+		++bleft;
+	}
 
 	words_left = bleft / 2;
 
 	/* save off server operating system */
 	len = UniStrnlen((wchar_t *) data, words_left);
 
-/* 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;