From patchwork Tue Apr 14 15:00:53 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 18136 Received: from lists.samba.org (mail.samba.org [66.70.73.150]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n3EF1RxU008485 for ; Tue, 14 Apr 2009 15:01:27 GMT Received: from dp.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 3C0EC163BF2 for ; Tue, 14 Apr 2009 15:01:07 +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=-3.8 required=3.8 tests=AWL,BAYES_00, FORGED_RCVD_HELO,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.1.7 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from mx2.redhat.com (mx2.redhat.com [66.187.237.31]) by lists.samba.org (Postfix) with ESMTP id 9F9A5163B95 for ; Tue, 14 Apr 2009 15:00:38 +0000 (GMT) Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n3EF0uXK019373; Tue, 14 Apr 2009 11:00:56 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n3EF0sgb026165; Tue, 14 Apr 2009 11:00:54 -0400 Received: from localhost.localdomain (vpn-12-45.rdu.redhat.com [10.11.12.45]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n3EF0rgx019648; Tue, 14 Apr 2009 11:00:53 -0400 From: Jeff Layton To: smfrench@gmail.com Date: Tue, 14 Apr 2009 11:00:53 -0400 Message-Id: <1239721253-6004-1-git-send-email-jlayton@redhat.com> X-Scanned-By: MIMEDefang 2.58 on 172.16.27.26 Cc: linux-cifs-client@lists.samba.org, sjayaraman@suse.de Subject: [linux-cifs-client] [PATCH] cifs: fix unicode string area word alignment in session setup 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: , Sender: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org Errors-To: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org The handling of unicode string area alignment is wrong. decode_unicode_ssetup improperly assumes that it will always be preceded by a pad byte. This isn't the case if the string area is already word-aligned. This problem, combined with the bad buffer sizing for the serverDomain string can cause memory corruption. The bad alignment can make it so that the alignment of the characters is off. This can make them translate to characters that are greater than 2 bytes each. If this happens we can overflow the allocation. Fix this by fixing the alignment in CIFS_SessSetup instead so we can verify it against the head of the response. Also, clean up the workaround for improperly terminated strings by checking for a odd-length unicode buffers and then forcibly terminating them. Finally, resize the buffer for serverDomain. Now that we've fixed the alignment, it's probably fine, but a malicious server could overflow it. A better solution for handling these strings is still needed, but this should be a suitable bandaid. Signed-off-by: Jeff Layton --- fs/cifs/sess.c | 44 +++++++++++++++++++++++--------------------- 1 files changed, 23 insertions(+), 21 deletions(-) diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index 5c68b42..70d04d0 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -285,27 +285,26 @@ static int decode_unicode_ssetup(char **pbcc_area, int bleft, 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; @@ -343,13 +342,10 @@ static int decode_unicode_ssetup(char **pbcc_area, int bleft, 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; @@ -702,12 +698,18 @@ CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses, int first_time, } /* 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 + ses, nls_cp); + } else { rc = decode_ascii_ssetup(&bcc_ptr, bytes_remaining, ses, nls_cp); + } ssetup_exit: if (spnego_key) {