Attachment 660887 Details for Bug 885743 – patch -- don't extend writes to cover entire page when pagecache is invalid

[patch] patch -- don't extend writes to cover entire page when pagecache is invalid

0001-nfs-don-t-extend-writes-to-cover-entire-page-if-page.patch (text/plain), 2.26 KB, created by Jeff Layton on 2012-12-10 14:13:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Layton

Created: 2012-12-10 14:13:50 UTC

Size: 2.26 KB

patch

obsolete

>From bed156dd134e95681f6fdb7142e240ba723a6875 Mon Sep 17 00:00:00 2001
>From: Jeff Layton <jlayton@redhat.com>
>Date: Mon, 10 Dec 2012 09:04:53 -0500
>Subject: [PATCH] nfs: don't extend writes to cover entire page if pagecache
> is invalid
>
>Jian reported that the following sequence would leave "testfile" with
>corrupt data:
>
>    # mount localhost:/export /mnt/nfs/ -o vers=3
>    # echo abc > /mnt/nfs/testfile; echo def >> /export/testfile; echo ghi >> /mnt/nfs/testfile
>    # cat -v /export/testfile
>    abc
>    ^@^@^@^@ghi
>
>While there's no locking involved here, the operations are serialized,
>so CTO should prevent corruption.
>
>The first write to the file is fine and writes 4 bytes. The file is then
>extended on the server. When it's reopened a GETATTR is issued and the
>size change is noticed. This causes NFS_INO_INVALID_DATA to be set on
>the file. Because the file is opened for write only,
>nfs_want_read_modify_write() returns 0 to nfs_write_begin().
>nfs_updatepage then calls nfs_write_pageuptodate() to see if it should
>extend the nfs_page to cover the whole page. NFS_INO_INVALID_DATA is
>still set on the file at that point, but that flag is ignored and
>nfs_pageuptodate erroneously extends the write to cover the whole page.
>
>This patch just has that function check for that flag in addition to
>NFS_INO_REVAL_PAGECACHE. This fixes the bug, but looking over the code,
>I wonder if we might have a similar bug in nfs_revalidate_size(). The
>difference between those two flags is very subtle, so it seems like
>we ought to be checking for NFS_INO_INVALID_DATA in most of the places
>that we look for NFS_INO_REVAL_PAGECACHE.
>
>Cc: <stable@vger.kernel.org>
>Reported-by: Jian Li <jiali@redhat.com>
>Signed-off-by: Jeff Layton <jlayton@redhat.com>
>---
> fs/nfs/write.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/fs/nfs/write.c b/fs/nfs/write.c
>index 9347ab7..623389f 100644
>--- a/fs/nfs/write.c
>+++ b/fs/nfs/write.c
>@@ -884,7 +884,8 @@ static bool nfs_write_pageuptodate(struct page *page, struct inode *inode)
> {
> 	if (nfs_have_delegated_attributes(inode))
> 		goto out;
>-	if (NFS_I(inode)->cache_validity & NFS_INO_REVAL_PAGECACHE)
>+	if (NFS_I(inode)->cache_validity &
>+			(NFS_INO_INVALID_DATA | NFS_INO_REVAL_PAGECACHE))
> 		return false;
> out:
> 	return PageUptodate(page) != 0;
>-- 
>1.7.11.7
>

Actions: View | Diff

Attachments on bug 885743: 660887