ext3: wait on all pending commits in ext3_sync_fs

From: Arthur Jones <[email protected]>

In ext3_sync_fs, we only wait for a commit to finish if we started it, but there may be one already in progress which will not be synced.

In the case of a data=ordered umount with pending long symlinks which are delayed due to a long list of other I/O on the backing block device, this causes the buffer associated with the long symlinks to not be moved to the inode dirty list in the second phase of fsync_super. Then, before they can be dirtied again, kjournald exits, seeing the UMOUNT flag and the dirty pages are never written to the backing block device, causing long symlink corruption and exposing new or previously freed block data to userspace.

This can be reproduced with a script created
by Eric Sandeen <[email protected]>:

	#!/bin/bash

	umount /mnt/test2
	mount /dev/sdb4 /mnt/test2
	rm -f /mnt/test2/*
	dd if=/dev/zero of=/mnt/test2/bigfile bs=1M count=512
	touch
	/mnt/test2/thisisveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylongfilename
	ln -s
	/mnt/test2/thisisveryveryveryveryveryveryveryveryveryveryveryveryveryveryveryverylongfilename
	/mnt/test2/link
	umount /mnt/test2
	mount /dev/sdb4 /mnt/test2
	ls /mnt/test2/
	umount /mnt/test2

To ensure all commits are synced, we flush all journal commits now when sync_fs'ing ext3.

---

 fs/ext3/super.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff -puN fs/ext3/super.c~ext3-wait-on-all-pending-commits-in-ext3_sync_fs fs/ext3/super.c
@@ -2390,13 +2390,12 @@ static void ext3_write_super (struct sup
 
 static int ext3_sync_fs(struct super_block *sb, int wait)
 {
-	tid_t target;

My version of this patch also cleaned up the following comment, which has been wrong since 2.5.70 or thereabouts...

This isn't urgent, so could you just queue this up for the next merge window in the -mm tree?

- Ted

ext3: Clean up outdated and incorrect comment for ext3_write_super()

---

index e5717a4..296c044 100644
@@ -2375,12 +2375,9 @@ int ext3_force_commit(struct super_block *sb)
 /*
  * Ext3 always journals updates to the superblock itself, so we don't
  * have to propagate any other updates to the superblock on disk at this
- * point.  Just start an async writeback to get the buffers on their way
- * to the disk.
- *
- * This implicitly triggers the writebehind on sync().
+ * point.  (We can probably nuke this function altogether, and remove
+ * any mention to sb->s_dirt in all of fs/ext3; eventual cleanup...)
  */

oh, I didn't spot that.

I looked at the version in linux-next and saw that it was propagating the error value back to the VFS as well. Or maybe that was done in a separate patch, dunno. But while that's a good change, I felt that we should separate it from this bugfix. I meant to mention it but I forgot, sorry.

Fair enough. Propagating the return value to the VFS is also a cleanup, although given that the VFS drops return value on the floor, I considered it a risk-free change and included it in the ext4 version of the patch. But yeah, I can see why separating could be argued to be the right thing.

So that means we'll need two cleanup patches for ext3; one to fix the comment, and another to propagate the error code back to the VFS.

- Ted

Hi Ted, ...

A trivial patch for this is in the linux-ext4
mailing list:

http://marc.info/?l=linux-kernel&m=122574905024321&w=2

Arthur