linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Xiao Yang <ice_yangxiao@163.com>
To: miklos@szeredi.hu, vgoyal@redhat.com, stefanha@redhat.com
Cc: linux-fsdevel@vger.kernel.org, yangx.jy@cn.fujitsu.com,
	Xiao Yang <ice_yangxiao@163.com>
Subject: [PATCH] fuse: Don't make buffered read forward overflow value to a userspace process
Date: Mon,  3 Feb 2020 15:36:52 +0800	[thread overview]
Message-ID: <20200203073652.12067-1-ice_yangxiao@163.com> (raw)

Buffered read in fuse normally goes via:
-> generic_file_buffered_read()
  ------------------------------
  -> fuse_readpages()
    -> fuse_send_readpages()
  or
  -> fuse_readpage() [if fuse_readpages() fails to get page]
    -> fuse_do_readpage()
  ------------------------------
      -> fuse_simple_request()

Buffered read changes original offset to page-aligned length by left-shift
and extends original count to be multiples of PAGE_SIZE and then fuse
forwards these new parameters to a userspace process, so it is possible
for the resulting offset(e.g page-aligned offset + extended count) to
exceed the whole file size(even the max value of off_t) when the userspace
process does read with new parameters.

xfstests generic/525 gets "pread: Invalid argument" error on virtiofs
because it triggers this issue.  See the following explanation:
PAGE_SIZE: 4096, file size: 2^63 - 1
Original: offset: 2^63 - 2, count: 1
Changed by buffered read: offset: 2^63 - 4096, count: 4096
New offset + new count exceeds the file size as well as LLONG_MAX

Make fuse calculate the number of bytes of data pages contain as
nfs_page_length() and generic_file_buffered_read() do, and then forward
page-aligned offset and normal count to a userspace process.

Signed-off-by: Xiao Yang <ice_yangxiao@163.com>
---
 fs/fuse/file.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index ce715380143c..5afc4b623eaf 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -19,6 +19,23 @@
 #include <linux/falloc.h>
 #include <linux/uio.h>
 
+static unsigned int fuse_page_length(struct page *page)
+{
+	loff_t i_size = i_size_read(page->mapping->host);
+
+	if (i_size > 0) {
+		pgoff_t index = page_index(page);
+		pgoff_t end_index = (i_size - 1) >> PAGE_SHIFT;
+
+		if (index < end_index)
+			return PAGE_SIZE;
+		if (index == end_index)
+			return ((i_size - 1) & ~PAGE_MASK) + 1;
+	}
+
+	return 0;
+}
+
 static struct page **fuse_pages_alloc(unsigned int npages, gfp_t flags,
 				      struct fuse_page_desc **desc)
 {
@@ -783,7 +800,7 @@ static int fuse_do_readpage(struct file *file, struct page *page)
 	struct inode *inode = page->mapping->host;
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	loff_t pos = page_offset(page);
-	struct fuse_page_desc desc = { .length = PAGE_SIZE };
+	struct fuse_page_desc desc = { .length = fuse_page_length(page) };
 	struct fuse_io_args ia = {
 		.ap.args.page_zeroing = true,
 		.ap.args.out_pages = true,
@@ -881,9 +898,12 @@ static void fuse_send_readpages(struct fuse_io_args *ia, struct file *file)
 	struct fuse_conn *fc = ff->fc;
 	struct fuse_args_pages *ap = &ia->ap;
 	loff_t pos = page_offset(ap->pages[0]);
-	size_t count = ap->num_pages << PAGE_SHIFT;
+	size_t count = 0;
 	ssize_t res;
-	int err;
+	int err, i;
+
+	for (i = 0; i < ap->num_pages; i++)
+		count += ap->descs[i].length;
 
 	ap->args.out_pages = true;
 	ap->args.page_zeroing = true;
@@ -944,7 +964,7 @@ static int fuse_readpages_fill(void *_data, struct page *page)
 
 	get_page(page);
 	ap->pages[ap->num_pages] = page;
-	ap->descs[ap->num_pages].length = PAGE_SIZE;
+	ap->descs[ap->num_pages].length = fuse_page_length(page);
 	ap->num_pages++;
 	data->nr_pages--;
 	return 0;
-- 
2.21.0



             reply	other threads:[~2020-02-03  7:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-03  7:36 Xiao Yang [this message]
2020-02-05 14:37 ` [PATCH] fuse: Don't make buffered read forward overflow value to a userspace process Miklos Szeredi
2020-02-06 12:14   ` Miklos Szeredi
2020-02-06 12:32     ` Xiao Yang
2020-02-06 15:40       ` Miklos Szeredi
2020-02-07  0:44         ` Xiao Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200203073652.12067-1-ice_yangxiao@163.com \
    --to=ice_yangxiao@163.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=stefanha@redhat.com \
    --cc=vgoyal@redhat.com \
    --cc=yangx.jy@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).