ext4: fix metadata checksum calculation for the superblock

The function ext4_handle_dirty_super() was calculating the superblock
on the wrong block data.  As a result, when the superblock is modified
while it is mounted (most commonly, when inodes are added or removed
from the orphan list), the superblock checksum would be wrong.  We
didn't notice because the superblock *was* being correctly calculated
in ext4_commit_super(), and this would get called when the file system
was unmounted.  So the problem only became obvious if the system
crashed while the file system was mounted.

Fix this by removing the poorly designed function signature for
ext4_superblock_csum_set(); if it only took a single argument, the
pointer to a struct superblock, the ambiguity which caused this
mistake would have been impossible.

Reported-by: George Spelvin <linux@horizon.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Cc: stable@vger.kernel.org

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 3ab2539b7b2eb477222a3dbb7dd8b51cea3cca84..78971cfd9c7f27a50464848ff7d62099dcfd92f6 100644 (file)
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2063,8 +2063,7 @@ extern int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count);
 extern int ext4_calculate_overhead(struct super_block *sb);
 extern int ext4_superblock_csum_verify(struct super_block *sb,
                                       struct ext4_super_block *es);
-extern void ext4_superblock_csum_set(struct super_block *sb,
-                                    struct ext4_super_block *es);
+extern void ext4_superblock_csum_set(struct super_block *sb);
 extern void *ext4_kvmalloc(size_t size, gfp_t flags);
 extern void *ext4_kvzalloc(size_t size, gfp_t flags);
 extern void ext4_kvfree(void *ptr);

diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
index bfa65b49d4243d75a5c73456fc2ee4e3698537f4..b4323ba846b52ad09fe0740699ea2d82ff92ba65 100644 (file)
--- a/fs/ext4/ext4_jbd2.c
+++ b/fs/ext4/ext4_jbd2.c
@@ -143,17 +143,13 @@ int __ext4_handle_dirty_super(const char *where, unsigned int line,
        struct buffer_head *bh = EXT4_SB(sb)->s_sbh;
        int err = 0;
 
+       ext4_superblock_csum_set(sb);
        if (ext4_handle_valid(handle)) {
-               ext4_superblock_csum_set(sb,
-                               (struct ext4_super_block *)bh->b_data);
                err = jbd2_journal_dirty_metadata(handle, bh);
                if (err)
                        ext4_journal_abort_handle(where, line, __func__,
                                                  bh, handle, err);
-       } else {
-               ext4_superblock_csum_set(sb,
-                               (struct ext4_super_block *)bh->b_data);
+       } else
                mark_buffer_dirty(bh);
-       }
        return err;
 }

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 982f6fc22c889ed28a182da483950d6e004708b8..5ededf135335ea804fb2efea55b60b865ba53769 100644 (file)
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -143,9 +143,10 @@ int ext4_superblock_csum_verify(struct super_block *sb,
        return es->s_checksum == ext4_superblock_csum(sb, es);
 }
 
-void ext4_superblock_csum_set(struct super_block *sb,
-                             struct ext4_super_block *es)
+void ext4_superblock_csum_set(struct super_block *sb)
 {
+       struct ext4_super_block *es = EXT4_SB(sb)->s_es;
+
        if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
                EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
                return;
@@ -4387,7 +4388,7 @@ static int ext4_commit_super(struct super_block *sb, int sync)
                cpu_to_le32(percpu_counter_sum_positive(
                                &EXT4_SB(sb)->s_freeinodes_counter));
        BUFFER_TRACE(sbh, "marking dirty");
-       ext4_superblock_csum_set(sb, es);
+       ext4_superblock_csum_set(sb);
        mark_buffer_dirty(sbh);
        if (sync) {
                error = sync_dirty_buffer(sbh);