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>

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index b144b1957dd99f7e011e7f635454f4d9215e4757..f55c437124a2287e1ae80834af5059c0f3388d38 100644 (file)
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -696,6 +696,17 @@ int nfs_flush_incompatible(struct file *file, struct page *page)
        return status;
 }
 
+/*
+ * 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.
  *
@@ -717,10 +728,13 @@ int nfs_updatepage(struct file *file, struct page *page,
                (long long)(page_offset(page) +offset));
 
        /* 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)) {
                count = max(count + offset, nfs_page_length(page));
                offset = 0;
        }