lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list] 
Message-Id: <FC3F2763-1AB0-4AEE-B63D-0E63153C0422@dilger.ca>
Date:   Sun, 3 Jun 2018 22:57:12 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Jan Kara <jack@...e.cz>
Cc:     Ted Tso <tytso@....edu>, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 10/10] e2fsck: Report only one sb corruption

On May 30, 2018, at 6:51 AM, Jan Kara <jack@...e.cz> wrote:
> 
> check_super_value() does not terminate in case of error anymore since
> c8b20b40ebf0 "misc: add plausibility checks to
> debugfs/tune2fs/dumpe2fs/e2fsck" which removed the PR_FATAL flag from
> PR_0_SB_CORRUPT problem. This results in potentially many errors for
> superblock being printed including the long message about how to deal
> with corrupted superblock. Restore the original behavior of reporting
> only one error and also remove the comments 'never get here' as they are
> not true anymore.
> 
> Signed-off-by: Jan Kara <jack@...e.cz>

Reviewed-by: Andreas Dilger <adilger@...ger.ca>

> ---
> e2fsck/super.c | 98 ++++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 58 insertions(+), 40 deletions(-)
> 
> diff --git a/e2fsck/super.c b/e2fsck/super.c
> index 00872b5ce5dc..26e531d5a13c 100644
> --- a/e2fsck/super.c
> +++ b/e2fsck/super.c
> @@ -24,7 +24,7 @@
> #define MAX_CHECK 2
> #define LOG2_CHECK 4
> 
> -static void check_super_value(e2fsck_t ctx, const char *descr,
> +static int check_super_value(e2fsck_t ctx, const char *descr,
> 			      unsigned long value, int flags,
> 			      unsigned long min_val, unsigned long max_val)
> {
> @@ -37,11 +37,13 @@ static void check_super_value(e2fsck_t ctx, const char *descr,
> 		pctx.num = value;
> 		pctx.str = descr;
> 		fix_problem(ctx, PR_0_MISC_CORRUPT_SUPER, &pctx);
> -		ctx->flags |= E2F_FLAG_ABORT; /* never get here! */
> +		ctx->flags |= E2F_FLAG_ABORT;
> +		return 0;
> 	}
> +	return 1;
> }
> 
> -static void check_super_value64(e2fsck_t ctx, const char *descr,
> +static int check_super_value64(e2fsck_t ctx, const char *descr,
> 				__u64 value, int flags,
> 				__u64 min_val, __u64 max_val)
> {
> @@ -54,8 +56,10 @@ static void check_super_value64(e2fsck_t ctx, const char *descr,
> 		pctx.num = value;
> 		pctx.str = descr;
> 		fix_problem(ctx, PR_0_MISC_CORRUPT_SUPER, &pctx);
> -		ctx->flags |= E2F_FLAG_ABORT; /* never get here! */
> +		ctx->flags |= E2F_FLAG_ABORT;
> +		return 0;
> 	}
> +	return 1;
> }
> 
> /*
> @@ -618,34 +622,46 @@ void check_super_block(e2fsck_t ctx)
> 	/*
> 	 * Verify the super block constants...
> 	 */
> -	check_super_value(ctx, "inodes_count", sb->s_inodes_count,
> -			  MIN_CHECK, 1, 0);
> -	check_super_value64(ctx, "blocks_count", ext2fs_blocks_count(sb),
> -			    MIN_CHECK | MAX_CHECK, 1, blks_max);
> -	check_super_value(ctx, "first_data_block", sb->s_first_data_block,
> -			  MAX_CHECK, 0, ext2fs_blocks_count(sb));
> -	check_super_value(ctx, "log_block_size", sb->s_log_block_size,
> -			  MIN_CHECK | MAX_CHECK, 0,
> -			  EXT2_MAX_BLOCK_LOG_SIZE - EXT2_MIN_BLOCK_LOG_SIZE);
> -	check_super_value(ctx, "log_cluster_size",
> -			  sb->s_log_cluster_size,
> -			  MIN_CHECK | MAX_CHECK, sb->s_log_block_size,
> -			  (EXT2_MAX_CLUSTER_LOG_SIZE -
> -			   EXT2_MIN_CLUSTER_LOG_SIZE));
> -	check_super_value(ctx, "clusters_per_group", sb->s_clusters_per_group,
> -			  MIN_CHECK | MAX_CHECK, 8, cpg_max);
> -	check_super_value(ctx, "blocks_per_group", sb->s_blocks_per_group,
> -			  MIN_CHECK | MAX_CHECK, 8, bpg_max);
> -	check_super_value(ctx, "inodes_per_group", sb->s_inodes_per_group,
> -			  MIN_CHECK | MAX_CHECK, inodes_per_block, ipg_max);
> -	check_super_value(ctx, "r_blocks_count", ext2fs_r_blocks_count(sb),
> -			  MAX_CHECK, 0, ext2fs_blocks_count(sb) / 2);
> -	check_super_value(ctx, "reserved_gdt_blocks",
> -			  sb->s_reserved_gdt_blocks, MAX_CHECK, 0,
> -			  fs->blocksize / sizeof(__u32));
> -	check_super_value(ctx, "desc_size",
> -			  sb->s_desc_size, MAX_CHECK | LOG2_CHECK, 0,
> -			  EXT2_MAX_DESC_SIZE);
> +	if (!check_super_value(ctx, "inodes_count", sb->s_inodes_count,
> +			       MIN_CHECK, 1, 0))
> +		return;
> +	if (!check_super_value64(ctx, "blocks_count", ext2fs_blocks_count(sb),
> +				 MIN_CHECK | MAX_CHECK, 1, blks_max))
> +		return;
> +	if (!check_super_value(ctx, "first_data_block", sb->s_first_data_block,
> +			       MAX_CHECK, 0, ext2fs_blocks_count(sb)))
> +		return;
> +	if (!check_super_value(ctx, "log_block_size", sb->s_log_block_size,
> +			       MIN_CHECK | MAX_CHECK, 0,
> +			       EXT2_MAX_BLOCK_LOG_SIZE - EXT2_MIN_BLOCK_LOG_SIZE))
> +		return;
> +	if (!check_super_value(ctx, "log_cluster_size",
> +			       sb->s_log_cluster_size,
> +			       MIN_CHECK | MAX_CHECK, sb->s_log_block_size,
> +			       (EXT2_MAX_CLUSTER_LOG_SIZE -
> +			        EXT2_MIN_CLUSTER_LOG_SIZE)))
> +		return;
> +	if (!check_super_value(ctx, "clusters_per_group",
> +			       sb->s_clusters_per_group,
> +			       MIN_CHECK | MAX_CHECK, 8, cpg_max))
> +		return;
> +	if (!check_super_value(ctx, "blocks_per_group", sb->s_blocks_per_group,
> +			       MIN_CHECK | MAX_CHECK, 8, bpg_max))
> +		return;
> +	if (!check_super_value(ctx, "inodes_per_group", sb->s_inodes_per_group,
> +			       MIN_CHECK | MAX_CHECK, inodes_per_block, ipg_max))
> +		return;
> +	if (!check_super_value(ctx, "r_blocks_count", ext2fs_r_blocks_count(sb),
> +			       MAX_CHECK, 0, ext2fs_blocks_count(sb) / 2))
> +		return;
> +	if (!check_super_value(ctx, "reserved_gdt_blocks",
> +			       sb->s_reserved_gdt_blocks, MAX_CHECK, 0,
> +			       fs->blocksize / sizeof(__u32)))
> +		return;
> +	if (!check_super_value(ctx, "desc_size",
> +			       sb->s_desc_size, MAX_CHECK | LOG2_CHECK, 0,
> +			       EXT2_MAX_DESC_SIZE))
> +		return;
> 
> 	should_be = (blk64_t)sb->s_inodes_per_group * fs->group_desc_count;
> 	if (should_be > UINT_MAX) {
> @@ -662,20 +678,22 @@ void check_super_block(e2fsck_t ctx)
> 			ext2fs_mark_super_dirty(fs);
> 		}
> 	}
> -	if (sb->s_rev_level > EXT2_GOOD_OLD_REV)
> -		check_super_value(ctx, "first_ino", sb->s_first_ino,
> -				  MIN_CHECK | MAX_CHECK,
> -				  EXT2_GOOD_OLD_FIRST_INO, sb->s_inodes_count);
> +	if (sb->s_rev_level > EXT2_GOOD_OLD_REV &&
> +	    !check_super_value(ctx, "first_ino", sb->s_first_ino,
> +			       MIN_CHECK | MAX_CHECK,
> +			       EXT2_GOOD_OLD_FIRST_INO, sb->s_inodes_count))
> +		return;
> 	inode_size = EXT2_INODE_SIZE(sb);
> -	check_super_value(ctx, "inode_size",
> -			  inode_size, MIN_CHECK | MAX_CHECK | LOG2_CHECK,
> -			  EXT2_GOOD_OLD_INODE_SIZE, fs->blocksize);
> +	if (!check_super_value(ctx, "inode_size",
> +			       inode_size, MIN_CHECK | MAX_CHECK | LOG2_CHECK,
> +			       EXT2_GOOD_OLD_INODE_SIZE, fs->blocksize))
> +		return;
> 	if (sb->s_blocks_per_group != (sb->s_clusters_per_group *
> 				       EXT2FS_CLUSTER_RATIO(fs))) {
> 		pctx.num = sb->s_clusters_per_group * EXT2FS_CLUSTER_RATIO(fs);
> 		pctx.str = "block_size";
> 		fix_problem(ctx, PR_0_MISC_CORRUPT_SUPER, &pctx);
> -		ctx->flags |= E2F_FLAG_ABORT; /* never get here! */
> +		ctx->flags |= E2F_FLAG_ABORT;
> 		return;
> 	}
> 
> --
> 2.13.6
> 


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