Re: [PATCH] ext4: revert commit which was causing fs corruption after journal replays

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 11 Jul 2014, Theodore Ts'o wrote:

> Date: Fri, 11 Jul 2014 17:52:36 -0400
> From: Theodore Ts'o <tytso@xxxxxxx>
> To: Ext4 Developers List <linux-ext4@xxxxxxxxxxxxxxx>
> Cc: Theodore Ts'o <tytso@xxxxxxx>
> 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@xxxxxxxxxx>

> 
> Cc: stable@xxxxxxxxxxxxxxx  # 3.15
> Reported-by: David Jander <david@xxxxxxxxxxx>
> Reported-by: Matteo Croce <technoboy85@xxxxxxxxx>
> Tested-by: Eric Whitney <enwlinux@xxxxxxxxx>
> Suggested-by: Eric Whitney <enwlinux@xxxxxxxxx>
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> ---
>  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) {
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux