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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <13F2FE76-C681-4E45-803A-7E5B46BF196A@dilger.ca>
Date:   Thu, 19 Apr 2018 14:42:36 -0600
From:   Andreas Dilger <adilger@...ger.ca>
To:     Wang Shilong <wangshilong1991@...il.com>
Cc:     Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Wang Shilong <wshilong@....com>,
        Shuichi Ihara <sihara@....com>, tytso@....edu
Subject: Re: [PATCH 1/2] ext4: wrap error handling and corrupted bitmaps
 setting

On Apr 19, 2018, at 7:39 AM, Wang Shilong <wangshilong1991@...il.com> wrote:
> 
> From: Wang Shilong <wshilong@....com>
> 
> Cleanup to handle ext4 error codes together with
> bitmaps corrupted bits setting.
> 
> Signed-off-by: Wang Shilong <wshilong@....com>

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

> ---
> fs/ext4/balloc.c  | 30 +++++++++++++++++-------------
> fs/ext4/ext4.h    | 35 ++++++++++++++++++++++++-----------
> fs/ext4/ialloc.c  | 37 +++++++++++++++++++------------------
> fs/ext4/mballoc.c | 20 ++++++++------------
> fs/ext4/super.c   | 11 +++++++----
> 5 files changed, 75 insertions(+), 58 deletions(-)
> 
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index 8ef65251d019..76ae05993390 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -193,7 +193,9 @@ static int ext4_init_block_bitmap(struct super_block *sb,
> 	if (!ext4_group_desc_csum_verify(sb, block_group, gdp)) {
> 		ext4_mark_group_bitmap_corrupted(sb, block_group,
> 					EXT4_GROUP_INFO_BBITMAP_CORRUPT |
> -					EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> +					EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> +					"bg %u: bad block bitmap checksum",
> +					block_group);
> 		return -EFSBADCRC;
> 	}
> 	memset(bh->b_data, 0, sb->s_blocksize);
> @@ -374,18 +376,19 @@ static int ext4_validate_block_bitmap(struct super_block *sb,
> 	if (unlikely(!ext4_block_bitmap_csum_verify(sb, block_group,
> 			desc, bh))) {
> 		ext4_unlock_group(sb, block_group);
> -		ext4_error(sb, "bg %u: bad block bitmap checksum", block_group);
> 		ext4_mark_group_bitmap_corrupted(sb, block_group,
> -					EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> +					EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> +					"bg %u: bad block bitmap checksum",
> +					block_group);
> 		return -EFSBADCRC;
> 	}
> 	blk = ext4_valid_block_bitmap(sb, desc, block_group, bh);
> 	if (unlikely(blk != 0)) {
> 		ext4_unlock_group(sb, block_group);
> -		ext4_error(sb, "bg %u: block %llu: invalid block bitmap",
> -			   block_group, blk);
> 		ext4_mark_group_bitmap_corrupted(sb, block_group,
> -					EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> +				EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> +				"bg %u: block %llu: invalid block bitmap",
> +				block_group, blk);
> 		return -EFSCORRUPTED;
> 	}
> 	set_buffer_verified(bh);
> @@ -418,10 +421,10 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
> 	bitmap_blk = ext4_block_bitmap(sb, desc);
> 	if ((bitmap_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
> 	    (bitmap_blk >= ext4_blocks_count(sbi->s_es))) {
> -		ext4_error(sb, "Invalid block bitmap block %llu in "
> -			   "block_group %u", bitmap_blk, block_group);
> 		ext4_mark_group_bitmap_corrupted(sb, block_group,
> -					EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> +				EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> +				"Invalid block bitmap block %llu in "
> +				"block_group %u", bitmap_blk, block_group);
> 		return ERR_PTR(-EFSCORRUPTED);
> 	}
> 	bh = sb_getblk(sb, bitmap_blk);
> @@ -497,11 +500,12 @@ int ext4_wait_block_bitmap(struct super_block *sb, ext4_group_t block_group,
> 		return -EFSCORRUPTED;
> 	wait_on_buffer(bh);
> 	if (!buffer_uptodate(bh)) {
> -		ext4_error(sb, "Cannot read block bitmap - "
> -			   "block_group = %u, block_bitmap = %llu",
> -			   block_group, (unsigned long long) bh->b_blocknr);
> 		ext4_mark_group_bitmap_corrupted(sb, block_group,
> -					EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> +				EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> +				"Cannot read block bitmap - "
> +				"block_group = %u, block_bitmap = %llu",
> +				block_group,
> +				(unsigned long long)bh->b_blocknr);
> 		return -EIO;
> 	}
> 	clear_buffer_new(bh);
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index fa52b7dd4542..ed5b988d3f8e 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2530,9 +2530,9 @@ extern int ext4_alloc_flex_bg_array(struct super_block *sb,
> 				    ext4_group_t ngroup);
> extern const char *ext4_decode_error(struct super_block *sb, int errno,
> 				     char nbuf[16]);
> -extern void ext4_mark_group_bitmap_corrupted(struct super_block *sb,
> -					     ext4_group_t block_group,
> -					     unsigned int flags);
> +extern void __ext4_mark_group_bitmap_corrupted(struct super_block *sb,
> +					       ext4_group_t block_group,
> +					       unsigned int flags);
> 
> extern __printf(4, 5)
> void __ext4_error(struct super_block *, const char *, unsigned int,
> @@ -2558,10 +2558,10 @@ extern __printf(3, 4)
> void __ext4_msg(struct super_block *, const char *, const char *, ...);
> extern void __dump_mmp_msg(struct super_block *, struct mmp_struct *mmp,
> 			   const char *, unsigned int, const char *);
> -extern __printf(7, 8)
> +extern __printf(8, 9)
> void __ext4_grp_locked_error(const char *, unsigned int,
> 			     struct super_block *, ext4_group_t,
> -			     unsigned long, ext4_fsblk_t,
> +			     unsigned long, ext4_fsblk_t, unsigned int,
> 			     const char *, ...);
> 
> #define EXT4_ERROR_INODE(inode, fmt, a...) \
> @@ -2591,9 +2591,16 @@ void __ext4_grp_locked_error(const char *, unsigned int,
> 	__ext4_msg(sb, level, fmt, ##__VA_ARGS__)
> #define dump_mmp_msg(sb, mmp, msg)					\
> 	__dump_mmp_msg(sb, mmp, __func__, __LINE__, msg)
> -#define ext4_grp_locked_error(sb, grp, ino, block, fmt, ...)		\
> -	__ext4_grp_locked_error(__func__, __LINE__, sb, grp, ino, block, \
> -				fmt, ##__VA_ARGS__)
> +#define ext4_grp_locked_error(sb, grp, ino, block, flags, fmt, ...)	\
> +	__ext4_grp_locked_error(__func__, __LINE__, sb, grp, ino, block,\
> +				flags, fmt, ##__VA_ARGS__)
> +#define ext4_mark_group_bitmap_corrupted(sb, group, flags, fmt, ...)	\
> +	do {								\
> +		__ext4_error(sb, __func__, __LINE__,			\
> +			     fmt, ##__VA_ARGS__);			\
> +		__ext4_mark_group_bitmap_corrupted(sb, group, flags);	\
> +	} while (0)
> +
> 
> #else
> 
> @@ -2634,11 +2641,17 @@ do {									\
> } while (0)
> #define dump_mmp_msg(sb, mmp, msg)					\
> 	__dump_mmp_msg(sb, mmp, "", 0, "")
> -#define ext4_grp_locked_error(sb, grp, ino, block, fmt, ...)		\
> +#define ext4_grp_locked_error(sb, grp, ino, block, flags, fmt, ...)	\
> do {									\
> -	no_printk(fmt, ##__VA_ARGS__);				\
> -	__ext4_grp_locked_error("", 0, sb, grp, ino, block, " ");	\
> +	no_printk(fmt, ##__VA_ARGS__);					\
> +	__ext4_grp_locked_error("", 0, sb, grp, ino, block, flags, " ");\
> } while (0)
> +#define ext4_mark_group_bitmap_corrupted(sb, group, flags, fmt, ...)	\
> +	do {								\
> +		no_printk(fmt, ##__VA_ARGS__);				\
> +		__ext4_error(sb, "", 0, " ");				\
> +		__ext4_mark_group_bitmap_corrupted(sb, group, flags);	\
> +	} while (0)
> 
> #endif
> 
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 4d6e007f3569..b7f729995978 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -94,10 +94,10 @@ static int ext4_validate_inode_bitmap(struct super_block *sb,
> 	if (!ext4_inode_bitmap_csum_verify(sb, block_group, desc, bh,
> 					   EXT4_INODES_PER_GROUP(sb) / 8)) {
> 		ext4_unlock_group(sb, block_group);
> -		ext4_error(sb, "Corrupt inode bitmap - block_group = %u, "
> -			   "inode_bitmap = %llu", block_group, blk);
> 		ext4_mark_group_bitmap_corrupted(sb, block_group,
> -					EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> +				EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> +				"Corrupt inode bitmap - block_group = %u, "
> +				"inode_bitmap = %llu", block_group, blk);
> 		return -EFSBADCRC;
> 	}
> 	set_buffer_verified(bh);
> @@ -127,10 +127,10 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
> 	bitmap_blk = ext4_inode_bitmap(sb, desc);
> 	if ((bitmap_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
> 	    (bitmap_blk >= ext4_blocks_count(sbi->s_es))) {
> -		ext4_error(sb, "Invalid inode bitmap blk %llu in "
> -			   "block_group %u", bitmap_blk, block_group);
> 		ext4_mark_group_bitmap_corrupted(sb, block_group,
> -					EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> +				EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> +				"Invalid inode bitmap blk %llu in "
> +				"block_group %u", bitmap_blk, block_group);
> 		return ERR_PTR(-EFSCORRUPTED);
> 	}
> 	bh = sb_getblk(sb, bitmap_blk);
> @@ -182,11 +182,11 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
> 	wait_on_buffer(bh);
> 	if (!buffer_uptodate(bh)) {
> 		put_bh(bh);
> -		ext4_error(sb, "Cannot read inode bitmap - "
> -			   "block_group = %u, inode_bitmap = %llu",
> -			   block_group, bitmap_blk);
> 		ext4_mark_group_bitmap_corrupted(sb, block_group,
> -				EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> +				EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> +				"Cannot read inode bitmap - "
> +				"block_group = %u, inode_bitmap = %llu",
> +				block_group, bitmap_blk);
> 		return ERR_PTR(-EIO);
> 	}
> 
> @@ -333,9 +333,10 @@ void ext4_free_inode(handle_t *handle, struct inode *inode)
> 		if (!fatal)
> 			fatal = err;
> 	} else {
> -		ext4_error(sb, "bit already cleared for inode %lu", ino);
> 		ext4_mark_group_bitmap_corrupted(sb, block_group,
> -					EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> +				EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> +				"bit already cleared for inode %lu",
> +				ino);
> 	}
> 
> error_return:
> @@ -904,10 +905,10 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
> 			goto next_group;
> 
> 		if (group == 0 && (ino + 1) < EXT4_FIRST_INO(sb)) {
> -			ext4_error(sb, "reserved inode found cleared - "
> -				   "inode=%lu", ino + 1);
> 			ext4_mark_group_bitmap_corrupted(sb, group,
> -					EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> +				EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> +				"reserved inode found cleared - "
> +				"inode=%lu", ino + 1);
> 			goto next_group;
> 		}
> 
> @@ -1097,10 +1098,10 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
> 		 * twice.
> 		 */
> 		err = -EIO;
> -		ext4_error(sb, "failed to insert inode %lu: doubly allocated?",
> -			   inode->i_ino);
> 		ext4_mark_group_bitmap_corrupted(sb, group,
> -					EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> +				EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> +				"failed to insert inode %lu: doubly allocated?",
> +				inode->i_ino);
> 		goto out;
> 	}
> 	inode->i_generation = prandom_u32();
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 0d473991eebd..065bce671de4 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -467,11 +467,10 @@ static void mb_free_blocks_double(struct inode *inode, struct ext4_buddy *e4b,
> 			ext4_grp_locked_error(sb, e4b->bd_group,
> 					      inode ? inode->i_ino : 0,
> 					      blocknr,
> +					      EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> 					      "freeing block already freed "
> 					      "(bit %u)",
> 					      first + i);
> -			ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
> -					EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> 		}
> 		mb_clear_bit(first + i, e4b->bd_info->bb_bitmap);
> 	}
> @@ -741,6 +740,7 @@ void ext4_mb_generate_buddy(struct super_block *sb,
> 
> 	if (free != grp->bb_free) {
> 		ext4_grp_locked_error(sb, group, 0, 0,
> +				      EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> 				      "block bitmap and bg descriptor "
> 				      "inconsistent: %u vs %u free clusters",
> 				      free, grp->bb_free);
> @@ -749,8 +749,6 @@ void ext4_mb_generate_buddy(struct super_block *sb,
> 		 * corrupt and update bb_free using bitmap value
> 		 */
> 		grp->bb_free = free;
> -		ext4_mark_group_bitmap_corrupted(sb, group,
> -					EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> 	}
> 	mb_set_largest_free_order(sb, grp);
> 
> @@ -1451,11 +1449,10 @@ static void mb_free_blocks(struct inode *inode, struct ext4_buddy *e4b,
> 		ext4_grp_locked_error(sb, e4b->bd_group,
> 				      inode ? inode->i_ino : 0,
> 				      blocknr,
> +				      EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> 				      "freeing already freed block "
> 				      "(bit %u); block bitmap corrupt.",
> 				      block);
> -		ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
> -				EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> 		mb_regenerate_buddy(e4b);
> 		goto done;
> 	}
> @@ -1949,11 +1946,10 @@ void ext4_mb_complex_scan_group(struct ext4_allocation_context *ac,
> 			 * we have free blocks
> 			 */
> 			ext4_grp_locked_error(sb, e4b->bd_group, 0, 0,
> +					EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> 					"%d free clusters as per "
> 					"group info. But bitmap says 0",
> 					free);
> -			ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
> -					EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> 			break;
> 		}
> 
> @@ -1961,11 +1957,10 @@ void ext4_mb_complex_scan_group(struct ext4_allocation_context *ac,
> 		BUG_ON(ex.fe_len <= 0);
> 		if (free < ex.fe_len) {
> 			ext4_grp_locked_error(sb, e4b->bd_group, 0, 0,
> +					EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> 					"%d free clusters as per "
> 					"group info. But got %d blocks",
> 					free, ex.fe_len);
> -			ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
> -					EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> 			/*
> 			 * The number of free blocks differs. This mostly
> 			 * indicate that the bitmap is corrupt. So exit
> @@ -3851,7 +3846,8 @@ ext4_mb_release_inode_pa(struct ext4_buddy *e4b, struct buffer_head *bitmap_bh,
> 			 pa, (unsigned long) pa->pa_lstart,
> 			 (unsigned long) pa->pa_pstart,
> 			 (unsigned long) pa->pa_len);
> -		ext4_grp_locked_error(sb, group, 0, 0, "free %u, pa_free %u",
> +		ext4_grp_locked_error(sb, group, 0, 0, 0,
> +					"free %u, pa_free %u",
> 					free, pa->pa_free);
> 		/*
> 		 * pa is already deleted so we use the value obtained
> @@ -4679,7 +4675,7 @@ ext4_mb_free_metadata(handle_t *handle, struct ext4_buddy *e4b,
> 		else {
> 			ext4_grp_locked_error(sb, group, 0,
> 				ext4_group_first_block_no(sb, group) +
> -				EXT4_C2B(sbi, cluster),
> +				EXT4_C2B(sbi, cluster), 0,
> 				"Block already on to-be-freed list");
> 			return 0;
> 		}
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index f5e9a1af067a..96981df03385 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -709,7 +709,7 @@ void __ext4_warning_inode(const struct inode *inode, const char *function,
> void __ext4_grp_locked_error(const char *function, unsigned int line,
> 			     struct super_block *sb, ext4_group_t grp,
> 			     unsigned long ino, ext4_fsblk_t block,
> -			     const char *fmt, ...)
> +			     unsigned int flags, const char *fmt, ...)
> __releases(bitlock)
> __acquires(bitlock)
> {
> @@ -740,6 +740,9 @@ __acquires(bitlock)
> 		va_end(args);
> 	}
> 
> +	if (flags)
> +		__ext4_mark_group_bitmap_corrupted(sb, grp, flags);
> +
> 	if (test_opt(sb, ERRORS_CONT)) {
> 		ext4_commit_super(sb, 0);
> 		return;
> @@ -763,9 +766,9 @@ __acquires(bitlock)
> 	return;
> }
> 
> -void ext4_mark_group_bitmap_corrupted(struct super_block *sb,
> -				     ext4_group_t group,
> -				     unsigned int flags)
> +void __ext4_mark_group_bitmap_corrupted(struct super_block *sb,
> +					ext4_group_t group,
> +					unsigned int flags)
> {
> 	struct ext4_sb_info *sbi = EXT4_SB(sb);
> 	struct ext4_group_info *grp = ext4_get_group_info(sb, group);
> --
> 2.14.3
> 


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