Attachment 860442 Details for Bug 1062578 – patch -- cifs: ensure that uncached writes handle unmapped areas correctly

[patch] patch -- cifs: ensure that uncached writes handle unmapped areas correctly

0001-cifs-ensure-that-uncached-writes-handle-unmapped-are.patch (text/plain), 3.28 KB, created by Jeff Layton on 2014-02-07 11:29:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Layton

Created: 2014-02-07 11:29:23 UTC

Size: 3.28 KB

patch

obsolete

>From 23270e2e0754e696c13e918bf792fd64690d256a Mon Sep 17 00:00:00 2001
>From: Jeff Layton <jlayton@redhat.com>
>Date: Thu, 6 Feb 2014 06:36:36 -0500
>Subject: [PATCH] cifs: ensure that uncached writes handle unmapped areas
> correctly
>
>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.
>
>Reviewed-by: Pavel Shilovsky <piastry@etersoft.ru>
>Reported-by: Al Viro <viro@zeniv.linux.org.uk>
>Signed-off-by: Jeff Layton <jlayton@redhat.com>
>---
> 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 d76c935..b76b966 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
>

Actions: View | Diff

Attachments on bug 1062578: 860442 | 862435