Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] ext4: fix check to prevent false positive report of incorrect used inodes
@ 2021-03-31 12:15 Zhang Yi
  2021-03-31 12:23 ` Jan Kara
  0 siblings, 1 reply; 3+ messages in thread
From: Zhang Yi @ 2021-03-31 12:15 UTC (permalink / raw)
  To: linux-ext4; +Cc: tytso, adilger.kernel, jack, yi.zhang

Commit <50122847007> ("ext4: fix check to prevent initializing reserved
inodes") check the block group zero and prevent initializing reserved
inodes. But in some special cases, the reserved inode may not all belong
to the group zero, it may exist into the second group if we format
filesystem below.

  mkfs.ext4 -b 4096 -g 8192 -N 1024 -I 4096 /dev/sda

So, it will end up triggering a false positive report of a corrupted
file system. This patch fix it by avoid check reserved inodes if no free
inode blocks will be zeroed.

Fixes: 50122847007 ("ext4: fix check to prevent initializing reserved inodes")
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Suggested-by: Jan Kara <jack@suse.cz>
---
Changes since v1:
 - Splict check of used_blks and used_inos to make code more
   comprehensible as Jan suggested.

 fs/ext4/ialloc.c | 48 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 633ae7becd61..5f0c7fe32672 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1513,6 +1513,7 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
 	handle_t *handle;
 	ext4_fsblk_t blk;
 	int num, ret = 0, used_blks = 0;
+	unsigned long used_inos = 0;
 
 	/* This should not happen, but just to be sure check this */
 	if (sb_rdonly(sb)) {
@@ -1543,22 +1544,37 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
 	 * used inodes so we need to skip blocks with used inodes in
 	 * inode table.
 	 */
-	if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)))
-		used_blks = DIV_ROUND_UP((EXT4_INODES_PER_GROUP(sb) -
-			    ext4_itable_unused_count(sb, gdp)),
-			    sbi->s_inodes_per_block);
-
-	if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group) ||
-	    ((group == 0) && ((EXT4_INODES_PER_GROUP(sb) -
-			       ext4_itable_unused_count(sb, gdp)) <
-			      EXT4_FIRST_INO(sb)))) {
-		ext4_error(sb, "Something is wrong with group %u: "
-			   "used itable blocks: %d; "
-			   "itable unused count: %u",
-			   group, used_blks,
-			   ext4_itable_unused_count(sb, gdp));
-		ret = 1;
-		goto err_out;
+	if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT))) {
+		used_inos = EXT4_INODES_PER_GROUP(sb) -
+			    ext4_itable_unused_count(sb, gdp);
+		used_blks = DIV_ROUND_UP(used_inos, sbi->s_inodes_per_block);
+
+		/* Bogus inode unused count? */
+		if (used_blks < 0 || used_blks > sbi->s_itb_per_group) {
+			ext4_error(sb, "Something is wrong with group %u: "
+				   "used itable blocks: %d; "
+				   "itable unused count: %u",
+				   group, used_blks,
+				   ext4_itable_unused_count(sb, gdp));
+			ret = 1;
+			goto err_out;
+		}
+
+		used_inos += group * EXT4_INODES_PER_GROUP(sb);
+		/*
+		 * Are there some uninitialized inodes in the inode table
+		 * before the first normal inode?
+		 */
+		if ((used_blks != sbi->s_itb_per_group) &&
+		     (used_inos < EXT4_FIRST_INO(sb))) {
+			ext4_error(sb, "Something is wrong with group %u: "
+				   "itable unused count: %u; "
+				   "itables initialized count: %ld",
+				   group, ext4_itable_unused_count(sb, gdp),
+				   used_inos);
+			ret = 1;
+			goto err_out;
+		}
 	}
 
 	blk = ext4_inode_table(sb, gdp) + used_blks;
-- 
2.25.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] ext4: fix check to prevent false positive report of incorrect used inodes
  2021-03-31 12:15 [PATCH v2] ext4: fix check to prevent false positive report of incorrect used inodes Zhang Yi
@ 2021-03-31 12:23 ` Jan Kara
  2021-04-09 15:32   ` Theodore Ts'o
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Kara @ 2021-03-31 12:23 UTC (permalink / raw)
  To: Zhang Yi; +Cc: linux-ext4, tytso, adilger.kernel, jack

On Wed 31-03-21 20:15:16, Zhang Yi wrote:
> Commit <50122847007> ("ext4: fix check to prevent initializing reserved
> inodes") check the block group zero and prevent initializing reserved
> inodes. But in some special cases, the reserved inode may not all belong
> to the group zero, it may exist into the second group if we format
> filesystem below.
> 
>   mkfs.ext4 -b 4096 -g 8192 -N 1024 -I 4096 /dev/sda
> 
> So, it will end up triggering a false positive report of a corrupted
> file system. This patch fix it by avoid check reserved inodes if no free
> inode blocks will be zeroed.
> 
> Fixes: 50122847007 ("ext4: fix check to prevent initializing reserved inodes")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> Suggested-by: Jan Kara <jack@suse.cz>

Thanks! The patch looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> Changes since v1:
>  - Splict check of used_blks and used_inos to make code more
>    comprehensible as Jan suggested.
> 
>  fs/ext4/ialloc.c | 48 ++++++++++++++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 633ae7becd61..5f0c7fe32672 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -1513,6 +1513,7 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
>  	handle_t *handle;
>  	ext4_fsblk_t blk;
>  	int num, ret = 0, used_blks = 0;
> +	unsigned long used_inos = 0;
>  
>  	/* This should not happen, but just to be sure check this */
>  	if (sb_rdonly(sb)) {
> @@ -1543,22 +1544,37 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
>  	 * used inodes so we need to skip blocks with used inodes in
>  	 * inode table.
>  	 */
> -	if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)))
> -		used_blks = DIV_ROUND_UP((EXT4_INODES_PER_GROUP(sb) -
> -			    ext4_itable_unused_count(sb, gdp)),
> -			    sbi->s_inodes_per_block);
> -
> -	if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group) ||
> -	    ((group == 0) && ((EXT4_INODES_PER_GROUP(sb) -
> -			       ext4_itable_unused_count(sb, gdp)) <
> -			      EXT4_FIRST_INO(sb)))) {
> -		ext4_error(sb, "Something is wrong with group %u: "
> -			   "used itable blocks: %d; "
> -			   "itable unused count: %u",
> -			   group, used_blks,
> -			   ext4_itable_unused_count(sb, gdp));
> -		ret = 1;
> -		goto err_out;
> +	if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT))) {
> +		used_inos = EXT4_INODES_PER_GROUP(sb) -
> +			    ext4_itable_unused_count(sb, gdp);
> +		used_blks = DIV_ROUND_UP(used_inos, sbi->s_inodes_per_block);
> +
> +		/* Bogus inode unused count? */
> +		if (used_blks < 0 || used_blks > sbi->s_itb_per_group) {
> +			ext4_error(sb, "Something is wrong with group %u: "
> +				   "used itable blocks: %d; "
> +				   "itable unused count: %u",
> +				   group, used_blks,
> +				   ext4_itable_unused_count(sb, gdp));
> +			ret = 1;
> +			goto err_out;
> +		}
> +
> +		used_inos += group * EXT4_INODES_PER_GROUP(sb);
> +		/*
> +		 * Are there some uninitialized inodes in the inode table
> +		 * before the first normal inode?
> +		 */
> +		if ((used_blks != sbi->s_itb_per_group) &&
> +		     (used_inos < EXT4_FIRST_INO(sb))) {
> +			ext4_error(sb, "Something is wrong with group %u: "
> +				   "itable unused count: %u; "
> +				   "itables initialized count: %ld",
> +				   group, ext4_itable_unused_count(sb, gdp),
> +				   used_inos);
> +			ret = 1;
> +			goto err_out;
> +		}
>  	}
>  
>  	blk = ext4_inode_table(sb, gdp) + used_blks;
> -- 
> 2.25.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] ext4: fix check to prevent false positive report of incorrect used inodes
  2021-03-31 12:23 ` Jan Kara
