* [PATCH] ext4: revert commit which was causing fs corruption after journal replays
@ 2014-07-11 21:52 Theodore Ts'o
2014-07-14 7:23 ` Lukáš Czerner
0 siblings, 1 reply; 2+ messages in thread
From: Theodore Ts'o @ 2014-07-11 21:52 UTC (permalink / raw)
To: Ext4 Developers List; +Cc: Theodore Ts'o
Commit 007649375f6af2 ("ext4: initialize multi-block allocator before
checking block descriptors") causes the block group descriptor's count
of the number of free blocks to become inconsistent with the number of
free blocks in the allocation bitmap. This is a harmless form of fs
corruption, but it causes the kernel to potentially remount the file
system read-only, or to panic, depending on the file systems's error
behavior.
Thanks to Eric Whitney for his tireless work to reproduce and to find
the guilty commit.
Fixes: 007649375f6af2 ("ext4: initialize multi-block allocator before checking block descriptors"
Cc: stable@vger.kernel.org # 3.15
Reported-by: David Jander <david@protonic.nl>
Reported-by: Matteo Croce <technoboy85@gmail.com>
Tested-by: Eric Whitney <enwlinux@gmail.com>
Suggested-by: Eric Whitney <enwlinux@gmail.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
fs/ext4/super.c | 51 ++++++++++++++++++++++++---------------------------
1 file changed, 24 insertions(+), 27 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 6297c07..6df7bc6 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3879,38 +3879,19 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
goto failed_mount2;
}
}
-
- /*
- * set up enough so that it can read an inode,
- * and create new inode for buddy allocator
- */
- sbi->s_gdb_count = db_count;
- if (!test_opt(sb, NOLOAD) &&
- EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_HAS_JOURNAL))
- sb->s_op = &ext4_sops;
- else
- sb->s_op = &ext4_nojournal_sops;
-
- ext4_ext_init(sb);
- err = ext4_mb_init(sb);
- if (err) {
- ext4_msg(sb, KERN_ERR, "failed to initialize mballoc (%d)",
- err);
- goto failed_mount2;
- }
-
if (!ext4_check_descriptors(sb, &first_not_zeroed)) {
ext4_msg(sb, KERN_ERR, "group descriptors corrupted!");
- goto failed_mount2a;
+ goto failed_mount2;
}
if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
if (!ext4_fill_flex_info(sb)) {
ext4_msg(sb, KERN_ERR,
"unable to initialize "
"flex_bg meta info!");
- goto failed_mount2a;
+ goto failed_mount2;
}
+ sbi->s_gdb_count = db_count;
get_random_bytes(&sbi->s_next_generation, sizeof(u32));
spin_lock_init(&sbi->s_next_gen_lock);
@@ -3945,6 +3926,14 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
sbi->s_stripe = ext4_get_stripe_size(sbi);
sbi->s_extent_max_zeroout_kb = 32;
+ /*
+ * set up enough so that it can read an inode
+ */
+ if (!test_opt(sb, NOLOAD) &&
+ EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_HAS_JOURNAL))
+ sb->s_op = &ext4_sops;
+ else
+ sb->s_op = &ext4_nojournal_sops;
sb->s_export_op = &ext4_export_ops;
sb->s_xattr = ext4_xattr_handlers;
#ifdef CONFIG_QUOTA
@@ -4134,13 +4123,21 @@ no_journal:
if (err) {
ext4_msg(sb, KERN_ERR, "failed to reserve %llu clusters for "
"reserved pool", ext4_calculate_resv_clusters(sb));
- goto failed_mount5;
+ goto failed_mount4a;
}
err = ext4_setup_system_zone(sb);
if (err) {
ext4_msg(sb, KERN_ERR, "failed to initialize system "
"zone (%d)", err);
+ goto failed_mount4a;
+ }
+
+ ext4_ext_init(sb);
+ err = ext4_mb_init(sb);
+ if (err) {
+ ext4_msg(sb, KERN_ERR, "failed to initialize mballoc (%d)",
+ err);
goto failed_mount5;
}
@@ -4217,8 +4214,11 @@ failed_mount8:
failed_mount7:
ext4_unregister_li_request(sb);
failed_mount6:
- ext4_release_system_zone(sb);
+ ext4_mb_release(sb);
failed_mount5:
+ ext4_ext_release(sb);
+ ext4_release_system_zone(sb);
+failed_mount4a:
dput(sb->s_root);
sb->s_root = NULL;
failed_mount4:
@@ -4242,14 +4242,11 @@ failed_mount3:
percpu_counter_destroy(&sbi->s_extent_cache_cnt);
if (sbi->s_mmp_tsk)
kthread_stop(sbi->s_mmp_tsk);
-failed_mount2a:
- ext4_mb_release(sb);
failed_mount2:
for (i = 0; i < db_count; i++)
brelse(sbi->s_group_desc[i]);
ext4_kvfree(sbi->s_group_desc);
failed_mount:
- ext4_ext_release(sb);
if (sbi->s_chksum_driver)
crypto_free_shash(sbi->s_chksum_driver);
if (sbi->s_proc) {
--
2.0.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] ext4: revert commit which was causing fs corruption after journal replays
2014-07-11 21:52 [PATCH] ext4: revert commit which was causing fs corruption after journal replays Theodore Ts'o
@ 2014-07-14 7:23 ` Lukáš Czerner
0 siblings, 0 replies; 2+ messages in thread
From: Lukáš Czerner @ 2014-07-14 7:23 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Ext4 Developers List
On Fri, 11 Jul 2014, Theodore Ts'o wrote:
> Date: Fri, 11 Jul 2014 17:52:36 -0400
> From: Theodore Ts'o <tytso@mit.edu>
> To: Ext4 Developers List <linux-ext4@vger.kernel.org>
> Cc: Theodore Ts'o <tytso@mit.edu>
> Subject: [PATCH] ext4: revert commit which was causing fs corruption after
> journal replays
>
> Commit 007649375f6af2 ("ext4: initialize multi-block allocator before
> checking block descriptors") causes the block group descriptor's count
> of the number of free blocks to become inconsistent with the number of
> free blocks in the allocation bitmap. This is a harmless form of fs
> corruption, but it causes the kernel to potentially remount the file
> system read-only, or to panic, depending on the file systems's error
> behavior.
>
> Thanks to Eric Whitney for his tireless work to reproduce and to find
> the guilty commit.
>
> Fixes: 007649375f6af2 ("ext4: initialize multi-block allocator before checking block descriptors"
Reviewed-by: Lukas Czerner <lczerner@redhat.com>
>
> Cc: stable@vger.kernel.org # 3.15
> Reported-by: David Jander <david@protonic.nl>
> Reported-by: Matteo Croce <technoboy85@gmail.com>
> Tested-by: Eric Whitney <enwlinux@gmail.com>
> Suggested-by: Eric Whitney <enwlinux@gmail.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
> fs/ext4/super.c | 51 ++++++++++++++++++++++++---------------------------
> 1 file changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 6297c07..6df7bc6 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -3879,38 +3879,19 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> goto failed_mount2;
> }
> }
> -
> - /*
> - * set up enough so that it can read an inode,
> - * and create new inode for buddy allocator
> - */
> - sbi->s_gdb_count = db_count;
> - if (!test_opt(sb, NOLOAD) &&
> - EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_HAS_JOURNAL))
> - sb->s_op = &ext4_sops;
> - else
> - sb->s_op = &ext4_nojournal_sops;
> -
> - ext4_ext_init(sb);
> - err = ext4_mb_init(sb);
> - if (err) {
> - ext4_msg(sb, KERN_ERR, "failed to initialize mballoc (%d)",
> - err);
> - goto failed_mount2;
> - }
> -
> if (!ext4_check_descriptors(sb, &first_not_zeroed)) {
> ext4_msg(sb, KERN_ERR, "group descriptors corrupted!");
> - goto failed_mount2a;
> + goto failed_mount2;
> }
> if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG))
> if (!ext4_fill_flex_info(sb)) {
> ext4_msg(sb, KERN_ERR,
> "unable to initialize "
> "flex_bg meta info!");
> - goto failed_mount2a;
> + goto failed_mount2;
> }
>
> + sbi->s_gdb_count = db_count;
> get_random_bytes(&sbi->s_next_generation, sizeof(u32));
> spin_lock_init(&sbi->s_next_gen_lock);
>
> @@ -3945,6 +3926,14 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> sbi->s_stripe = ext4_get_stripe_size(sbi);
> sbi->s_extent_max_zeroout_kb = 32;
>
> + /*
> + * set up enough so that it can read an inode
> + */
> + if (!test_opt(sb, NOLOAD) &&
> + EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_HAS_JOURNAL))
> + sb->s_op = &ext4_sops;
> + else
> + sb->s_op = &ext4_nojournal_sops;
> sb->s_export_op = &ext4_export_ops;
> sb->s_xattr = ext4_xattr_handlers;
> #ifdef CONFIG_QUOTA
> @@ -4134,13 +4123,21 @@ no_journal:
> if (err) {
> ext4_msg(sb, KERN_ERR, "failed to reserve %llu clusters for "
> "reserved pool", ext4_calculate_resv_clusters(sb));
> - goto failed_mount5;
> + goto failed_mount4a;
> }
>
> err = ext4_setup_system_zone(sb);
> if (err) {
> ext4_msg(sb, KERN_ERR, "failed to initialize system "
> "zone (%d)", err);
> + goto failed_mount4a;
> + }
> +
> + ext4_ext_init(sb);
> + err = ext4_mb_init(sb);
> + if (err) {
> + ext4_msg(sb, KERN_ERR, "failed to initialize mballoc (%d)",
> + err);
> goto failed_mount5;
> }
>
> @@ -4217,8 +4214,11 @@ failed_mount8:
> failed_mount7:
> ext4_unregister_li_request(sb);
> failed_mount6:
> - ext4_release_system_zone(sb);
> + ext4_mb_release(sb);
> failed_mount5:
> + ext4_ext_release(sb);
> + ext4_release_system_zone(sb);
> +failed_mount4a:
> dput(sb->s_root);
> sb->s_root = NULL;
> failed_mount4:
> @@ -4242,14 +4242,11 @@ failed_mount3:
> percpu_counter_destroy(&sbi->s_extent_cache_cnt);
> if (sbi->s_mmp_tsk)
> kthread_stop(sbi->s_mmp_tsk);
> -failed_mount2a:
> - ext4_mb_release(sb);
> failed_mount2:
> for (i = 0; i < db_count; i++)
> brelse(sbi->s_group_desc[i]);
> ext4_kvfree(sbi->s_group_desc);
> failed_mount:
> - ext4_ext_release(sb);
> if (sbi->s_chksum_driver)
> crypto_free_shash(sbi->s_chksum_driver);
> if (sbi->s_proc) {
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-07-14 7:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-11 21:52 [PATCH] ext4: revert commit which was causing fs corruption after journal replays Theodore Ts'o
2014-07-14 7:23 ` Lukáš Czerner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox