Commit 06db49e68ae70cf16819b85a14057acb2820776a

Authored by Theodore Ts'o
1 parent dee1f973ca

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

Showing 3 changed files with 7 additions and 11 deletions Side-by-side Diff

... ... @@ -2063,8 +2063,7 @@
2063 2063 extern int ext4_calculate_overhead(struct super_block *sb);
2064 2064 extern int ext4_superblock_csum_verify(struct super_block *sb,
2065 2065 struct ext4_super_block *es);
2066   -extern void ext4_superblock_csum_set(struct super_block *sb,
2067   - struct ext4_super_block *es);
  2066 +extern void ext4_superblock_csum_set(struct super_block *sb);
2068 2067 extern void *ext4_kvmalloc(size_t size, gfp_t flags);
2069 2068 extern void *ext4_kvzalloc(size_t size, gfp_t flags);
2070 2069 extern void ext4_kvfree(void *ptr);
... ... @@ -143,18 +143,14 @@
143 143 struct buffer_head *bh = EXT4_SB(sb)->s_sbh;
144 144 int err = 0;
145 145  
  146 + ext4_superblock_csum_set(sb);
146 147 if (ext4_handle_valid(handle)) {
147   - ext4_superblock_csum_set(sb,
148   - (struct ext4_super_block *)bh->b_data);
149 148 err = jbd2_journal_dirty_metadata(handle, bh);
150 149 if (err)
151 150 ext4_journal_abort_handle(where, line, __func__,
152 151 bh, handle, err);
153   - } else {
154   - ext4_superblock_csum_set(sb,
155   - (struct ext4_super_block *)bh->b_data);
  152 + } else
156 153 mark_buffer_dirty(bh);
157   - }
158 154 return err;
159 155 }
... ... @@ -143,9 +143,10 @@
143 143 return es->s_checksum == ext4_superblock_csum(sb, es);
144 144 }
145 145  
146   -void ext4_superblock_csum_set(struct super_block *sb,
147   - struct ext4_super_block *es)
  146 +void ext4_superblock_csum_set(struct super_block *sb)
148 147 {
  148 + struct ext4_super_block *es = EXT4_SB(sb)->s_es;
  149 +
149 150 if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
150 151 EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
151 152 return;
... ... @@ -4387,7 +4388,7 @@
4387 4388 cpu_to_le32(percpu_counter_sum_positive(
4388 4389 &EXT4_SB(sb)->s_freeinodes_counter));
4389 4390 BUFFER_TRACE(sbh, "marking dirty");
4390   - ext4_superblock_csum_set(sb, es);
  4391 + ext4_superblock_csum_set(sb);
4391 4392 mark_buffer_dirty(sbh);
4392 4393 if (sync) {
4393 4394 error = sync_dirty_buffer(sbh);