linux-kernel - [PATCH 06/11] pipe: no need to confirm page cache buf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1473842236-28655-7-git-send-email-mszeredi@redhat.com>
Date:   Wed, 14 Sep 2016 10:37:11 +0200
From:   Miklos Szeredi <mszeredi@...hat.com>
To:     linux-fsdevel@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, Al Viro <viro@...iv.linux.org.uk>
Subject: [PATCH 06/11] pipe: no need to confirm page cache buf

page_cache_pipe_buf_confirm() is used only in page_cache_pipe_buf_ops.

page_cache_pipe_buf_ops is used in two places:

1) __generic_file_splice_read()

This iterates all the pages, if not uptodate locks page, and if still not
uptodate does ->readpage() which reads the page synchronously.

2) shmem_file_splice_read()

This calls shmem_getpage() on individual pages.  shmem_getpage() swaps in
the page synchronously if not in memory, so it also seems to return an
uptodate page.

So all pages put into the buffer will be uptodate.

Things could happen to that page that make it not uptodate while sitting in
the pipe, but it's questionable whether we should care about that.
Checking for being uptodate in the face of such page state change is always
going to be racy.

Signed-off-by: Miklos Szeredi <mszeredi@...hat.com>
---
 fs/splice.c | 43 +------------------------------------------
 1 file changed, 1 insertion(+), 42 deletions(-)

diff --git a/fs/splice.c b/fs/splice.c
index ba7a2240d58c..0ecbe3011796 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -92,51 +92,10 @@ static void page_cache_pipe_buf_release(struct pipe_inode_info *pipe,
 	buf->flags &= ~PIPE_BUF_FLAG_LRU;
 }
 
-/*
- * Check whether the contents of buf is OK to access. Since the content
- * is a page cache page, IO may be in flight.
- */
-static int page_cache_pipe_buf_confirm(struct pipe_inode_info *pipe,
-				       struct pipe_buffer *buf)
-{
-	struct page *page = buf->page;
-	int err;
-
-	if (!PageUptodate(page)) {
-		lock_page(page);
-
-		/*
-		 * Page got truncated/unhashed. This will cause a 0-byte
-		 * splice, if this is the first page.
-		 */
-		if (!page->mapping) {
-			err = -ENODATA;
-			goto error;
-		}
-
-		/*
-		 * Uh oh, read-error from disk.
-		 */
-		if (!PageUptodate(page)) {
-			err = -EIO;
-			goto error;
-		}
-
-		/*
-		 * Page is ok afterall, we are done.
-		 */
-		unlock_page(page);
-	}
-
-	return 0;
-error:
-	unlock_page(page);
-	return err;
-}
 
 const struct pipe_buf_operations page_cache_pipe_buf_ops = {
 	.can_merge = 0,
-	.confirm = page_cache_pipe_buf_confirm,
+	.confirm = generic_pipe_buf_confirm,
 	.release = page_cache_pipe_buf_release,
 	.steal = page_cache_pipe_buf_steal,
 	.get = generic_pipe_buf_get,
-- 
2.5.5