PATCH: alignment changes for CIFS VFS

Craig Matsuura <[email protected]>
Newsgroups gmane.linux.file-systems.cifs
Organization Control4
Message-ID <[email protected]>
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
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;
 
@@ -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) {


-- 




Craig Matsuura - Principal Engineer
Control4
11734 South Election Road - Suite 200
Salt Lake City, UT 84020-6432
PH: 801-523-3161
FX: 801-523-3199

_______________________________________________
linux-cifs-client mailing list
[email protected]
https://lists.samba.org/mailman/listinfo/linux-cifs-client
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.