@ 2021-04-09 15:32   ` Theodore Ts'o
  0 siblings, 0 replies; 3+ messages in thread
From: Theodore Ts'o @ 2021-04-09 15:32 UTC (permalink / raw)
  To: Jan Kara; +Cc: Zhang Yi, linux-ext4, adilger.kernel

On Wed, Mar 31, 2021 at 02:23:54PM +0200, Jan Kara wrote:
> On Wed 31-03-21 20:15:16, Zhang Yi wrote:
> > Commit <50122847007> ("ext4: fix check to prevent initializing reserved
> > inodes") check the block group zero and prevent initializing reserved
> > inodes. But in some special cases, the reserved inode may not all belong
> > to the group zero, it may exist into the second group if we format
> > filesystem below.
> > 
> >   mkfs.ext4 -b 4096 -g 8192 -N 1024 -I 4096 /dev/sda
> > 
> > So, it will end up triggering a false positive report of a corrupted
> > file system. This patch fix it by avoid check reserved inodes if no free
> > inode blocks will be zeroed.
> > 
> > Fixes: 50122847007 ("ext4: fix check to prevent initializing reserved inodes")
> > Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> > Suggested-by: Jan Kara <jack@suse.cz>
> 
> Thanks! The patch looks good to me. Feel free to add:
> 
> Reviewed-by: Jan Kara <jack@suse.cz>

Thanks, applied.

					- Ted

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-31 12:15 [PATCH v2] ext4: fix check to prevent false positive report of incorrect used inodes Zhang Yi
2021-03-31 12:23 ` Jan Kara
2021-04-09 15:32   ` Theodore Ts'o

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git