From patchwork Mon Jun 29 17:30:01 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Craig Matsuura X-Patchwork-Id: 29294 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.samba.org (mail.samba.org [66.70.73.150]) by bilbo.ozlabs.org (Postfix) with ESMTP id DCE11B70FF for ; Tue, 30 Jun 2009 03:31:36 +1000 (EST) Received: from dp.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 1BBB6163C41 for ; Mon, 29 Jun 2009 17:31:00 +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=-0.6 required=3.8 tests=AWL,BAYES_00,HTML_40_50, HTML_ATTR_UNIQUE,HTML_MESSAGE autolearn=no version=3.1.7 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from c4mail01.control4.com (mail.control4.com [204.246.145.15]) by lists.samba.org (Postfix) with ESMTP id 7CD67163B73 for ; Mon, 29 Jun 2009 17:30:13 +0000 (GMT) Received: from c4mail02.control4.com ([10.11.16.18]) by c4mail01.control4.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 29 Jun 2009 11:30:49 -0600 X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Subject: RE: [linux-cifs-client] PATCH: alignment changes for CIFS VFS Date: Mon, 29 Jun 2009 11:30:01 -0600 Message-ID: <4D81CA9C484D3842B958A9CD18344D1D04915FAD@c4mail02.control4.com> In-Reply-To: <200906252018.41356.cmatsuura@control4.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [linux-cifs-client] PATCH: alignment changes for CIFS VFS Thread-Index: Acn2BkdIcFksdU70SieIBScxABJPKwC2K4Xw References: <200906252018.41356.cmatsuura@control4.com> From: "Craig Matsuura" To: "Craig Matsuura" , X-OriginalArrivalTime: 29 Jun 2009 17:30:49.0165 (UTC) FILETIME=[57D397D0:01C9F8DF] 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: , Mime-version: 1.0 Sender: linux-cifs-client-bounces+incoming=patchwork.ozlabs.org@lists.samba.org Errors-To: linux-cifs-client-bounces+incoming=patchwork.ozlabs.org@lists.samba.org 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: linux-cifs-client-bounces+cmatsuura=control4.com@lists.samba.org [mailto:linux-cifs-client-bounces+cmatsuura=control4.com@lists.samba.org ] On Behalf Of Craig Matsuura Sent: Thursday, June 25, 2009 8:19 PM To: linux-cifs-client@lists.samba.org 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 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) { 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 #include #include +#include #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 #include #include +#include #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 #include #include +#include #include #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 +#include #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)