linux-kernel - Re: [PATCH] ext3: wait on all pending commits in ext3_sync

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081103220144.GD18117@mit.edu>
Date:	Mon, 3 Nov 2008 17:01:44 -0500
From:	Theodore Tso <tytso@....edu>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	ajones@...erbed.com, sandeen@...hat.com,
	linux-ext4@...r.kernel.org, sct@...hat.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ext3: wait on all pending commits in ext3_sync_fs

Here's the ext4 version of the patch.  It's slightly altered from the
ext3 version in that we do reflect the errors up to sync_fs's callers
(which then throw it on the floor, but we might as well take away some
of the fun from all of those academic researchers who like to write
papers complaining about how often Linux doesn't do appropriat eerror
checking :-).

After doing some testing, I plan to carry it in the ext4 patch queue.
I think similar changes should be made to the ext3 version of the
patch; agreed?

						- Ted

commit 8106ea5364c2680a385ed240e8f898611babf661
Author: Theodore Ts'o <tytso@....edu>
Date:   Mon Nov 3 17:01:22 2008 -0500

    ext4: wait on all pending commits in ext4_sync_fs()
    
    In ext4_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.
    
    To ensure all commits are synced, we flush all journal commits now
    when sync_fs'ing ext4.
    
    Signed-off-by: Arthur Jones <ajones@...erbed.com>
    Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
    Signed-off-by: "Theodore Ts'o" <tytso@....edu>
    Cc: Eric Sandeen <sandeen@...hat.com>
    Cc: <linux-ext4@...r.kernel.org>

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 97cb896..e199773 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2899,8 +2899,9 @@ int ext4_force_commit(struct super_block *sb)
 		return 0;
 
 	journal = EXT4_SB(sb)->s_journal;
-	sb->s_dirt = 0;
 	ret = ext4_journal_force_commit(journal);
+	if (!ret)
+		sb->s_dirt = 0;
 	return ret;
 }
 
@@ -2922,15 +2923,16 @@ static void ext4_write_super(struct super_block *sb)
 
 static int ext4_sync_fs(struct super_block *sb, int wait)
 {
-	tid_t target;
+	int ret;
 
 	trace_mark(ext4_sync_fs, "dev %s wait %d", sb->s_id, wait);
-	sb->s_dirt = 0;
-	if (jbd2_journal_start_commit(EXT4_SB(sb)->s_journal, &target)) {
-		if (wait)
-			jbd2_log_wait_commit(EXT4_SB(sb)->s_journal, target);
-	}
-	return 0;
+	if (wait)
+		ret = ext4_force_commit(sb);
+	else
+		ret = jbd2_journal_start_commit(EXT4_SB(sb)->s_journal, NULL);
+	if (!ret)
+		sb->s_dirt = 0;
+	return ret;
 }
 
 /*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/