From: Jeff Layton <jlayton@redhat.com>
Date: Tue, 29 Jun 2010 19:07:21 -0400
Subject: [fs] cifs: fix kernel BUG with remote OS/2 server
Message-id: <1277838441-31170-1-git-send-email-jlayton@redhat.com>
Patchwork-id: 26628
O-Subject: [RHEL5.6 PATCH] BZ#608588: cifs: Fix a kernel BUG with remote OS/2
	server (try #3)
Bugzilla: 608588
CVE: CVE-2010-2248
RH-Acked-by: Jarod Wilson <jarod@redhat.com>

From: Suresh Jayaraman <sjayaraman@suse.de>

The following patch went upstream a couple of months or so ago. With this bug,
it's possible that a malicious server could crash a client that's writing to
it, so it's a security issue.

The problem is due to some nebulousness in the specs for CIFS. The SNIA CIFS
spec states that a 16 bit word in the WRITE_ANDX response is "CountHigh" --
the upper couple of bytes that indicates how much was written. The official MS
document though says that this word is reserved and must be set to 0. Most
servers do set this to 0, unless they allow very large writes, but some OS/2
servers put garbage in here, and that confuses the Linux client. The fix is to
mask off the high 16 bits whenever the response indicates that more was
written than was requested.

I've run this through the normal gamut of tests, but don't have access to any
servers that behave this way so don't have a great way to verify the fix.
Suresh apparently did so on a more recent kernel however.

-------------------------------[snip]----------------------------

While chasing a bug report involving a OS/2 server, I noticed the server sets
pSMBr->CountHigh to a incorrect value even in case of normal writes. This
results in 'nbytes' being computed wrongly and triggers a kernel BUG at
mm/filemap.c.

void iov_iter_advance(struct iov_iter *i, size_t bytes)
{
        BUG_ON(i->count < bytes);    <--- BUG here

Why the server is setting 'CountHigh' is not clear but only does so after
writing 64k bytes. Though this looks like the server bug, the client side
crash may not be acceptable.

The workaround is to mask off high 16 bits if the number of bytes written as
returned by the server is greater than the bytes requested by the client as
suggested by Jeff Layton.

CC: Stable <stable@kernel.org>
Reviewed-by: Jeff Layton <jlayton@samba.org>
Signed-off-by: Suresh Jayaraman <sjayaraman@suse.de>
Signed-off-by: Steve French <sfrench@us.ibm.com>

diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index f112010..3cb16b9 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1588,6 +1588,14 @@ CIFSSMBWrite(const int xid, struct cifsTconInfo *tcon,
 		*nbytes = le16_to_cpu(pSMBr->CountHigh);
 		*nbytes = (*nbytes) << 16;
 		*nbytes += le16_to_cpu(pSMBr->Count);
+
+		/*
+		 * Mask off high 16 bits when bytes written as returned by the
+		 * server is greater than bytes requested by the client. Some
+		 * OS/2 servers are known to set incorrect CountHigh values.
+		 */
+		if (*nbytes > count)
+			*nbytes &= 0xFFFF;
 	}
 
 	cifs_buf_release(pSMB);
@@ -1676,6 +1684,14 @@ CIFSSMBWrite2(const int xid, struct cifsTconInfo *tcon,
 		*nbytes = le16_to_cpu(pSMBr->CountHigh);
 		*nbytes = (*nbytes) << 16;
 		*nbytes += le16_to_cpu(pSMBr->Count);
+
+		/*
+		 * Mask off high 16 bits when bytes written as returned by the
+		 * server is greater than bytes requested by the client. OS/2
+		 * servers are known to set incorrect CountHigh values.
+		 */
+		if (*nbytes > count)
+			*nbytes &= 0xFFFF;
 	}
 
 /*	cifs_small_buf_release(pSMB); */ /* Freed earlier now in SendReceive2 */