CVE-2014-0069 cifs: incorrect handling of bogus user pointers (rhbz 1064253 1062578)

This commit is contained in:
Josh Boyer 2014-02-14 12:40:30 -05:00
parent ef49c2e612
commit e41ad71714
3 changed files with 290 additions and 0 deletions

View File

@ -0,0 +1,149 @@
Path: news.gmane.org!not-for-mail
From: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Newsgroups: gmane.linux.kernel.cifs
Subject: [PATCH] cifs: ensure that uncached writes handle unmapped areas correctly
Date: Fri, 14 Feb 2014 07:20:35 -0500
Lines: 92
Approved: news@gmane.org
Message-ID: <1392380435-6903-1-git-send-email-jlayton@redhat.com>
NNTP-Posting-Host: plane.gmane.org
X-Trace: ger.gmane.org 1392380436 7379 80.91.229.3 (14 Feb 2014 12:20:36 GMT)
X-Complaints-To: usenet@ger.gmane.org
NNTP-Posting-Date: Fri, 14 Feb 2014 12:20:36 +0000 (UTC)
Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Original-X-From: linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Fri Feb 14 13:20:44 2014
Return-path: <linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Envelope-to: glkc-linux-cifs-wOFGN7rlS/M9smdsby/KFg@public.gmane.org
Original-Received: from vger.kernel.org ([209.132.180.67])
by plane.gmane.org with esmtp (Exim 4.69)
(envelope-from <linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>)
id 1WEHl6-0001gj-3d
for glkc-linux-cifs-wOFGN7rlS/M9smdsby/KFg@public.gmane.org; Fri, 14 Feb 2014 13:20:44 +0100
Original-Received: (majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org) by vger.kernel.org via listexpand
id S1752014AbaBNMUn (ORCPT <rfc822;glkc-linux-cifs@m.gmane.org>);
Fri, 14 Feb 2014 07:20:43 -0500
Original-Received: from mail-qa0-f41.google.com ([209.85.216.41]:53300 "EHLO
mail-qa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S1751935AbaBNMUn (ORCPT
<rfc822;linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>); Fri, 14 Feb 2014 07:20:43 -0500
Original-Received: by mail-qa0-f41.google.com with SMTP id w8so18341449qac.0
for <linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; Fri, 14 Feb 2014 04:20:42 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20130820;
h=x-gm-message-state:sender:from:to:cc:subject:date:message-id;
bh=U+Hg0/eYYIJkioXu/lkD6lkFJ0tCG+CokyC++yC+DQM=;
b=k0jKxn6Oe6WzaA8qsSjcTaszuxeetSkSa7zALlqcXb5ZMePVPSZXV4ceIYOs3RRg3T
ABfigeWTQiCwmRFobeOh77zBI4vsGXRx9UkDYTwNVzMIVGD+eFm90m4gB2DtSl0HqPFI
vUaBZDWNIduFFfuTEADF4FoguV+lX07T9lgAPXO6F05sC/dxQ5ffrTFrao0k1kzJO/tg
72LueH0aRcwWHxA9e+WW3KkvRZALlcqWIe+Bnomw51qdtpetKPmag3/BTtLwwUSbAU0e
uppFe/6fHJHN+SHYgVGrVLFH2OXbRSyv8R4wC1BWlau6khtclu/R0X22jk5Okd+zM8Jd
DvkA==
X-Gm-Message-State: ALoCoQlx+9xv55qz9RE2vMXqhqjy8E3xo9guNbdH3Gwu3BbXxDqM4CRvLyHWarIQK4MEnf47FoZK
X-Received: by 10.140.31.247 with SMTP id f110mr11536194qgf.58.1392380442431;
Fri, 14 Feb 2014 04:20:42 -0800 (PST)
Original-Received: from tlielax.poochiereds.net ([2001:470:8:d63:3a60:77ff:fe93:a95d])
by mx.google.com with ESMTPSA id r40sm7432383qga.23.2014.02.14.04.20.41
for <multiple recipients>
(version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Fri, 14 Feb 2014 04:20:41 -0800 (PST)
X-Mailer: git-send-email 1.8.5.3
Original-Sender: linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Precedence: bulk
List-ID: <linux-cifs.vger.kernel.org>
X-Mailing-List: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Xref: news.gmane.org gmane.linux.kernel.cifs:9401
Archived-At: <http://permalink.gmane.org/gmane.linux.kernel.cifs/9401>
It's possible for userland to pass down an iovec via writev() that has a
bogus user pointer in it. If that happens and we're doing an uncached
write, then we can end up getting less bytes than we expect from the
call to iov_iter_copy_from_user.
cifs_iovec_write isn't set up to handle that situation however. It'll
blindly keep chugging through the page array and not filling those pages
with anything useful. Worse yet, we'll later end up with a negative
number in wdata->tailsz, which will confuse the sending routines and
cause an oops at the very least.
Fix this by having the copy phase of cifs_iovec_write stop copying data
in this situation and send the last write as a short one. At the same
time, we want to avoid sending a zero-length write to the server, so
break out of the loop and set rc to -EFAULT if that happens. This also
allows us to handle the case where no address in the iovec is valid.
[Note: Marking this for stable on v3.4+ kernels, but kernels as old as
v2.6.38 may have a similar problem and may need similar fix]
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.4+
Reviewed-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
Reported-by: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/file.c | 37 ++++++++++++++++++++++++++++++++++---
1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 03d1f454c713..ae1a4d58e672 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2387,7 +2387,7 @@ cifs_iovec_write(struct file *file, const struct iovec *iov,
unsigned long nr_segs, loff_t *poffset)
{
unsigned long nr_pages, i;
- size_t copied, len, cur_len;
+ size_t bytes, copied, len, cur_len;
ssize_t total_written = 0;
loff_t offset;
struct iov_iter it;
@@ -2442,14 +2442,45 @@ cifs_iovec_write(struct file *file, const struct iovec *iov,
save_len = cur_len;
for (i = 0; i < nr_pages; i++) {
- copied = min_t(const size_t, cur_len, PAGE_SIZE);
+ bytes = min_t(const size_t, cur_len, PAGE_SIZE);
copied = iov_iter_copy_from_user(wdata->pages[i], &it,
- 0, copied);
+ 0, bytes);
cur_len -= copied;
iov_iter_advance(&it, copied);
+ /*
+ * If we didn't copy as much as we expected, then that
+ * may mean we trod into an unmapped area. Stop copying
+ * at that point. On the next pass through the big
+ * loop, we'll likely end up getting a zero-length
+ * write and bailing out of it.
+ */
+ if (copied < bytes)
+ break;
}
cur_len = save_len - cur_len;
+ /*
+ * If we have no data to send, then that probably means that
+ * the copy above failed altogether. That's most likely because
+ * the address in the iovec was bogus. Set the rc to -EFAULT,
+ * free anything we allocated and bail out.
+ */
+ if (!cur_len) {
+ for (i = 0; i < nr_pages; i++)
+ put_page(wdata->pages[i]);
+ kfree(wdata);
+ rc = -EFAULT;
+ break;
+ }
+
+ /*
+ * i + 1 now represents the number of pages we actually used in
+ * the copy phase above. Bring nr_pages down to that, and free
+ * any pages that we didn't use.
+ */
+ for ( ; nr_pages > i + 1; nr_pages--)
+ put_page(wdata->pages[nr_pages - 1]);
+
wdata->sync_mode = WB_SYNC_ALL;
wdata->nr_pages = nr_pages;
wdata->offset = (__u64)offset;
--
1.8.5.3

View File

@ -0,0 +1,129 @@
Path: news.gmane.org!not-for-mail
From: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Newsgroups: gmane.linux.kernel.cifs
Subject: [PATCH] cifs: sanity check length of data to send before sending
Date: Fri, 14 Feb 2014 07:21:00 -0500
Lines: 72
Approved: news@gmane.org
Message-ID: <1392380460-6997-1-git-send-email-jlayton@redhat.com>
NNTP-Posting-Host: plane.gmane.org
X-Trace: ger.gmane.org 1392380460 7637 80.91.229.3 (14 Feb 2014 12:21:00 GMT)
X-Complaints-To: usenet@ger.gmane.org
NNTP-Posting-Date: Fri, 14 Feb 2014 12:21:00 +0000 (UTC)
Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Original-X-From: linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Fri Feb 14 13:21:08 2014
Return-path: <linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Envelope-to: glkc-linux-cifs-wOFGN7rlS/M9smdsby/KFg@public.gmane.org
Original-Received: from vger.kernel.org ([209.132.180.67])
by plane.gmane.org with esmtp (Exim 4.69)
(envelope-from <linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>)
id 1WEHlT-0001gj-Pf
for glkc-linux-cifs-wOFGN7rlS/M9smdsby/KFg@public.gmane.org; Fri, 14 Feb 2014 13:21:08 +0100
Original-Received: (majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org) by vger.kernel.org via listexpand
id S1751935AbaBNMVH (ORCPT <rfc822;glkc-linux-cifs@m.gmane.org>);
Fri, 14 Feb 2014 07:21:07 -0500
Original-Received: from mail-qc0-f170.google.com ([209.85.216.170]:60206 "EHLO
mail-qc0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S1751288AbaBNMVG (ORCPT
<rfc822;linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>); Fri, 14 Feb 2014 07:21:06 -0500
Original-Received: by mail-qc0-f170.google.com with SMTP id e9so20306487qcy.29
for <linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>; Fri, 14 Feb 2014 04:21:05 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20130820;
h=x-gm-message-state:sender:from:to:cc:subject:date:message-id;
bh=8FvTfO9jKY+Fzw5nmoxw6PuPxfXM/khvtS/Hnxnu+to=;
b=i7Gy5AwQiFX7hVMJJMjvAR61u2zO8E7tTAgE0SfPp25untipj/RTae8xcA+0vVrznY
sZwOPATUyFmu2mXAKh5L0WutFNF3SQirPVgM5jaKlMZQT253YInioO1AwD00Jtio00LX
wOt19I6v3umZfSqCBw1dyDiH66kX4xKaKqdQoY2+nEsicG4RJvp4PpaLhG454nfpVxfN
HeskcNJ5RBvg17JpDvKX6THqbKWsHSHHqKKCROERxeTbs7HTgV9jTNSDeOuPr6Loiovi
9DBQwQwMmlC9NebDqR5xatva30WRhyCp/xyHNaoY+aVk8N6r5YlFphLmLRvmaZ0Ed2CH
17WA==
X-Gm-Message-State: ALoCoQmHyaepi0IHvwKS024wRq/srAdGRdo0UGUynLggzf843S3Yj7dwW53RjNbTDG1Y4w+/mqOB
X-Received: by 10.229.13.133 with SMTP id c5mr12315963qca.22.1392380465000;
Fri, 14 Feb 2014 04:21:05 -0800 (PST)
Original-Received: from tlielax.poochiereds.net ([2001:470:8:d63:3a60:77ff:fe93:a95d])
by mx.google.com with ESMTPSA id u4sm15047497qai.21.2014.02.14.04.21.04
for <multiple recipients>
(version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Fri, 14 Feb 2014 04:21:04 -0800 (PST)
X-Mailer: git-send-email 1.8.5.3
Original-Sender: linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Precedence: bulk
List-ID: <linux-cifs.vger.kernel.org>
X-Mailing-List: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Xref: news.gmane.org gmane.linux.kernel.cifs:9402
Archived-At: <http://permalink.gmane.org/gmane.linux.kernel.cifs/9402>
We had a bug discovered recently where an upper layer function
(cifs_iovec_write) could pass down a smb_rqst with an invalid amount of
data in it. The length of the SMB frame would be correct, but the rqst
struct would cause smb_send_rqst to send nearly 4GB of data.
This should never be the case. Add some sanity checking to the beginning
of smb_send_rqst that ensures that the amount of data we're going to
send agrees with the length in the RFC1002 header. If it doesn't, WARN()
and return -EIO to the upper layers.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/transport.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index b37570952846..18cd5650a5fc 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -270,6 +270,26 @@ cifs_rqst_page_to_kvec(struct smb_rqst *rqst, unsigned int idx,
iov->iov_len = rqst->rq_pagesz;
}
+static unsigned long
+rqst_len(struct smb_rqst *rqst)
+{
+ unsigned int i;
+ struct kvec *iov = rqst->rq_iov;
+ unsigned long buflen = 0;
+
+ /* total up iov array first */
+ for (i = 0; i < rqst->rq_nvec; i++)
+ buflen += iov[i].iov_len;
+
+ /* add in the page array if there is one */
+ if (rqst->rq_npages) {
+ buflen += rqst->rq_pagesz * (rqst->rq_npages - 1);
+ buflen += rqst->rq_tailsz;
+ }
+
+ return buflen;
+}
+
static int
smb_send_rqst(struct TCP_Server_Info *server, struct smb_rqst *rqst)
{
@@ -277,6 +297,7 @@ smb_send_rqst(struct TCP_Server_Info *server, struct smb_rqst *rqst)
struct kvec *iov = rqst->rq_iov;
int n_vec = rqst->rq_nvec;
unsigned int smb_buf_length = get_rfc1002_length(iov[0].iov_base);
+ unsigned long send_length;
unsigned int i;
size_t total_len = 0, sent;
struct socket *ssocket = server->ssocket;
@@ -285,6 +306,14 @@ smb_send_rqst(struct TCP_Server_Info *server, struct smb_rqst *rqst)
if (ssocket == NULL)
return -ENOTSOCK;
+ /* sanity check send length */
+ send_length = rqst_len(rqst);
+ if (send_length != smb_buf_length + 4) {
+ WARN(1, "Send length mismatch(send_length=%lu smb_buf_length=%u)\n",
+ send_length, smb_buf_length);
+ return -EIO;
+ }
+
cifs_dbg(FYI, "Sending smb: smb_len=%u\n", smb_buf_length);
dump_smb(iov[0].iov_base, iov[0].iov_len);
--
1.8.5.3

View File

@ -629,6 +629,10 @@ Patch25194: tick-Clear-broadcast-pending-bit-when-switching-to-oneshot.patch
#rhbz 1045755
Patch25195: cgroup-fixes.patch
#CVE-2014-0069 rhbz 1064253 1062578
Patch25200: cifs-ensure-that-uncached-writes-handle-unmapped-areas-correctly.patch
Patch25201: cifs-sanity-check-length-of-data-to-send-before-sending.patch
# END OF PATCH DEFINITIONS
%endif
@ -1280,6 +1284,11 @@ ApplyPatch tick-Clear-broadcast-pending-bit-when-switching-to-oneshot.patch
#rhbz 1045755
ApplyPatch cgroup-fixes.patch
#CVE-2014-0069 rhbz 1064253 1062578
ApplyPatch cifs-ensure-that-uncached-writes-handle-unmapped-areas-correctly.patch
ApplyPatch cifs-sanity-check-length-of-data-to-send-before-sending.patch
# END OF PATCH APPLICATIONS
%endif
@ -2059,6 +2068,9 @@ fi
# ||----w |
# || ||
%changelog
* Fri Feb 14 2014 Josh Boyer <jwboyer@fedoraproject.org>
- CVE-2014-0069 cifs: incorrect handling of bogus user pointers (rhbz 1064253 1062578)
* Thu Feb 13 2014 Josh Boyer <jwboyer@fedoraproject.org> - 3.14.0-0.rc2.git3.1
- Linux v3.14-rc2-271-g4675348