[Devel,RHEL7,COMMIT] fuse: fuse_prepare_write() cannot handle page from killed request

Submitted by Konstantin Khorenko on Feb. 20, 2017, 1:03 p.m.

Details

Message ID 201702201303.v1KD3wcX008855@finist_cl7.x64_64.work.ct
State New
Series "fuse: fuse_prepare_write() cannot handle page from killed request"
Headers show

Commit Message

Konstantin Khorenko Feb. 20, 2017, 1:03 p.m.
The commit is pushed to "branch-rh7-3.10.0-514.6.1.vz7.28.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-514.6.1.vz7.28.6
------>
commit 0c207f2d6340640f0ed69d0f32e6b37b626de775
Author: Maxim Patlasov <mpatlasov@virtuozzo.com>
Date:   Mon Feb 20 17:03:58 2017 +0400

    fuse: fuse_prepare_write() cannot handle page from killed request
    
    After fuse_prepare_write() called __fuse_readpage(file, page, ...),
    the page might be already unlocked by fuse_kill_requests():
    
    >		for (i = 0; i < req->num_pages; i++) {
    >			struct page *page = req->pages[i];
    >			SetPageError(page);
    >			unlock_page(page);
    
    so it is incorrect to touch it at all. The problem can be easily
    fixed the same way is it was done in fuse_readpage() checking "killed"
    flag.
    
    Another minor complication is that there are three different use-cases
    for that snippet from fuse_kill_requests() above: fuse_readpages(),
    fuse_readpage() and fuse_prepare_write(). Among them only the latter
    needs explicit page_cache_release() call. That's why the patch introduces
    ad-hoc request flag "page_needs_release".
    
    https://jira.sw.ru/browse/PSBM-54547
    
    Signed-off-by: Maxim Patlasov <mpatlasov@virtuozzo.com>
    Acked-by: Dmitry Monakhov <dmonakhov@virtuozzo.com>
---
 fs/fuse/file.c   | 15 ++++++++++-----
 fs/fuse/fuse_i.h |  3 +++
 fs/fuse/inode.c  |  2 ++
 3 files changed, 15 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index a514748..41ed6f0 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1008,7 +1008,7 @@  static void fuse_short_read(struct fuse_req *req, struct inode *inode,
 
 static int __fuse_readpage(struct file *file, struct page *page, size_t count,
 			   int *err, struct fuse_req **req_pp, u64 *attr_ver_p,
-			   bool *killed_p)
+			   bool page_needs_release, bool *killed_p)
 {
 	struct fuse_io_priv io = { .async = 0, .file = file };
 	struct inode *inode = page->mapping->host;
@@ -1040,6 +1040,7 @@  static int __fuse_readpage(struct file *file, struct page *page, size_t count,
 	req->pages[0] = page;
 	req->page_descs[0].length = count;
 	req->page_cache = 1;
+	req->page_needs_release = page_needs_release;
 
 	num_read = fuse_send_read(req, &io, page_offset(page), count, NULL);
 	killed = req->killed;
@@ -1071,7 +1072,7 @@  static int fuse_readpage(struct file *file, struct page *page)
 		goto out;
 
 	num_read = __fuse_readpage(file, page, count, &err, &req, &attr_ver,
-				   &killed);
+				   false, &killed);
 	if (!err) {
 		/*
 		 * Short read means EOF.  If file size is larger, truncate it
@@ -1153,6 +1154,7 @@  static void fuse_send_readpages(struct fuse_req *req, struct file *file)
 	req->out.page_zeroing = 1;
 	req->out.page_replace = 1;
 	req->page_cache = 1;
+	req->page_needs_release = false;
 	fuse_read_fill(req, file, pos, count, FUSE_READ);
 	fuse_account_request(fc, count);
 	req->misc.read.attr_ver = fuse_get_attr_version(fc);
@@ -2368,6 +2370,7 @@  static int fuse_prepare_write(struct fuse_conn *fc, struct file *file,
 	unsigned num_read;
 	unsigned page_len;
 	int err;
+	bool killed = false;
 
 	if (fuse_file_fail_immediately(file)) {
 		unlock_page(page);
@@ -2385,12 +2388,14 @@  static int fuse_prepare_write(struct fuse_conn *fc, struct file *file,
 	}
 
 	num_read = __fuse_readpage(file, page, page_len, &err, &req, NULL,
-				   NULL);
+				   true, &killed);
 	if (req)
 		fuse_put_request(fc, req);
 	if (err) {
-		unlock_page(page);
-		page_cache_release(page);
+		if (!killed) {
+			unlock_page(page);
+			page_cache_release(page);
+		}
 	} else if (num_read != PAGE_CACHE_SIZE) {
 		zero_user_segment(page, num_read, PAGE_CACHE_SIZE);
 	}
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 22eb9c9..fefa8ff7 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -330,6 +330,9 @@  struct fuse_req {
 	/** Request contains pages from page-cache */
 	unsigned page_cache:1;
 
+	/** Request pages need page_cache_release() */
+	unsigned page_needs_release:1;
+
 	/** Request was killed -- pages were released */
 	unsigned killed:1;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index b63aae2..ddd858c 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -378,6 +378,8 @@  static void fuse_kill_requests(struct fuse_conn *fc, struct inode *inode,
 				struct page *page = req->pages[i];
 				SetPageError(page);
 				unlock_page(page);
+				if (req->page_needs_release)
+					page_cache_release(page);
 				req->pages[i] = NULL;
 			}