Attachment 294296 Details for Bug 429755 – NFS: Fix a potential file corruption issue when writing

[patch] NFS: Fix a potential file corruption issue when writing

linux-2.6.18-001-fix_write_race.dif (text/plain), 2.64 KB, created by Trond Myklebust on 2008-02-08 01:25:29 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Trond Myklebust

Created: 2008-02-08 01:25:29 UTC

Size: 2.64 KB

patch

obsolete

>From: Trond Myklebust <Trond.Myklebust@netapp.com>
>Date: Thu, 7 Feb 2008 17:24:07 -0500
>NFS: Fix a potential file corruption issue when writing
>
>If the inode is flagged as having an invalid mapping, then we can't rely on
>the PageUptodate() flag. Ensure that we don't use the "anti-fragmentation"
>write optimisation in nfs_updatepage(), since that will cause NFS to write
>out areas of the page that are no longer guaranteed to be up to date.
>
>A potential corruption could occur in the following scenario:
>
>client 1			client 2
>===============			===============
>				fd=open("f",O_CREAT|O_WRONLY,0644);
>				write(fd,"fubar\n",6);	// cache last page
>				close(fd);
>fd=open("f",O_WRONLY|O_APPEND);
>write(fd,"foo\n",4);
>close(fd);
>
>				fd=open("f",O_WRONLY|O_APPEND);
>				write(fd,"bar\n",4);
>				close(fd);
>-----
>The bug may lead to the file "f" reading 'fubar\n\0\0\0\nbar\n' because
>client 2 does not update the cached page after re-opening the file for
>write. Instead it keeps it marked as PageUptodate() until someone calls
>invaldate_inode_pages2() (typically by calling read()).
>
>Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
>---
>
> fs/nfs/write.c |   20 +++++++++++++++++---
> 1 files changed, 17 insertions(+), 3 deletions(-)
>
>diff --git a/fs/nfs/write.c b/fs/nfs/write.c
>index b57128f..4f05585 100644
>--- a/fs/nfs/write.c
>+++ b/fs/nfs/write.c
>@@ -824,6 +824,17 @@ int nfs_flush_incompatible(struct file *file, struct page *page)
> }
> 
> /*
>+ * If the page cache is marked as unsafe or invalid, then we can't rely on
>+ * the PageUptodate() flag. In this case, we will need to turn off
>+ * write optimisations that depend on the page contents being correct.
>+ */
>+static int nfs_write_pageuptodate(struct page *page, struct inode *inode)
>+{
>+	return PageUptodate(page) &&
>+		!(NFS_I(inode)->cache_validity & (NFS_INO_REVAL_PAGECACHE|NFS_INO_INVALID_DATA));
>+}
>+
>+/*
>  * Update and possibly write a cached page of an NFS file.
>  *
>  * XXX: Keep an eye on generic_file_read to make sure it doesn't do bad
>@@ -855,10 +866,13 @@ int nfs_updatepage(struct file *file, struct page *page,
> 	}
> 
> 	/* If we're not using byte range locks, and we know the page
>-	 * is entirely in cache, it may be more efficient to avoid
>-	 * fragmenting write requests.
>+	 * is up to date, it may be more efficient to extend the write
>+	 * to cover the entire page in order to avoid fragmentation
>+	 * inefficiencies.
> 	 */
>-	if (PageUptodate(page) && inode->i_flock == NULL && !(file->f_mode & O_SYNC)) {
>+	if (nfs_write_pageuptodate(page, inode) &&
>+			inode->i_flock == NULL &&
>+			!(file->f_mode & O_SYNC)) {
> 		loff_t end_offs = i_size_read(inode) - 1;
> 		unsigned long end_index = end_offs >> PAGE_CACHE_SHIFT;
>

Actions: View | Diff

Attachments on bug 429755: 292558 | 294295 | 294296 | 295054