The following patch is marked "Cc: [email protected]",
but I do not see it in any -stable release as of 3.6.6.
Ted, Greg, can you two please figure out where it got dropped?
It's pretty significant if you're using metadata_csum, so
it should probably go in. It's already in Linus' v3.7-rc3.
(06db49e68ae70cf16819b85a14057acb2820776a).
You can also add my Tested-by: if youlike.
It still applies cleanly to 3.6.6, but needs one more call site
fixed up to compile:
diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
index 71241bc..101b41c 100644
--- a/fs/ext4/resize.c
+++ b/fs/ext4/resize.c
@@ -979,7 +979,7 @@ static void update_backups(struct super_block *sb,
goto exit_err;
}
- ext4_superblock_csum_set(sb, (struct ext4_super_block *)data);
+ ext4_superblock_csum_set(sb);
while ((group = ext4_list_backups(sb, &three, &five, &seven)) < last) {
struct buffer_head *bh;
I'm not sure if you should apply the above hunk, or just cherry-pick
commit bef53b01faeb791e27605cba1a71ba21364cb23e from mainline which
removed the call entirely.
>From [email protected] Mon Oct 08 02:41:29 2012
Date: Sun, 7 Oct 2012 22:41:26 -0400
From: Theodore Ts'o <[email protected]>
To: George Spelvin <[email protected]>
Cc: [email protected], [email protected]
Subject: Re: metadata_csum + unclean shutdown = failure to boot
References: <[email protected]>
<[email protected]>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <[email protected]>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-SA-Exim-Connect-IP: <locally generated>
X-SA-Exim-Mail-From: [email protected]
X-SA-Exim-Scanned: No (on imap.thunk.org); SAEximRunCond expanded to false
I found the problem. It turns out ext4_handle_dirty_super() was
completely FUBAR'ed and was calculating the checksum on the wrong data
(for all but 1k block file systems, sigh).
We just didn't notice because the checksum would be correctly set when
the file system was unmounted cleanly. (Sigh).
The following patch should fix things. Thanks for testing out the
metadata checksum on the root file system, and reporting this
problem!!!
- Ted
>From bdd7ed290bf12c2e9132fbe97208a1af79c7a29d Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sun, 7 Oct 2012 22:18:56 -0400
Subject: [PATCH] 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 <[email protected]>
Signed-off-by: "Theodore Ts'o" <[email protected]>
Cc: [email protected]
---
fs/ext4/ext4.h | 3 +--
fs/ext4/ext4_jbd2.c | 8 ++------
fs/ext4/super.c | 7 ++++---
3 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 3ab2539..78971cf 100644
--- 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 bfa65b4..b4323ba 100644
--- 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 982f6fc..5ededf1 100644
--- 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);
--
1.7.12.rc0.22.gcdd159b
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
