Path: news.gmane.org!not-for-mail From: Jeff Layton 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: 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 ) 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 ); 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 ); Fri, 14 Feb 2014 07:20:43 -0500 Original-Received: by mail-qa0-f41.google.com with SMTP id w8so18341449qac.0 for ; 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 (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: X-Mailing-List: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Xref: news.gmane.org gmane.linux.kernel.cifs:9401 Archived-At: 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: # v3.4+ Reviewed-by: Pavel Shilovsky Reported-by: Al Viro Signed-off-by: Jeff Layton --- 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