[patch 1/4] introduce write_begin write_end aops important fix

[patch 1/4] introduce write_begin write_end aops important fix

[Nick Piggin]

Credit for these next 4 patches goes to Hugh. He found and fixed the
problem, I just split them up and added a bit of a changelog and 
hopefully no new bugs.

--

When running kbuild stress testing, it data corruptions on ext2 were
noticed occasionally.

The page being written to by write(2) was being unlocked in generic_write_end
before updating i_size, and that renders an extending-write vulnerable to have
its newly written data zeroed out if writepage comes at the wrong time and
finds the page unlocked but i_size is not yet updated.

Fortunately ext3 wasn't affected by this bug, but ext2 and others using
generic_write_end would be.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
Signed-off-by: Nick Piggin <npiggin@suse.de>

Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c
+++ linux-2.6/fs/buffer.c
@@ -2013,19 +2013,22 @@ int generic_write_end(struct file *file,
 
 	copied = block_write_end(file, mapping, pos, len, copied, page, fsdata);
 
-	unlock_page(page);
-	mark_page_accessed(page); /* XXX: put this in caller? */
-	page_cache_release(page);
-
 	/*
 	 * No need to use i_size_read() here, the i_size
 	 * cannot change under us because we hold i_mutex.
+	 *
+	 * But it's important to update i_size while still holding page lock:
+	 * page writeout could otherwise come in and zero beyond i_size.
 	 */
 	if (pos+copied > inode->i_size) {
 		i_size_write(inode, pos+copied);
 		mark_inode_dirty(inode);
 	}
 
+	unlock_page(page);
+	mark_page_accessed(page);
+	page_cache_release(page);
+
 	return copied;
 }
 EXPORT_SYMBOL(generic_write_end);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 2/4] reiserfs convert to new aops fix

[Nick Piggin]

Lock ordering fix for the same problem for reiserfs.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
Signed-off-by: Nick Piggin <npiggin@suse.de>

Index: linux-2.6/fs/reiserfs/inode.c
===================================================================
--- linux-2.6.orig/fs/reiserfs/inode.c
+++ linux-2.6/fs/reiserfs/inode.c
@@ -2694,9 +2694,6 @@ static int reiserfs_write_end(struct fil
 	flush_dcache_page(page);
 
 	reiserfs_commit_page(inode, page, start, start + copied);
-	unlock_page(page);
-	mark_page_accessed(page);
-	page_cache_release(page);
 
 	/* generic_commit_write does this for us, but does not update the
 	 ** transaction tracking stuff when the size changes.  So, we have
@@ -2746,6 +2743,9 @@ static int reiserfs_write_end(struct fil
 	}
 
       out:
+	unlock_page(page);
+	mark_page_accessed(page);
+	page_cache_release(page);
 	return ret == 0 ? copied : ret;
 
       journal_error:
@@ -2757,7 +2757,7 @@ static int reiserfs_write_end(struct fil
 		reiserfs_write_unlock(inode->i_sb);
 	}
 
-	return ret;
+	goto out;
 }
 
 int reiserfs_commit_write(struct file *f, struct page *page,

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 3/4] affs convert to new aops fix

[Nick Piggin]

Hugh noticed the page wasn't being unlocked at all in the affs
conversion.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
Signed-off-by: Nick Piggin <npiggin@suse.de>

Index: linux-2.6/fs/affs/file.c
===================================================================
--- linux-2.6.orig/fs/affs/file.c
+++ linux-2.6/fs/affs/file.c
@@ -13,6 +13,7 @@
  */
 
 #include "affs.h"
+#include <linux/swap.h> /* mark_page_accessed */
 
 #if PAGE_SIZE < 4096
 #error PAGE_SIZE must be at least 4096
@@ -767,6 +768,10 @@ done:
 	if (tmp > inode->i_size)
 		inode->i_size = AFFS_I(inode)->mmu_private = tmp;
 
+	unlock_page(page);
+	mark_page_accessed(page);
+	page_cache_release(page);
+
 	return written;
 
 out:

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 4/4] hostfs convert to new aops fix

[Nick Piggin]

Fix lock ordering for hostfs. It seems that this filesystem may not be
vulnerable to the bug, given that it implements its own writepage, but
it is better to retain the safe ordering.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
Signed-off-by: Nick Piggin <npiggin@suse.de>

Index: linux-2.6/fs/hostfs/hostfs_kern.c
===================================================================
--- linux-2.6.orig/fs/hostfs/hostfs_kern.c
+++ linux-2.6/fs/hostfs/hostfs_kern.c
@@ -16,6 +16,7 @@
 #include <linux/list.h>
 #include <linux/statfs.h>
 #include <linux/kdev_t.h>
+#include <linux/swap.h> /* mark_page_accessed */
 #include <asm/uaccess.h>
 #include "hostfs.h"
 #include "kern_util.h"
@@ -493,14 +494,15 @@ int hostfs_write_end(struct file *file, 
 
 	if (!PageUptodate(page) && err == PAGE_CACHE_SIZE)
 		SetPageUptodate(page);
-	unlock_page(page);
-	page_cache_release(page);
 
 	/* If err > 0, write_file has added err to pos, so we are comparing
 	 * i_size against the last byte written.
 	 */
 	if (err > 0 && (pos > inode->i_size))
 		inode->i_size = pos;
+	unlock_page(page);
+	mark_page_accessed(page);
+	page_cache_release(page);
 
 	return err;
 }

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>