RE: PATCH: alignment changes for CIFS VFS

No comments regarding this patch?  I sent a more recent patch, but the
moderator has not approve it apparently?  I was hoping to get some
feedback as if anynone else has seen such problems with alignment.  I
wanted to fix this in our kernel so there are not fixups.

 

Thanks,

Craig

 

From: [email protected]
[mailto:[email protected]
] On Behalf Of Craig Matsuura
Sent: Thursday, June 25, 2009 8:19 PM
To: [email protected]
Subject: [linux-cifs-client] PATCH: alignment changes for CIFS VFS

 

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